Skip to content

Throw instead of dropping data for overlapping JSON paths - #581

Merged
alex-clickhouse merged 5 commits into
mainfrom
polyglot/json-overlapping-typed-hints
Aug 24, 2026
Merged

alex-clickhouse merged 5 commits into
mainfrom
polyglot/json-overlapping-typed-hints

Conversation

@polyglotAI-bot

@polyglotAI-bot polyglotAI-bot commented Aug 23, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

ClickHouse accepts a JSON column which declares a path both as a value and as the parent of another path:

CREATE TABLE t (data JSON(a Int64, a.b Int64)) ENGINE = Memory;

Both paths are present in every row, so the server renders the row with a duplicate key:

row server renders
{"a.b": 7} {"a":0,"a":{"b":7}}
{"a": 5} {"a":5,"a":{"b":0}}

A JsonObject cannot hold two values for one key, and the driver silently picked one:

column row driver before driver now
JSON(a Nullable(Int64), a.b Nullable(Int64)) {"a.b":7} {"a":{"b":7}} unchanged
JSON(a Nullable(Int64), a.b Nullable(Int64)) {"a":5} {"a":5} unchanged
JSON(a Int64, a.b Int64) {"a.b":7} {"a":0} — the 7 is gone throws
JSON(a Int64, a.b Int64) {"a":5} {"a":5} — the subtree is gone throws
JSON(a Map(String, Int64)) {"a":{"b":1},"a.b":7} {"a":{"b":7}} — the map entry is gone throws
JSON(a Map(String, Int64)) {"a":{"x":1},"a.b":7} {"a":{"x":1,"b":7}} — two values merged into one throws

With non-Nullable paths an absent path materializes as 0, so the leaf is never null and always overwrites the subtree. With a Map path the loss has a second shape: a map decodes to a JsonObject, which is the same type as the container the path walk builds, so the walk descended into the map and merged the deeper path into it. Well-formed input, silent wrong answer, in both shapes. This predates #529.

Why there is no fix that returns the right data

For a non-Nullable overlap the driver cannot tell whether a = 0 means "stored zero" or "path absent" — the server collapses both into the same bytes. For the Map overlap the two values are simply distinct and a JsonObject has one slot. No rule recovers the truth, so the choice is between a silent wrong answer and a loud failure. This picks loud.

The rule

A row throws a SerializationException when both sides of the overlap hold a value:

  • parent walk — an intermediate part holding a scalar, an array, or an object value (a Map, or a Dynamic holding an object);
  • leaf — an incoming value landing on a subtree which is not all-null.

