Repository navigation
Conversation
Hakaou
force-pushed
the
unknown-type-names-are-rejected
branch
3 times, most recently
from
October 20, 2026 19:49
0a53f99 to
f463773
Compare
The type parser had no entry for Nested, so a column typed Array(Nested(key String, value String)) was created as Array(Void) and the native block stream fell out of sync: an empty server exception without compression, "some data was not read" with LZ4. A top-level Nested column, which the server sends with flatten_nested = 0, was rejected as unsupported. The server defines Nested as a custom name over Array(Tuple(...)) (DataTypeNested.cpp). The parser now recognises Nested as its own node, like Map and SimpleAggregateFunction, and the column factory builds it in the Tuple case, wrapped in an Array. Sharing that case keeps the AST read by reference, so a deeply nested type is not copied at every level. Closes ClickHouse#571
ValidateAST only looked at the root of a type name. Inside Array, Nullable, Tuple, Map, LowCardinality or Nested, a name the parser does not know became a Void terminal, the column was created as ColumnNothing, and the block stream was read one byte per row with no error naming the type: without compression, SELECT [CAST(1, 'Int256')] returned a row of garbage. The elements of those containers are now validated too. The arguments of every other type, such as the settings and typed paths of JSON or the function name of SimpleAggregateFunction, are not types and are left alone. Nothing, which the server sends for SELECT [] and SELECT NULL, is registered as the type it already was by accident, so those queries keep working. A client now gets "unsupported column type: Array(Int256)" instead of wrong data or an unrelated error.
Hakaou
force-pushed
the
unknown-type-names-are-rejected
branch
from
October 21, 2026 21:52
f463773 to
718119a
Compare
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.
Closes #589, the second point of the suggested fix in #571. Draft until #575 lands: this branch is stacked on it because it needs the
Nestednode, so the first commit here is #575's and only the last one belongs to this PR.Summary
ValidateASTonly looked at the root of a type name. InsideArray,Nullable,Tuple,Map,LowCardinalityorNested, a name the parser does not know became aVoidterminal, the column was created asColumnNothing, and the block was read one byte per row, with no error naming the type.CreateColumnByType("Array(Int256)")returned a column typedArray(Void), andSELECT [CAST(1, 'Int256')]returned a row of wrong data without compression and failed withsome data was not readwith LZ4.Behavior changes
unsupported column type: Array(Int256)instead of wrong data or an unrelated error, the same error it already gets forInt256at the root. Other names this applies to, all seen intoTypeNameon 26.11:Variant,Dynamic,BFloat16,UInt256, theInterval*types,LineString,MultiPoint,QBit,AggregateFunction.JSON(max_dynamic_paths=1, a Int256, SKIP b)keep decoding as before, and the function name ofSimpleAggregateFunction(anyLast, LowCardinality(String))is not a type.Nothing, which the server sends forSELECT []andSELECT NULL, is registered as the type it already was by accident, so those queries keep working.Tests
type_parser_ut: a parametrised test with a fresh AST per case, six containers holding an unknown name rejected,Nothing,SimpleAggregateFunction, parameterisedJSON,Enum8andDateTime64accepted.CreateColumnByType_ut:Array(Int256)andArray(Nested(key String, value Dynamic))givenullptr,Array(Nothing),Nullable(Nothing)and parameterisedJSONgive a column.client_ut, both fixtures:SELECT [] AS empty, NULL AS null_value, [NULL] AS nullsdecodes,SELECT [CAST(1, 'Int256')]throwsUnimplementedError, andCAST('{"a":1}', 'JSON(max_dynamic_paths=1)')reads back{"a":1}withoutput_format_native_write_json_as_string = 1.Full suite green locally (macOS arm64, Apple clang 21) against ClickHouse 26.11.1 master build, except the
Remote*tests, which need external TLS endpoints, andSocketcase.connecttimeout, not run. The new tests also pass against 25.12 and 26.3. Parser, factory, types and client suites pass under AddressSanitizer and UBSan, UBSan reporting only the out-of-bounds reads inInsertDatathat #576 fixes. The branch also builds without warnings with GCC 14 on Linux aarch64, where the server-free suite passes.Open question
Inside a container, should an unknown name make
CreateColumnByTypereturnnullptr, as it does at the root, or would you rather have an exception naming the inner type? With an exception, the parser and factory tests above would expect it instead offalseandnullptr, and the client test, which expectsUnimplementedError, would hold if that is the type thrown.