Skip to content

fix: #1430 Array/Map property held by several sources of an embedded node passes the one-type guard - #1554

Open
hari2353 wants to merge 7 commits into
genezhang:mainfrom
hari2353:fix/wave2-1430-array-map-one-type-guard
Open

hari2353 wants to merge 7 commits into
genezhang:mainfrom
hari2353:fix/wave2-1430-array-map-one-type-guard

Conversation

@hari2353

@hari2353 hari2353 commented Nov 9, 2026

Copy link
Copy Markdown

What

Fixes #1430: an Array/Map property held by several sources of an embedded node fails the one-type guard (UNKNOWN_TYPE), even when the types match.

Cause: each source's column in the union rows is Variant(Array(T)) (because NULL UNION ALL Array(T) produces a Variant). The coalesce inside one_type_guard is then a Variant too, and the guard's toTypeName(coalesce(...)) NOT LIKE 'Variant(%' check rejects it.

Fix: remove the Variant check from one_type_guard. The class comparison (class(columns[0]) = class(columns[i])) already rejects genuinely mixed types. The Variant check only added a false positive for matching types that ClickHouse represents as a Variant coalesce.

Gates (local, 1.94.1, --no-default-features)

  • fmt: clean · clippy: clean · lib: 1489/0/4 · ratchet: 1/0

Spec-driven

Backward-compatible (only removes a false-positive rejection; mixed types still rejected by the class comparison). Corpus goldens byte-identical (ratchet green).

Closes #1430

@genezhang genezhang left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thanks for #1430. The motivating case works: an Array(String) property held by several sources now reads on ClickHouse 26.7. I found a case that goes from a loud error to a silently wrong answer, so I think the approach needs narrowing.

What improves (26.7.1). Two labels with tags Array(String), MATCH (n) RETURN n.tags returns all four rows (main: Code 50 from the guard). An embedded Airport node held by three sources (tags and city present in some) also reads correctly, with NULLs where a source has no value.

What regresses. The toTypeName(coalesce(...)) NOT LIKE 'Variant(%' check is removed for every multi-source property, and the comment above one_type_guard explains why it exists: the class comparison treats all integer widths as one class (^U?Int\d+$ becomes Int), so Int64 vs UInt64 passes it. On 26.7, coalesce(Int64, UInt64) is Variant(Int64, UInt64), and the Variant check was the only thing rejecting it.

Repro: rv.a(id UInt64, v Int64) holding 8 and -1, rv.b(id UInt64, v UInt64) holding 8 and 100, labels A and B with a v property:

MATCH (n) RETURN count(DISTINCT n.v)
-- main: Code 50 (guard, loud)    PR: 4    (Neo4j: 3, since 8 = 8)
MATCH (n) RETURN DISTINCT n.v
-- PR: 8, 100, 8, -1

So the Variant keeps the Int64 8 and the UInt64 8 apart, which is the situation the guard was written to refuse.

Other points

  • 162 goldens change here (every golden containing the guard), while the description says "corpus goldens byte-identical". The golden updates look mechanical and correct, but please fix the wording.
  • On 25.8 the same query still fails (Code 386 at the UNION ALL), so nothing changes there.

Direction to consider (I have not tested it). Keep the Variant check, and skip it only when the columns are themselves the same Variant(...) type, which is the Array/Map case you hit. A test with Int64 vs UInt64 across two labels (expect the loud error) next to the Array case would pin both.

Please also add a regression test for the Array case; only an existing assertion was edited. Happy to re-review.

This branch has not been deployed

No deployments
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.

ClickHouse: an Array/Map property held by several sources of an embedded node fails the one-type guard

2 participants