feat(client-v2, jdbc-v2): add QBit data type support#2939
Open
polyglotAI-bot wants to merge 3 commits into
Open
feat(client-v2, jdbc-v2): add QBit data type support#2939polyglotAI-bot wants to merge 3 commits into
polyglotAI-bot wants to merge 3 commits into
Conversation
QBit(element_type, dimension) is transmitted over RowBinary exactly like Array(element_type) - a var-int length followed by that many element values - so it is read and written as a Java array of its element type (float[] for BFloat16/Float32, double[] for Float64) reusing the existing array codec, while keeping dataType == QBit for metadata fidelity. In jdbc-v2 QBit maps to java.sql.Types.ARRAY and is exposed as a java.sql.Array. Element types: BFloat16, Float32, Float64. The bit-transposed on-disk layout is a server storage detail and is not exposed to the client. Implements: #2610
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Client V2 CoverageCoverage Report
Class Coverage
|
JDBC V2 CoverageCoverage Report
Class Coverage
|
JDBC V1 CoverageCoverage Report
Class Coverage
|
Client V1 CoverageCoverage Report
Class Coverage
|
QBit(element_type, dimension) is a fixed-size vector: the server requires exactly `dimension` elements and rejects any other length over RowBinary with a late SERIALIZATION_ERROR (verified against server 26.5). The write path reused serializeArrayData, which wrote the actual array/list length without checking it, so a wrong-sized (including empty) vector was serialized like a normal Array and only failed on the server after a round-trip. Split the QBit case out of the Array fall-through into serializeQBitData, which validates the element count against column.getPrecision() (the dimension) and throws a clear IllegalArgumentException naming the column, expected dimension, and actual length before delegating to serializeArrayData. This mirrors the client-side length enforcement already applied to the other fixed-size type, FixedString(N). A correctly-sized QBit is still byte-for-byte identical to Array(element_type). Addresses Cursor Bugbot review on PR #2939.
TriageCategory: Summary What this impacts
Concerns
Required reviewer action
|
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 0e1a6bb. Configure here.
A non-null QBit value that is neither a Java array nor a List fell through serializeQBitData to serializeArrayData, which writes no bytes for such a value, desynchronizing the RowBinary stream so the following columns are misread or rejected by the server. Reject it up-front with a clear IllegalArgumentException, consistent with the existing fixed-dimension validation and the Geometry reject-unsupported path. Addresses Cursor Bugbot review on #2939.
|
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
1 participant
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
Implements #2610.
Adds read/write support for the experimental
QBit(element_type, dimension)vector data type (ClickHouse25.10+) toclient-v2andjdbc-v2.QBitwas previously only an unimplemented type constant (#2646), and every data-type test skipped it with// TODO: add support.Supported element types:
BFloat16,Float32,Float64(the server rejects any other element type). Theallow_experimental_qbit_typeserver setting is required only to create aQBitcolumn — insert/select on an existing column do not need it.Design
The key finding (verified byte-for-byte against a live ClickHouse
26.5server): aQBit(element_type, dimension)value is transmitted overRowBinaryexactly likeArray(element_type)— a var-int length prefix followed by that many little-endian element values. The bit-transposedTuple(FixedString(...))layout referenced in the issue is a MergeTree on-disk storage detail, not the wire format; on the wire the server presents (and accepts) the logical vector as an array, and reports the column type string asQBit(Float32, 8).So
QBitis implemented as a one-level array of its element type, reusing the existing, well-tested array code paths, while keepingdataType == QBitfor metadata fidelity:clickhouse-dataClickHouseColumn): aQBitbranch parses(element_type, dimension), builds the nested element column, setsarrayLevel = 1/arrayBaseColumn, and keeps the dimension as the columnprecision.client-v2BinaryStreamReader):case QBitreuses theArrayread (readArray).client-v2SerializerUtils):case QBitreusesserializeArrayData.float[]forBFloat16/Float32,double[]forFloat64— identical toArray(element_type), through generic records and POJO binding.QBitmaps tojava.sql.Types.ARRAY;getObject/getArrayreturn ajava.sql.Arrayof the element type, andPreparedStatementbinding renders it via the array-literal path (DataTypeConverter).Entry points covered:
client-v2read + write (generic + POJO);jdbc-v2read + write; the shared type parser.Changes
clickhouse-data/.../ClickHouseColumn.java—KEYWORD_QBIT;QBitparse branch;QBitcase inupdate().client-v2/.../BinaryStreamReader.java—case QBit→ array read.client-v2/.../SerializerUtils.java—case QBit→ array write.client-v2/.../internal/DataTypeConverter.java—case QBit→ array-literal rendering (JDBC/SQL param path).jdbc-v2/.../internal/JdbcUtils.java—QBit→JDBCType.ARRAY.jdbc-v2/.../types/Array.java— resolve the array element column viagetArrayNestedLevel() > 0(wasisArray(), which isfalseforQBit) sogetBaseType()/getBaseTypeName()report the element type, not the wholeQBittype.docs/features.md,CHANGELOG.md— documentation.Test
ClickHouseColumnTest(unit,@DataProvider): parsingQBit(E, N)for all three element types (dataType, element column, dimension) + a negative missing-dimension case.DataTypeTests(client-v2 integration): full client write → server → read round-trip forFloat32/Float64/BFloat16. TheQBitcolumn sits mid-schema with a trailingInt32column, so a codec byte-misalignment would shift it and fail the assertion.JdbcDataTypeTests(jdbc-v2 integration):PreparedStatementwrite +ResultSetread; assertsgetColumnType == ARRAY,Array.getBaseType == FLOAT/getBaseTypeName == "Float32", and the element values.Regression: full
clickhouse-dataunit suite (1660) and the existingArrayintegration tests in both modules pass unedited.Notes
QBitnested insideVariant/Dynamic— the server support there is experimental and the pre-existing tests already skip those combinations; left out of scope.nullinto a non-nullableQBitcolumn throws a clear client-side error (rather than emitting an empty vector asArraydoes), which is intentional givenQBit's fixed dimension — an empty vector is never valid.Pre-PR validation gate
Array(element_type);QBit→ARRAYlikeGeometry/Point)docs/features.md(client-v2 + jdbc-v2) andCHANGELOG.mdupdatedAGENTS.md/docs/features.md/docs/changes_checklist.md