Repository navigation
Conversation
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 ("Can't cast from Void",
"some data was not read"). 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 as its Tuple wrapped in an Array.
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 without any error: 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.
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 without any error.CreateColumnByType("Array(Int256)")returned a column typedArray(Void), andSELECT [CAST(1, 'Int256')]returned a row of wrong data.Behavior changes
unsupported column type: Array(Int256)instead of wrong data, 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 unknown names inside containers 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, the new tests also against 25.12 and 26.3. Parser, factory, types and client suites pass under AddressSanitizer and UBSan.
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? The tests hold either way, only the end ofValidateASTwould change.