A side which holds nothing gives way rather than colliding — this keeps every representable row working:

  • an incoming null landing on a subtree keeps the subtree (Fix JSON reads dropping a typed path whose value is NULL #529's guard, unchanged);
  • an incoming empty or all-null object does the same;
  • an incoming value landing on an all-null subtree replaces it, so {"a":5} under Nullable overlapping paths still reads {"a":5};
  • an empty object value in the parent walk is replaced by the container, so JSON(a Map(String, Int64), `a.b` Int64) — where every key is routed to a path of its own and the map is always empty — reads normally.

The rule is order-independent: whichever of the two paths the wire puts first, a row where both hold a value throws, and a row where one holds nothing reads the other.

Object values are told apart from walk-built containers by tracking them by reference for the row being read. That set is allocated only where an object-valued path exists, which a column of plain scalar paths never has. The all-null scan likewise runs only on a collision. No hot-path cost.

The message names both paths and points at JsonReadMode.String, which returns the server's JSON text unchanged, duplicate key included.

The parent walk previously cast the occupant to JsonObject, so a scalar occupant already surfaced as InvalidCastException for two overlapping dynamic paths. That cast is now the explicit throw.

Opting out: AllowDuplicateJsonKeys

Throwing is a behavior change for anyone whose column has such an overlap and who has been living with the wrong answer. The new AllowDuplicateJsonKeys setting (default false) keeps the column readable as a JsonObject: the driver keeps whichever of the two values the row carries last and drops the other, which is the previous resolution.

var settings = new ClickHouseClientSettings("Host=localhost") { AllowDuplicateJsonKeys = true };
// or "Host=localhost;AllowDuplicateJsonKeys=true"

The expected values were taken from the driver's own output before this change rather than written by hand, so the setting is verified to restore the prior resolution rather than some new one:

column row before default AllowDuplicateJsonKeys
JSON(a Int64, a.b Int64) {"a.b":7} {"a":0} throws {"a":0}
JSON(a Int64, a.b Int64) {"a":5} {"a":5} throws {"a":5}
JSON(a Int64, a.b.c Int64) {"a.b.c":7} {"a":0} throws {"a":0}
JSON(a Nullable(Int64), a.b Nullable(Int64)) duplicate-key doc {"a":5} throws {"a":5}
JSON(a Map(String, Int64)) {"a":{"x":1},"a.b":7} {"a":{"x":1,"b":7}} throws {"a":{"x":1,"b":7}}
JSON(a Map(String, Int64)) {"a":{"b":1},"a.b":7} {"a":{"b":7}} throws {"a":{"b":7}}
JSON duplicate-key doc InvalidCastException throws throws
JSON(a Array(Int64)) {"a":[1,2],"a.b":7} InvalidCastException throws throws

The last two are the limit of the setting: where the parent holds a scalar or an array there is nowhere to place a subtree, so the row stays unreadable. Both already threw, and now throw a SerializationException naming the paths rather than a bare InvalidCastException.

The setting only affects JsonReadMode.Binary and JsonReadMode.None. JsonReadMode.String returns the server's text and is unaffected. It also costs nothing when set: only the throw consults the object-value reference set, so that set is never allocated.

Tests

ClickHouse.Driver.Tests/Types/JsonTypeTests.cs, integration against a real server:

  • Read_WithOverlappingPathsHoldingValues_ShouldThrowUnlessDuplicateKeysAreAllowed — 8 shapes × both values of the setting = 16 cases, from one OverlappingPathShapes() source that carries the expected document per shape (null = still throws). The shapes: non-Nullable overlap with the value in the leaf, in the subtree, and two levels down; a Nullable overlap and two dynamic paths fed a duplicate-key document; a Map path overlapped by a deeper path with a disjoint key and with the same key; an Array path overlapped by a deeper path. The throw branch asserts both path names and JsonReadMode.String are in the message; the read branch asserts the exact document.
  • Read_WithOverlappingEmptyObjectValue_ShouldKeepTheSubtree — an empty map does not collide.
  • Read_WithOverlappingPathsHoldingValues_ShouldBeReadableAsString — the escape hatch works.
  • Read_WithNestedPathsThatDoNotOverlap_ShouldReadEveryPath — a nested, non-overlapping schema does not false-positive.
  • Two new cases in TypedPathNullValueTestCases pinning the all-null-subtree replacement, one of them two levels deep so both branches of the recursive scan run.

Every new-behavior test fails on main — with the wrong value, or with InvalidCastException.

The overlap fix was run against ClickHouse 25.8 and 26.7: full suite green on both (10903 / 10788 passed, 0 failed). The AllowDuplicateJsonKeys commit was added later and has been verified on 26.7 only (10911 passed, 0 failed); its expected documents are exact strings, so if 25.8 orders overlapping paths differently on the wire that will surface as a string mismatch in the new cases.

Two documents only insert on 26.3+, so those cases are version-gated; the rest use a flattened {"a.b": 7} key, which stores identically on every server in the matrix.

Notes for the maintainer

  • Validate fragments will fail. The changelog entry is written straight into the Unreleased section of CHANGELOG.md rather than as a changelog.d fragment, at the requester's instruction. Please fold it into a fragment, or accept it as-is.
  • This is a behavior change: rows which silently returned wrong data now throw. Marked Breaking Change in the changelog entry, which also documents AllowDuplicateJsonKeys as the way back.
  • This adds public API (it did not before AllowDuplicateJsonKeys was added): ClickHouseClientSettings.AllowDuplicateJsonKeys, ClickHouseConnectionStringBuilder.AllowDuplicateJsonKeys, ClickHouseDefaults.AllowDuplicateJsonKeys, and the matching five lines in PublicAPI.Unshipped.txt.
  • Add JsonNullPathMode to opt out of typed-path nulls #582 is stacked on this branch and has been rebased onto it. The two settings compose orthogonally — verified together on all the shapes above plus the null-path shapes Add JsonNullPathMode to opt out of typed-path nulls #582 covers.
  • Still backportable on its own.

🤖 Generated with Claude Code

ClickHouse accepts a JSON column which declares a path both as a value and
as the parent of another path, for example `JSON(a Int64, a.b Int64)`. Both
paths are typed, so both are present in every row, and the server renders
the row with a duplicate key: `{"a":0,"a":{"b":7}}`.

A JsonObject cannot hold two values for one key. The wire puts `a.b` before
`a`, so the leaf overwrote the subtree, and reading `{"a":{"b":7}}` back out
of that column returned `{"a":0}`. Well-formed input, silent wrong answer.

With non-Nullable paths the driver cannot tell a stored `0` from an absent
path — the server collapses both — so no rule recovers the real value. The
only choice is between a wrong answer and a clear failure.

ReadAsJsonObject now throws a SerializationException naming the two paths
when both sides of an overlap hold a value: a parent which holds a scalar or
an array, and a leaf value landing on a subtree which is not all-null. The
representable cases are unchanged — a null leaf still keeps the subtree, and
a leaf value still replaces an all-null subtree — so `Nullable` overlaps,
which fill in one side per row, keep reading as before.

The parent walk previously cast the occupant to JsonObject, which surfaced
the same collision as an InvalidCastException for two overlapping dynamic
paths. That cast is now the explicit throw.

The all-null scan runs only on a collision, so a non-overlapping schema
never pays for it.
Copilot AI lite review requested due to automatic review settings August 23, 2026 18:10
@codecov

codecov Bot commented Aug 23, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the JSON binary read path to throw instead of silently dropping data when ClickHouse returns overlapping JSON paths that cannot be represented in a single System.Text.Json.Nodes.JsonObject (duplicate key with both parent and child holding values). It adds integration tests to pin the behavior and documents the new failure mode and the JsonReadMode.String escape hatch.

Changes:

  • Detect overlapping parent/leaf JSON-path collisions during JsonType.ReadAsJsonObject and throw a SerializationException with an actionable message.
  • Add integration tests covering non-Nullable overlaps (typed + dynamic), the string-mode escape hatch, and non-overlapping nested schemas.
  • Document the overlapping-path behavior in the JSON overview docs.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
docs/overview.mdx Documents overlapping typed paths behavior and recommends JsonReadMode.String as an escape hatch.
ClickHouse.Driver/Types/JsonType.cs Adds explicit overlap detection and an all-null-subtree check to avoid silent data loss.
ClickHouse.Driver.Tests/Types/JsonTypeTests.cs Adds integration tests for overlap collisions, escape hatch, and non-overlapping nested paths.
CHANGELOG.md Adds a changelog entry directly to Unreleased (requires moving to changelog.d fragment).

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread CHANGELOG.md Resolved
Comment thread docs/overview.mdx Resolved
The all-null scan recurses, but every overlap case stopped one level down,
so neither recursive branch ran. `JSON(a Int64, a.b.c Int64)` reaches the
branch which finds a value and has to report `a.b.c` rather than the first
key under `a`; the Nullable counterpart reaches the branch which finds the
whole subtree null and lets the leaf's value replace it.
A Map path, or a Dynamic one holding an object, decodes to a JsonObject —
the same type as the container the path walk builds for a deeper path. The
walk could not tell them apart, so it descended into the value and merged
the deeper path into it. Two values the server sent under one key came back
as one merged object, and where they shared a key the map's own entry was
overwritten and gone: `JSON(a Map(String, Int64))` holding
`{"a":{"b":1},"a.b":7}` read back as `{"a":{"b":7}}`.

Object values are now tracked by reference for the row being read, so the
walk throws on one instead of descending. The set is allocated only where
such a path exists, which a column of plain scalar paths never has.

The leaf now also mirrors the null rule onto the incoming value: an empty
or all-null object holds nothing, so it gives way to the subtree rather
than colliding with it. Without that, `JSON(a Map(String, Int64), `a.b`
Int64)` threw on every row — every key of the document is routed to a path
of its own, so the map is always empty, and no data was at risk.

Both sides of the rule are now order-independent: whichever of the two
paths the wire puts first, a row where both hold a value throws, and a row
where one holds nothing reads the other.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

There are 2 total unresolved issues (including 1 from previous review).

Fix All in Cursor

❌ 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 ef063d4. Configure here.

Comment thread ClickHouse.Driver/Types/JsonType.cs Resolved
polyglotAI-bot and others added 2 commits August 23, 2026 18:57
Three of the new documents only insert on 26.3 and later.

A nested `{"a": {"b": 7}}` is parsed against the overlapping `a Int64` by
25.8 and rejected with INCORRECT_DATA. The flattened `{"a.b": 7}` key
stores identically on every server in the matrix — both render
`{"a":0,"a":{"b":7}}` — so the cases now use it, as
TypedPathNullValueTestCases already does for the same reason.

A duplicate-key document is rejected outright by 25.8 unless duplicated
paths are skipped, which would drop one of the two values and remove the
thing under test. Those two cases are gated at 26.3, the oldest server
which keeps both.

The empty-object case relied on 25.8 and 26.7 routing a nested document to
the same paths, which they do not: 26.7 empties the map and 25.8 fills it.
Leaving the map out of the document empties it on both.
An overlap where both paths hold a value has no faithful JsonObject
representation, so reading such a row throws. AllowDuplicateJsonKeys keeps
the column readable instead: the driver keeps whichever of the two values
the row carries last and drops the other.

A path whose parent holds a scalar or an array still throws, because a
subtree cannot be placed under either.

Co-Authored-By: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants