GH-39484: [Java] Support 256 bit decimals in JdbcToArrowUtils #39485
Conversation
|
|
| @@ -169,7 +171,11 @@ public static ArrowType getArrowTypeFromJdbcType(final JdbcFieldInfo fieldInfo, | |||
| case Types.DECIMAL: | |||
| int precision = fieldInfo.getPrecision(); | |||
| int scale = fieldInfo.getScale(); | |||
| return new ArrowType.Decimal(precision, scale, 128); | |||
| if (precision > 38) { | |||
There was a problem hiding this comment.
Is this correct? this is how BigQuery seems to handle it, but not sure if it's any different on the Arrow side.
There was a problem hiding this comment.
Should be right. Arrow Decimal128 maxes out at 38 as well.
github-actions
bot
added
awaiting committer review
| if (precision > 18) { | ||
| return new ArrowType.Decimal(precision, 0); |
There was a problem hiding this comment.
We ran into an issue with this on Snowflake, which can have precision up to 38 (128 bit). Although technically some 19 precision numbers can be expressed in 64 bit, the safest way to ensure the conversion always works seems to be setting it to Decimal if the column precision > 18.
I can remove this, or add it in a separate PR if ya'll think think that's better.
There was a problem hiding this comment.
Nvm... I removed this since it broke some tests.
aiguofer
force-pushed
the
support_256_decimal
branch
from
162a45f to
2b7624d
Compare
| @@ -169,7 +171,11 @@ public static ArrowType getArrowTypeFromJdbcType(final JdbcFieldInfo fieldInfo, | |||
| case Types.DECIMAL: | |||
| int precision = fieldInfo.getPrecision(); | |||
| int scale = fieldInfo.getScale(); | |||
| return new ArrowType.Decimal(precision, scale, 128); | |||
| if (precision > 38) { | |||
There was a problem hiding this comment.
Should be right. Arrow Decimal128 maxes out at 38 as well.
|
After merging your PR, Conbench analyzed the 5 benchmarking runs that have been run so far on merge-commit 1622a2e. There were no benchmark performance regressions. 🎉 The full Conbench report has more details. It also includes information about 1 possible false positive for unstable benchmarks that are known to sometimes produce them. |
…pache#39485) ### Rationale for this change This PR allows users of `JdbcToArrowUtils` to convert 256 bit decimals. ### What changes are included in this PR? Add a `Decimal256Consumer` and logic to ### Are these changes tested? No, at this point there are no good tests for JDBC consumers. ### Are there any user-facing changes? Converting 256 bit decimals and ints bigger than 64 bit should now work * Closes: apache#39484 Authored-by: Diego Fernandez <aiguo.fernandez@gmail.com> Signed-off-by: David Li <li.davidm96@gmail.com>
…pache#39485) ### Rationale for this change This PR allows users of `JdbcToArrowUtils` to convert 256 bit decimals. ### What changes are included in this PR? Add a `Decimal256Consumer` and logic to ### Are these changes tested? No, at this point there are no good tests for JDBC consumers. ### Are there any user-facing changes? Converting 256 bit decimals and ints bigger than 64 bit should now work * Closes: apache#39484 Authored-by: Diego Fernandez <aiguo.fernandez@gmail.com> Signed-off-by: David Li <li.davidm96@gmail.com>
…pache#39485) ### Rationale for this change This PR allows users of `JdbcToArrowUtils` to convert 256 bit decimals. ### What changes are included in this PR? Add a `Decimal256Consumer` and logic to ### Are these changes tested? No, at this point there are no good tests for JDBC consumers. ### Are there any user-facing changes? Converting 256 bit decimals and ints bigger than 64 bit should now work * Closes: apache#39484 Authored-by: Diego Fernandez <aiguo.fernandez@gmail.com> Signed-off-by: David Li <li.davidm96@gmail.com>
Rationale for this change
This PR allows users of
JdbcToArrowUtilsto convert 256 bit decimals.What changes are included in this PR?
Add a
Decimal256Consumerand logic toAre these changes tested?
No, at this point there are no good tests for JDBC consumers.
Are there any user-facing changes?
Converting 256 bit decimals and ints bigger than 64 bit should now work