You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
GH-1377: [FlightRPC] Bind DOUBLE to DECIMAL parameters - #1378
JDBC allows setDouble on a DECIMAL column, and tools rely on it.
What changes are included in this PR?
DecimalParameterCoercer widens the Avatica rep to BigDecimal and scales it to the target vector; the converter calls it instead of casting.
Are these changes tested?
Yes. DecimalAvaticaParameterConverterTest covers DOUBLE, FLOAT, INTEGER, LONG and BigDecimal binding, rounding to the vector scale, NaN and infinity, and a non-decimal vector. Reverting the converter to the cast fails 10 of its 14 tests.
Are there any user-facing changes?
setDouble, setFloat, setInt and setLong now bind to DECIMAL parameters, and setBigDecimal accepts a value whose scale differs from the column's.
The reason will be displayed to describe this comment to others. Learn more.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Enables JDBC-style numeric bindings (e.g., setDouble) to Arrow DECIMAL parameters by coercing Avatica TypedValue inputs into scaled BigDecimal values before writing to DecimalVector.
Changes:
Added DecimalParameterNarrower to widen numeric Avatica reps into BigDecimal and apply vector scale rounding (HALF_UP).
Updated DecimalAvaticaParameterConverter to use the new coercion path instead of directly casting.
Added comprehensive JUnit tests for multiple numeric reps, rounding behavior, and non-decimal vectors.
The reason will be displayed to describe this comment to others. Learn more.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
lidavidm
changed the title
GH-1377: [FlightRPC][Java] Bind DOUBLE to DECIMAL parameters
GH-1377: [FlightRPC] Bind DOUBLE to DECIMAL parameters
Oct 22, 2026
For Float inputs, promoting to double before decimal conversion introduces extra digits: 1.0005f at scale 3 rounds to 1.000 instead of 1.001, and 0.1f at scale 9 becomes 0.100000001. Convert finite floats through Float.toString(), keeping BigDecimal.valueOf for doubles. Add a regression test with an inexact float, since the current 2.5f case cannot detect this.
DecimalAvaticaParameterConverter cast the local value to BigDecimal, which
only holds when the client called setBigDecimal. TypedValue.toLocal() returns
a Double for Rep.DOUBLE, so setDouble against a decimal parameter failed with
"Binding value of type DOUBLE is not yet supported".
The converter now widens the value and scales it to the vector, which rejects
any scale but its own.
BigDecimal.valueOf raises NumberFormatException for non-finite doubles,
which AvaticaParameterBinder does not catch. Before this converter widened
doubles, the same value failed as a ClassCastException and came back as
"Binding value of type DOUBLE is not yet supported". Throwing the cast error
keeps that message.
Also tests that a value too large for a long survives: Avatica has no rep
for BigInteger, so toLocal() returns a BigDecimal and the longValue()
branch is unreachable for it.
Widening a Float to double before BigDecimal exposes the gap between the two
binary formats: 1.0005f arrives as 1.000499963760376 and rounds to 1.000 at
scale 3, not the 1.001 the client sent. Float.toString gives the shortest
decimal that round-trips at float width.
The existing 2.5f test cannot catch this, as 2.5 is exact in binary.
Fixed the float precision point from the last review.
toLocal() does hand back a Float for Rep.FLOAT, so the branch was reachable. Widening it to a double first exposes the gap between the two binary formats: 1.0005f arrives as 1.000499963760376 and rounds to 1.000 at scale 3, not the 1.001 the client sent. The coercer now reads a float through Float.toString(), which gives the shortest decimal that round-trips at float width, and keeps BigDecimal.valueOf for doubles.
testBindFloatKeepsTheDigitsTheClientSent covers it. The existing 2.5f case can't, since 2.5 is exact in binary.
Also rebased on main, ran mvn spotless:apply for the formatting failure, and changed the issue line to Closes #1377. so the linked-issue check sees it.
@lidavidm could you approve the workflow runs when you get a chance? The rebase reset the approval, so Test, Integration, Dev and RC are all sitting at action_required and haven't run against the fixes.
Ensure PR format has gone green.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Rationale for this change
JDBC allows
setDoubleon a DECIMAL column, and tools rely on it.What changes are included in this PR?
DecimalParameterCoercerwidens the Avatica rep toBigDecimaland scales it to the target vector; the converter calls it instead of casting.Are these changes tested?
Yes.
DecimalAvaticaParameterConverterTestcovers DOUBLE, FLOAT, INTEGER, LONG and BigDecimal binding, rounding to the vector scale, NaN and infinity, and a non-decimal vector. Reverting the converter to the cast fails 10 of its 14 tests.Are there any user-facing changes?
setDouble,setFloat,setIntandsetLongnow bind to DECIMAL parameters, andsetBigDecimalaccepts a value whose scale differs from the column's.Closes #1377.