[CALCITE-5094] Calcite JDBC Adapter and Avatica should support MySQL UNSIGNED types of TINYINT, SMALLINT, INT, BIGINT #275

strongduanmu · 2025-02-02T10:27:48Z

For more details, refer jira: https://issues.apache.org/jira/browse/CALCITE-5094

mihaibudiu

The changes look fine, but it's hard to tell whether all code paths that may be relevant have been handled until we have some end-to-end tests.

strongduanmu · 2025-02-03T09:25:57Z

The changes look fine, but it's hard to tell whether all code paths that may be relevant have been handled until we have some end-to-end tests.

Yes, I agree with you, end-to-end testing is very important. Currently, I have tested different unsigned types with calcite avatica 1.26.0-SNAPSHOT and they all work fine. When this PR is merged, I will try to support unsigned types in calcite and add more end-to-end test cases.

mihaibudiu · 2025-02-03T19:28:31Z

I am wondering whether it will pay off to actually build a set of classes UnsignedInt, UnsignedShort, etc, which would encapsulate these abstractions in a clean way. I am not sure that this is the right place for them - BTW. But I think you will need them eventually in the runtime of Calcite to properly implement the semantics of unsigned arithmetic.

mihaibudiu · 2025-02-03T19:29:22Z

I notice that Guava has some of these.

strongduanmu · 2025-02-06T01:39:06Z

@mihaibudiu Thank you for your remind, I will try to implement this feature using a class like Guava UnsignedInt.

strongduanmu · 2025-02-09T03:28:26Z

Hi @mihaibudiu, I spent some time looking at Guava. Currently, it only provides UnsignedInteger and UnsignedLong. Other UnsignedByte and UnsignedShort are not provided by Guava.

In addition, I found that MySQL provides another idea, we can handle them by mapping a wider range of Java types. For example, the BIGINT type is usually mapped to java.lang.Long for processing, while BIGINT UNSIGNED needs to be mapped to java.math.BigInteger for processing. For more type mappings, please refer to the document: https://dev.mysql.com/doc/connector-j/en/connector-j-reference-type-conversions.html

Through this mapping conversion method, we do not need to add any classes, and the Calcite runtime can handle UNSIGNED types. What do you think of this approach? I will keep trying if you have any suggestions.

ILuffZhe

Overall looks good.

core/src/test/java/org/apache/calcite/avatica/AvaticaResultSetThrowsSqlExceptionTest.java

core/src/main/java/org/apache/calcite/avatica/AvaticaSite.java

mihaibudiu · 2025-02-10T18:48:01Z

