Fix client-v2: accept any Number for Float32/Float64 insert values#2933
Open
polyglotAI-bot wants to merge 1 commit into
Open
Fix client-v2: accept any Number for Float32/Float64 insert values#2933polyglotAI-bot wants to merge 1 commit into
polyglotAI-bot wants to merge 1 commit into
Conversation
Float32/Float64 columns threw ClassCastException when a value whose boxed type did not match the column was supplied through the Object-typed insert surface (for example a Double, the natural type of a Java literal like 1.5, for a Float32 column, or a Float for a Float64 column). The RowBinary serializer narrowed with a direct `(float) value` / `(double) value` cast, which is a narrowing reference conversion to Float/Double followed by unboxing and fails for any other boxed numeric type. Route the two branches through the shared NumberConverter.toFloat/toDouble converters, matching the neighboring UInt/Decimal branches, so the float columns accept any Number (and String/Boolean) exactly like the Int* columns already do. Fixes: #2930
Client V2 CoverageCoverage Report
Class Coverage
|
JDBC V2 CoverageCoverage Report
Class Coverage
|
JDBC V1 CoverageCoverage Report
Class Coverage
|
Client V1 CoverageCoverage Report
Class Coverage
|
Member
|
@cursor review |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit fb6ed59. Configure here.
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes a client-v2 RowBinary insert serialization bug where Float32/Float64 columns could throw ClassCastException when values were supplied via the generic Object insert surface using a different boxed Number type (e.g., Double into Float32, Float into Float64).
Changes:
- Updated
SerializerUtils.serializePrimitiveDatato serializeFloat32/Float64usingNumberConverter.toFloat(Object)/toDouble(Object)instead of direct unboxing casts. - Added TestNG
@DataProvider-driven coverage to ensure float columns accept variousNumbersubtypes (plusString/Boolean) and to verify unsupported types are rejected. - Added a
CHANGELOG.mdentry documenting the fix and the expanded accepted input types.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
client-v2/src/main/java/com/clickhouse/client/api/data_formats/internal/SerializerUtils.java |
Fixes float serialization by routing through NumberConverter for polymorphic Object value handling. |
client-v2/src/test/java/com/clickhouse/client/api/data_formats/internal/SerializerUtilsPrimitiveSerializationTests.java |
Adds regression tests for Float32/Float64 inserts across diverse numeric and supported non-numeric inputs. |
CHANGELOG.md |
Documents the user-visible fix for Float32/Float64 insert serialization. |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Description
Fixes #2930.
client-v2'sRowBinaryserializer threwClassCastExceptionwhen a value whose boxed Java type did not match aFloat32/Float64column was supplied through theObject-typed insert surface — for example aDouble(the natural type of a Java float literal like1.5, and whatNumber#doubleValue()and many JSON parsers produce) for aFloat32column, or aFloatfor aFloat64column.SerializerUtils.serializePrimitiveDatanarrowed with a direct(float) value/(double) valuecast, which the compiler implements as a narrowing reference conversion toFloat/Doublefollowed by unboxing, so any other boxed numeric type fails at runtime. TheInt*branches never had this problem because they route through polymorphic converters, which madeFloat32/Float64the only numeric columns whose insert serialization was not polymorphic overNumber.Changes
SerializerUtils.serializePrimitiveData: route theFloat32/Float64branches through the existing sharedNumberConverter.toFloat(Object)/toDouble(Object)converters — the same class the neighboringUInt64+/Decimal*branches and the read path already use — instead of a direct unboxing cast. The float columns now accept anyNumber(andString/Boolean), matching theInt*columns. This is additive: values that already worked (Float→Float32,Double→Float64) are unchanged.Test
testSerializeFloatColumnFromVariousNumberTypes(a@DataProvider-driven test) inSerializerUtilsPrimitiveSerializationTests— the existing file for "serialize a value whose Java type does not match the column type". It round-trips each value throughserializeData→RowBinary→ reader for bothFloat32andFloat64acrossDouble/Float/Integer/Long/Short/Byte/BigInteger/BigDecimal/a customNumber/String/Boolean, plus contrast rows (same-type value, which passed before the fix) and out-of-range narrowing (Double.MAX_VALUE→Float32infinity). Without the fix, 24 of the rows fail withClassCastException; with it, all pass.testSerializeFloatColumnRejectsUnsupportedValueasserting an unsupported value type throwsIllegalArgumentException(the shared converter's contract, matching the other numeric branches).CHANGELOG.md.changes_checklist.md
Float→Float32andDouble→Float64; the change only makes previously-rejected input types serialize to the correct 4/8-byte value, verified by round-trip against the reader.nullinto a non-nullableFloat32/Float64column now throwsIllegalArgumentException(via the shared converter) instead ofNullPointerException, consistent with theInt*/Decimalbranches.nullinto aNullable(...)column is handled earlier and is unaffected.Pre-PR validation gate
ClassCastExceptiononmain)