Fix client-v2: serialize Nested columns in RowBinary writer - #2936
Merged
Conversation
RowBinaryFormatWriter threw `UnsupportedOperationException: Unsupported data type: Nested` when inserting into a table whose schema keeps an un-flattened `Nested(...)` column (created with `flatten_nested = 0`), because SerializerUtils.serializeData had no case for the Nested type and fell through to serializePrimitiveData's default throw. A `Nested(f1 T1, ..., fN TN)` column has the same RowBinary layout as `Array(Tuple(T1, ..., TN))` - a var-uint element count followed by that many tuples - which is exactly how BinaryStreamReader.readNested reads it. serializeData now serializes it symmetrically. Fixes: #2477
Client V2 CoverageCoverage Report
Class Coverage
|
JDBC V2 CoverageCoverage Report
Class Coverage
|
JDBC V1 CoverageCoverage Report
Class Coverage
|
Client V1 CoverageCoverage Report
Class Coverage
|
8 tasks
Member
|
@cursor review |
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes client-v2 RowBinary insert serialization for un-flattened Nested(...) columns (tables created with flatten_nested = 0), aligning the write path with the existing read-path behavior and ClickHouse’s RowBinary wire layout.
Changes:
- Add
Nestedhandling inSerializerUtils.serializeDataand implementNestedserialization asArray(Tuple(...))(var-uint row count + tuples). - Add unit round-trip coverage for
Nested(...)serialization (including nullable field alignment cases). - Add an integration test that reproduces the reported failure through
RowBinaryFormatWriter+client.insert, and updateCHANGELOG.md.
Reviewed changes
Copilot reviewed 4 out of 4 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 | Adds Nested case and serializes it with the correct RowBinary layout (equivalent to Array(Tuple(...))). |
| client-v2/src/test/java/com/clickhouse/client/api/data_formats/internal/SerializerUtilsTest.java | Adds deterministic unit round-trip tests covering Nested(...), including nullable-in-middle alignment. |
| client-v2/src/test/java/com/clickhouse/client/datatypes/RowBinaryFormatWriterTest.java | Adds integration coverage for flatten_nested = 0 tables using an un-flattened Nested(...) column. |
| CHANGELOG.md | Documents the client-v2 bug fix and links the issue. |
…nd-trip Adds an Object[][] "matrix array" case to the Nested RowBinary round-trip DataProvider, exercising convertArrayValueToList's array branch and serializeTupleData's array branch (the existing cases used List shapes only). Documents that array-shaped Nested values round-trip correctly.
TriageCategory: Summary What this impacts
Concerns
Required reviewer action
|
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!
1 issue from previous review remains unresolved.
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit e0f4674. Configure here.
chernser
requested changes
Jul 24, 2026
chernser
left a comment
chernser
left a comment
Contributor
There was a problem hiding this comment.
Add support in JDBC:
- Add tests
- Make sure it works with
java.sql.Array - User may request
java.sql.Array#getResultSet()what also should work
A `java.sql.Array` wrapping an un-flattened `Nested(f1 T1, ..., fN TN)` column (tables created with `flatten_nested = 0`) threw from `getResultSet()`: `ArrayResultSet` derived the VALUE column as the first nested field instead of the element `Tuple(...)`, so reading each element tried to coerce the tuple `Object[]` to that field's scalar type. Resolve the element as the tuple for Nested columns (shared `elementColumn` helper, also used by the getArray element path). Insert (createArrayOf/setArray/ setObject) and read (getArray/getObject/getResultSet) of Nested columns are covered by a new jdbc-v2 integration test. Fixes: #2477
Collaborator
Author
|
@chernser addressed in 5112ee3 (jdbc-v2). I traced the JDBC (
New integration test |
… type testNestedType inserts into Nested(a Int8, b Nullable(String)); align the createArrayOf element type to Tuple(Int8, Nullable(String)) so the declared element type honestly matches the column. Rendering is unchanged (the array's base data type is Tuple either way, and a null field renders as NULL value-driven), so this only makes the declared type honest and removes brittleness if createArrayOf ever validates element types more strictly.
Member
|
@Cursore review |
…sted-writer # Conflicts: # CHANGELOG.md
chernser
reviewed
Jul 28, 2026
chernser
reviewed
Jul 28, 2026
chernser
requested changes
Jul 28, 2026
chernser
left a comment
chernser
left a comment
Contributor
There was a problem hiding this comment.
- add Dynamic encoding/decoding for nested
- add more integration tests. Especially with arrays and complex types
- see other comments
…ests - CHANGELOG: reclassify the Nested change as a New Features entry (new type support), consolidating the client-v2 write and jdbc-v2 exposure. - docs/features.md: add a client-v2 entry describing how un-flattened Nested writes are exposed (Client#insert / RowBinaryFormatWriter#setValue). - client-v2 tests: add two integration tests covering un-flattened Nested with array / nested-array / tuple / nullable fields and with varied scalar type families, each with a fixed-width trailing field. Refs: #2477
Collaborator
Author
|
@chernser thanks for the review — addressed the points on head
|
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 2576802. Configure here.
…sted-writer # Conflicts: # client-v2/src/main/java/com/clickhouse/client/api/data_formats/internal/SerializerUtils.java # client-v2/src/test/java/com/clickhouse/client/api/data_formats/internal/SerializerUtilsTest.java
…sted-writer # Conflicts: # CHANGELOG.md
|
chernser
approved these changes
Jul 29, 2026
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 #2477.
RowBinaryFormatWriter(client-v2) threwUnsupportedOperationException: Unsupported data type: Nestedwhen inserting into a table whose schema keeps an un-flattenedNested(...)column. ANestedcolumn stays typed asNested(...)(rather than being expanded into parallelArray(...)sub-columns) when the table is created withflatten_nested = 0, which is the case the reporter hit.SerializerUtils.serializeDatahad nocase Nested, so it fell through toserializePrimitiveData'sdefault:branch and threw.A
Nested(f1 T1, ..., fN TN)column has the sameRowBinarylayout asArray(Tuple(T1, ..., TN)): a var-uint element (row) count followed by that many tuples, each carrying the N field values in declaration order. I confirmed this byte-for-byte against a live ClickHouse 26.5 server —SELECT n FROM <nested table> FORMAT RowBinaryis identical to the equivalentArray(Tuple(...))literal. It is also exactly how the read path already interprets it (BinaryStreamReader.readNested). The writer now serializes it symmetrically.Changes
SerializerUtils.serializeData: addedcase Nested→ new private helperserializeNestedTypeData.serializeNestedTypeData: writes the var-uint element count, then serializes each element via the existingserializeTupleData(aNestedcolumn'sgetNestedColumns()are the tuple field columns). Anullvalue serializes as a zero-length array, matchingserializeArrayData. Nullable fields inside the tuple keep their null-marker byte via the existingserializeNestedDatapath, soNested(a UInt32, b Nullable(String))round-trips correctly.No public API change; no
docs/features.mdentry (it has no RowBinary-writer type list — theNestedmention there is the unrelated JDBC result-set type map). No existing tests modified.Test
SerializerUtilsTest, deterministic round-tripserializeData→BinaryStreamReader.readValue, via@DataProvider):Nested(a Int32, b String)(two rows);Nested(a Int32, b Nullable(String), c Float64)with a present and a null middle field plus a trailing fixed-widthFloat64(so a dropped null-marker byte misaligns the float and is caught); and an emptyNested.RowBinaryFormatWriterTest.writeNestedTypeTests, the reported entry point —client.insert+RowBinaryFormatWriter, thenSELECTback): aflatten_nested = 0table withn Nested(a UInt32, b Nullable(String)).client.insertpath with "Insert request failed", caused by theUnsupportedOperationException) and pass with it. Full module runs stay green (44 unit, 18 integration in the touched files); no existing test was changed.Pre-PR validation gate
main, passes on branch — unit and integration)case Nested; wire format =Array(Tuple(...)))serializeData, the single write funnel; proven via theRowBinaryFormatWriter/client.insertintegration test)CHANGELOG.mdupdated (client-v2 Bug Fixes)AGENTS.md/docs/changes_checklist.mdchanges_checklist.mdserializeNestedTypeDataisprivate, does one thing (serialize aNestedcolumn asArray(Tuple(...))), reuses existing helpers rather than duplicating them, and is covered by behavior tests.RowBinaryoutput.Note on "probably more unsupported types"
While auditing the writer against the reader I found one further genuine, reachable gap of the same kind:
SimpleAggregateFunction(func, T)is handled by the reader but not the writer. It is intentionally not included here to keep this PR focused on the reportedNestedbug — it has an extra subtlety worth its own change: when the underlying type isNullable(T)the null-marker lives on the inner column while the outer column is non-nullable, so both the serializer delegation andwriteValuePreambleneed to account for that. Happy to follow up with a dedicated PR. (DateTime32is not a gap — DESCRIBE normalizes it toDateTime, which is already handled.)