Skip to content

Reject wrapper types built over unsupported nested types - #578

Open
iliaal wants to merge 1 commit into
ClickHouse:masterfrom
iliaal:fix/factory-null-nested
Open

iliaal wants to merge 1 commit into
ClickHouse:masterfrom
iliaal:fix/factory-null-nested

Conversation

@iliaal

@iliaal iliaal commented Oct 18, 2026

Copy link
Copy Markdown
Contributor

CreateColumnByType("Array(5)"), "Nullable(DateTime64)" and "LowCardinality(Nullable(5))" segfault instead of reporting an unsupported type. A server can send these in a block header.

CreateColumnFromAst in columns/factory.cpp passes the nested result straight to the ColumnArray and ColumnNullable constructors. Both call nested->Type(), so they dereference the nullptr the factory returns for any type it can't build.

Fix: Array and Nullable now return nullptr when the nested column is null, as Tuple and Map already do, so Client reports "unsupported column type". LowCardinality(Nullable(T)) throws UnimplementedError, matching the generic LowCardinality branch.

Reproducer type strings: Array(Null), Array(5), Array(DateTime64), Array(Time64), Nullable(5), Nullable(DateTime64), Array(Nullable(5)), LowCardinality(Nullable(5)).

Test: CreateColumnByType.UnsupportedNestedType segfaults on master and passes here. It also checks that Nullable(UInt8), Array(String), LowCardinality(Nullable(String)) and Array(Nullable(DateTime64(3))) still round-trip their type names.

This doesn't overlap #575; the two merge cleanly.

CreateColumnByType("Array(5)"), "Nullable(DateTime64)" or
"LowCardinality(Nullable(5))" crashed instead of reporting an
unsupported type. CreateColumnFromAst in columns/factory.cpp passed the
nested result straight to the ColumnArray and ColumnNullable
constructors, which call nested->Type() and dereference the null
pointer the factory returns for types it cannot create. Array and
Nullable now return nullptr like Tuple and Map already do, so Client
reports "unsupported column type", and LowCardinality(Nullable(T))
throws UnimplementedError like the generic LowCardinality branch.
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.

1 participant