From my experience, in the long term it pays off to implement the right abstractions.
I expect there will be a lot of work to be done using unsigned types, including in the type checker, expression simplification, and runtime system. In particular, at runtime the enumerable implementation looks at the runtime types to make some decisions (maybe that's a bug in the implementation). So if the runtime will see a Long it won't know whether it's a real Long or an UnsignedInt in disguise.

strongduanmu · 2025-02-11T00:22:42Z

From my experience, in the long term it pays off to implement the right abstractions. I expect there will be a lot of work to be done using unsigned types, including in the type checker, expression simplification, and runtime system. In particular, at runtime the enumerable implementation looks at the runtime types to make some decisions (maybe that's a bug in the implementation). So if the runtime will see a Long it won't know whether it's a real Long or an UnsignedInt in disguise.

Hi @mihaibudiu, thank you for sharing these valuable experiences. These are issues I had not considered before. Since I am still a novice, I will spend some time to understand type checking, expression simplification, etc., and then try to add new UnsignedInt, UnsignedLong and other types.

mihaibudiu · 2025-05-28T00:46:35Z

Can we merge this PR?
Frankly, we won't know whether this stuff works until there are some end-to-end tests, so it would be nice to have a confirmation that Calcite will be able to use this.

strongduanmu · 2025-05-28T01:54:58Z

Can we merge this PR? Frankly, we won't know whether this stuff works until there are some end-to-end tests, so it would be nice to have a confirmation that Calcite will be able to use this.

Hi @mihaibudiu, according to your review suggestion, we'd better introduce unsigned type to facilitate Calcite to do some boundary checking on types. This part of work is not included in the current PR. I suggest merging this PR first, and then I will introduce https://github.com/jOOQ/jOOU in the next PR to support unsigned type. What do you think?

mihaibudiu · 2025-05-28T17:34:48Z

I don't know if Avatica needs to take a dependency on joou.
Can you write some tests in Calcite that confirm that this PR works as expected?
Such tests can be merged after this PR is merged; you will have to run them by linking Calcite with an unreleased version of Avatica.

strongduanmu · 2025-05-29T01:36:27Z

I don't know if Avatica needs to take a dependency on joou. Can you write some tests in Calcite that confirm that this PR works as expected? Such tests can be merged after this PR is merged; you will have to run them by linking Calcite with an unreleased version of Avatica.

Good suggestion, I will add tests in calcite.

mihaibudiu · 2025-06-17T18:16:00Z

There is now apache/calcite#4411; I guess the tests should use both PRs.

strongduanmu · 2025-06-18T00:26:09Z

There is now apache/calcite#4411; I guess the tests should use both PRs.

Yes, I will test in calcite when #4411 is merged.

mihaibudiu · 2025-06-19T00:18:42Z

I have resolved the issue in Jira, perhaps you should open a new one specifically mentioning Avatica.

mihaibudiu · 2025-08-22T22:30:29Z

core/src/main/java/org/apache/calcite/avatica/util/AbstractCursor.java


+    @Override
+    public UShort getUShort() throws SQLException {
+      return UShort.valueOf(String.valueOf(getLong()));


this seems a bit convoluted

This is because the UShort.valueOf method does not support long type parameters.

mihaibudiu · 2025-08-22T22:33:44Z

core/src/test/java/org/apache/calcite/avatica/CursorFactoryDeduceTest.java

+            Types.TINYINT, "TINYINT", ColumnMetaData.Rep.BYTE), true, true),
+        MetaImpl.columnMetaData("shortField", 2, ColumnMetaData.scalar(
+            Types.SMALLINT, "SMALLINT", ColumnMetaData.Rep.SHORT), true, true),
+        MetaImpl.columnMetaData("intField", 3, ColumnMetaData.scalar(


this we don't have, right?

Yes, while I was adding tests for unsigned types, I noticed that signed types were also lacking tests, so I added those tests.

mihaibudiu · 2025-08-26T00:53:18Z

Please rebase and squash the commits.

…UNSIGNED types of TINYINT, SMALLINT, INT, BIGINT

strongduanmu changed the title ~~[CALCITE-1466] Support for UNSIGNED types of int,long,float etc. in type system~~ [CALCITE-1466] Support for UNSIGNED types of TINYINT, SMALLINT, MEDIUMINT, INT BIGINT in type system Feb 2, 2025

strongduanmu changed the title ~~[CALCITE-1466] Support for UNSIGNED types of TINYINT, SMALLINT, MEDIUMINT, INT BIGINT in type system~~ [CALCITE-1466] Support for UNSIGNED types of TINYINT, SMALLINT, MEDIUMINT, INT, BIGINT in type system Feb 2, 2025

strongduanmu force-pushed the dev-CALCITE-1466 branch from 2677782 to 629a5ef Compare February 2, 2025 11:00

mihaibudiu approved these changes Feb 3, 2025

View reviewed changes

ILuffZhe approved these changes Feb 10, 2025

View reviewed changes

core/src/test/java/org/apache/calcite/avatica/AvaticaResultSetThrowsSqlExceptionTest.java Show resolved Hide resolved

core/src/main/java/org/apache/calcite/avatica/AvaticaSite.java Outdated Show resolved Hide resolved

F21 force-pushed the main branch 8 times, most recently from c7cee40 to a9dc6f8 Compare February 23, 2025 22:29

mihaibudiu mentioned this pull request May 28, 2025

[SQL] Add support for unsigned numbers feldera/feldera#4078

Closed

3 tasks

strongduanmu force-pushed the dev-CALCITE-1466 branch 2 times, most recently from cdc32af to ebaed66 Compare June 1, 2025 01:45

strongduanmu changed the title ~~[CALCITE-1466] Support for UNSIGNED types of TINYINT, SMALLINT, MEDIUMINT, INT, BIGINT in type system~~ [CALCITE-5094] Calcite JDBC Adapter and Avatica should support MySQL UNSIGNED types of TINYINT, SMALLINT, INT, BIGINT Jun 19, 2025

strongduanmu mentioned this pull request Jun 25, 2025

[CALCITE-5094] Calcite JDBC Adapter and Avatica should support MySQL UNSIGNED types of TINYINT, SMALLINT, INT, BIGINT apache/calcite#4424

Merged

xiedeyantu added the needs-a-final-review label Aug 21, 2025

mihaibudiu approved these changes Aug 22, 2025

View reviewed changes

xiedeyantu removed the needs-a-final-review label Aug 26, 2025

strongduanmu force-pushed the dev-CALCITE-1466 branch from 0a7faf9 to 7ce7b5e Compare August 26, 2025 01:21

[CALCITE-5094] Calcite JDBC Adapter and Avatica should support MySQL …

274b117

…UNSIGNED types of TINYINT, SMALLINT, INT, BIGINT

strongduanmu force-pushed the dev-CALCITE-1466 branch from 7ce7b5e to 274b117 Compare August 26, 2025 01:23

mihaibudiu merged commit 9e6f954 into apache:main Aug 26, 2025
12 of 13 checks passed

strongduanmu deleted the dev-CALCITE-1466 branch August 26, 2025 04:27

[CALCITE-5094] Calcite JDBC Adapter and Avatica should support MySQL UNSIGNED types of TINYINT, SMALLINT, INT, BIGINT #275

[CALCITE-5094] Calcite JDBC Adapter and Avatica should support MySQL UNSIGNED types of TINYINT, SMALLINT, INT, BIGINT #275

Uh oh!

Conversation

strongduanmu commented Feb 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mihaibudiu left a comment

Choose a reason for hiding this comment

Uh oh!

strongduanmu commented Feb 3, 2025

Uh oh!

mihaibudiu commented Feb 3, 2025

Uh oh!

mihaibudiu commented Feb 3, 2025

Uh oh!

strongduanmu commented Feb 6, 2025

Uh oh!

strongduanmu commented Feb 9, 2025

Uh oh!

ILuffZhe left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mihaibudiu commented Feb 10, 2025

Uh oh!

strongduanmu commented Feb 11, 2025

Uh oh!

mihaibudiu commented May 28, 2025

Uh oh!

strongduanmu commented May 28, 2025

Uh oh!

mihaibudiu commented May 28, 2025

Uh oh!

strongduanmu commented May 29, 2025

Uh oh!

mihaibudiu commented Jun 17, 2025

Uh oh!

strongduanmu commented Jun 18, 2025

Uh oh!

mihaibudiu commented Jun 19, 2025

Uh oh!

mihaibudiu Aug 22, 2025

Choose a reason for hiding this comment

Uh oh!

strongduanmu Aug 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mihaibudiu Aug 22, 2025

Choose a reason for hiding this comment

Uh oh!

strongduanmu Aug 23, 2025

Choose a reason for hiding this comment

Uh oh!

mihaibudiu commented Aug 26, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

strongduanmu commented Feb 2, 2025 •

edited

Loading

strongduanmu Aug 23, 2025 •

edited

Loading