From b414b1851fb32f170a121934f61a7af7e43eae95 Mon Sep 17 00:00:00 2001 From: Hakaou Date: Sun, 18 Oct 2026 01:37:11 +0200 Subject: [PATCH 1/2] Create Nested(...) columns as Array(Tuple(...)) 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 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 #571 --- README.md | 1 + clickhouse/columns/factory.cpp | 11 +-- clickhouse/types/type_parser.cpp | 4 ++ clickhouse/types/type_parser.h | 3 +- docs/index.mdx | 1 + ut/CreateColumnByType_ut.cpp | 65 ++++++++++++++++++ ut/client_ut.cpp | 111 +++++++++++++++++++++++++++++++ ut/type_parser_ut.cpp | 26 ++++++++ 8 files changed, 217 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index a3f29a23..7a96e924 100644 --- a/README.md +++ b/README.md @@ -185,6 +185,7 @@ still might break. * String * LowCardinality(String) or LowCardinality(FixedString(N)) * Tuple +* Nested, read and written as Array(Tuple(...)) * UInt8, UInt16, UInt32, UInt64, Int8, Int16, Int32, Int64 * UInt128, Int128 * UUID diff --git a/clickhouse/columns/factory.cpp b/clickhouse/columns/factory.cpp index 2fb0d39f..eb4c67ae 100644 --- a/clickhouse/columns/factory.cpp +++ b/clickhouse/columns/factory.cpp @@ -166,7 +166,8 @@ static ColumnRef CreateColumnFromAst(const TypeAst& ast, CreateColumnByTypeSetti return CreateTerminalColumn(ast); } - case TypeAst::Tuple: { + case TypeAst::Tuple: + case TypeAst::Nested: { std::vector columns; std::vector names; @@ -185,10 +186,12 @@ static ColumnRef CreateColumnFromAst(const TypeAst& ast, CreateColumnByTypeSetti } } - if (any_named) { - return std::make_shared(columns, std::move(names)); + auto tuple = any_named ? std::make_shared(columns, std::move(names)) + : std::make_shared(columns); + if (ast.meta == TypeAst::Nested) { + return std::make_shared(tuple); } - return std::make_shared(columns); + return tuple; } case TypeAst::Enum: { diff --git a/clickhouse/types/type_parser.cpp b/clickhouse/types/type_parser.cpp index d305a398..84b3df4b 100644 --- a/clickhouse/types/type_parser.cpp +++ b/clickhouse/types/type_parser.cpp @@ -146,6 +146,10 @@ static TypeAst::Meta GetTypeMeta(const StringView& name) { return TypeAst::Map; } + if (name == "Nested") { + return TypeAst::Nested; + } + return TypeAst::Terminal; } diff --git a/clickhouse/types/type_parser.h b/clickhouse/types/type_parser.h index cdef18ee..dc6db8cb 100644 --- a/clickhouse/types/type_parser.h +++ b/clickhouse/types/type_parser.h @@ -22,7 +22,8 @@ struct TypeAst { Enum, LowCardinality, SimpleAggregateFunction, - Map + Map, + Nested }; /// Type's category. diff --git a/docs/index.mdx b/docs/index.mdx index 4460ebc8..eaa2a0fa 100644 --- a/docs/index.mdx +++ b/docs/index.mdx @@ -164,6 +164,7 @@ client.Select( - `Nullable(T)` - `Array(T)` - `Tuple` +- `Nested`, read and written as `Array(Tuple(...))` - `Map` - `IPv4`, `IPv6` - `Point`, `Ring`, `Polygon`, `MultiPolygon` diff --git a/ut/CreateColumnByType_ut.cpp b/ut/CreateColumnByType_ut.cpp index 0f1fbc4f..4abfaf60 100644 --- a/ut/CreateColumnByType_ut.cpp +++ b/ut/CreateColumnByType_ut.cpp @@ -9,12 +9,33 @@ #include +#include +#include +#include #include +#if !defined(_WIN32) +#include +#endif + namespace { using namespace clickhouse; } +#if !defined(_WIN32) +namespace { +long MaxResidentKiB() { + struct rusage usage {}; + getrusage(RUSAGE_SELF, &usage); +#if defined(__APPLE__) + return usage.ru_maxrss / 1024; +#else + return usage.ru_maxrss; +#endif +} +} +#endif + TEST(CreateColumnByType, CreateSimpleAggregateFunction) { auto col = CreateColumnByType("SimpleAggregateFunction(funt, Int32)"); @@ -280,3 +301,47 @@ INSTANTIATE_TEST_SUITE_P(Nested, CreateColumnByTypeWithName, ::testing::Values( "Array(Nullable(LowCardinality(FixedString(10000))))", "Array(Enum8('ONE' = 1, 'TWO' = 2))" )); + +class CreateColumnByNestedType : public ::testing::TestWithParam> {}; + +TEST_P(CreateColumnByNestedType, CreateColumnByType) { + const auto & [type_name, expected_name] = GetParam(); + const auto col = CreateColumnByType(type_name); + ASSERT_NE(nullptr, col) << "CreateColumnByType returned nullptr for " << type_name; + EXPECT_EQ(expected_name, col->GetType().GetName()); +} + +INSTANTIATE_TEST_SUITE_P(Nested, CreateColumnByNestedType, ::testing::Values( + std::make_tuple("Nested(key String, value UInt8)", "Array(Tuple(key String, value UInt8))"), + std::make_tuple("Array(Nested(key String, value UInt8))", "Array(Array(Tuple(key String, value UInt8)))"), + std::make_tuple("Map(String, Nested(key String, value UInt8))", "Map(String, Array(Tuple(key String, value UInt8)))"), + std::make_tuple("Nested(a Nested(b String))", "Array(Tuple(a Array(Tuple(b String))))") +)); + +TEST(CreateColumnByTypeDeathTest, DeepNestedDoesNotCopyAst) { +#if defined(_WIN32) + GTEST_SKIP() << "peak RSS is read with getrusage"; +#else + const size_t depth = 200; + const size_t width = 8000; + std::string type_name; + for (size_t i = 0; i < depth; ++i) { + type_name += "Nested(n "; + } + type_name += "Nested(f0 UInt8"; + for (size_t i = 1; i < width; ++i) { + type_name += ", f" + std::to_string(i) + " UInt8"; + } + type_name += ")"; + type_name.append(depth, ')'); + + const auto create_within_peak_budget = [&type_name] { + const long before = MaxResidentKiB(); + const bool created = CreateColumnByType(type_name) != nullptr; + const long growth = MaxResidentKiB() - before; + std::cerr << "peak RSS grew by " << growth << " KiB"; + std::exit(created && growth < 16 * 1024 ? EXIT_SUCCESS : EXIT_FAILURE); + }; + EXPECT_EXIT(create_within_peak_budget(), ::testing::ExitedWithCode(EXIT_SUCCESS), ""); +#endif +} diff --git a/ut/client_ut.cpp b/ut/client_ut.cpp index 79ee9ca8..aca0ac21 100644 --- a/ut/client_ut.cpp +++ b/ut/client_ut.cpp @@ -1823,6 +1823,117 @@ const auto LocalHostEndpoint = ClientOptions() .SetPassword( getEnvOrDefault("CLICKHOUSE_PASSWORD", "")) .SetDefaultDatabase(getEnvOrDefault("CLICKHOUSE_DB", "default")); +namespace { + +using KeyValuePairs = std::vector>; + +KeyValuePairs KeyValuePairsOf(const ColumnRef& column) { + auto tuples = column->AsStrict(); + auto keys = (*tuples)[0]->AsStrict(); + auto values = (*tuples)[1]->AsStrict(); + KeyValuePairs pairs; + for (size_t i = 0; i < tuples->Size(); ++i) { + pairs.emplace_back(std::string(keys->At(i)), values->At(i)); + } + return pairs; +} + +std::vector ArraysOfKeyValuePairsOf(const ColumnRef& column) { + auto arrays = column->AsStrict(); + std::vector arrays_of_pairs; + for (size_t i = 0; i < arrays->Size(); ++i) { + arrays_of_pairs.push_back(KeyValuePairsOf(arrays->GetAsColumn(i))); + } + return arrays_of_pairs; +} + +} + +TEST_P(ClientCase, ArrayOfNested_SelectServerTypedColumn) { + client_->Execute("DROP TEMPORARY TABLE IF EXISTS test_clickhouse_cpp_array_of_nested"); + client_->Execute("CREATE TEMPORARY TABLE test_clickhouse_cpp_array_of_nested " + "(id UInt8, arrays_of_pairs Array(Nested(key String, value UInt8)))"); + client_->Execute("INSERT INTO test_clickhouse_cpp_array_of_nested VALUES " + "(1, [[('a', 1), ('b', 2)], []]), (2, []), (3, [[('c', 3)]])"); + + std::vector> rows; + std::string type_name; + client_->Select("SELECT id, arrays_of_pairs FROM test_clickhouse_cpp_array_of_nested ORDER BY id", [&](const Block& block) { + if (block.GetRowCount() == 0) { + return; + } + type_name = block[1]->GetType().GetName(); + auto arrays_of_pairs = block[1]->AsStrict(); + for (size_t row = 0; row < block.GetRowCount(); ++row) { + rows.push_back(ArraysOfKeyValuePairsOf(arrays_of_pairs->GetAsColumn(row))); + } + }); + + EXPECT_EQ("Array(Array(Tuple(key String, value UInt8)))", type_name); + const std::vector> expected = { + {{{"a", 1}, {"b", 2}}, {}}, + {}, + {{{"c", 3}}}, + }; + EXPECT_EQ(expected, rows); +} + +TEST_P(ClientCase, ArrayOfNested_ReinsertSelectedRows) { + client_->Execute("DROP TEMPORARY TABLE IF EXISTS test_clickhouse_cpp_array_of_nested_source"); + client_->Execute("DROP TEMPORARY TABLE IF EXISTS test_clickhouse_cpp_array_of_nested_copy"); + client_->Execute("CREATE TEMPORARY TABLE test_clickhouse_cpp_array_of_nested_source " + "(id UInt8, arrays_of_pairs Array(Nested(key String, value UInt8)))"); + client_->Execute("CREATE TEMPORARY TABLE test_clickhouse_cpp_array_of_nested_copy " + "(id UInt8, arrays_of_pairs Array(Nested(key String, value UInt8)))"); + client_->Execute("INSERT INTO test_clickhouse_cpp_array_of_nested_source VALUES (1, [[('a', 1), ('b', 2)], []]), (2, [])"); + + std::vector selected_blocks; + client_->Select("SELECT id, arrays_of_pairs FROM test_clickhouse_cpp_array_of_nested_source ORDER BY id", [&](const Block& block) { + if (block.GetRowCount() > 0) { + selected_blocks.push_back(block); + } + }); + for (const auto& block : selected_blocks) { + client_->Insert("test_clickhouse_cpp_array_of_nested_copy", block); + } + + std::vector rendered_rows; + client_->Select("SELECT toString(arrays_of_pairs) FROM test_clickhouse_cpp_array_of_nested_copy ORDER BY id", + [&](const Block& block) { + for (size_t row = 0; row < block.GetRowCount(); ++row) { + rendered_rows.emplace_back(block[0]->AsStrict()->At(row)); + } + }); + + const std::vector expected = {"[[('a',1),('b',2)],[]]", "[]"}; + EXPECT_EQ(expected, rendered_rows); +} + +TEST_P(ClientCase, Nested_UnflattenedColumn) { + client_->Execute("SET flatten_nested = 0"); + client_->Execute("DROP TEMPORARY TABLE IF EXISTS test_clickhouse_cpp_nested_unflattened"); + client_->Execute("CREATE TEMPORARY TABLE test_clickhouse_cpp_nested_unflattened " + "(id UInt8, pairs Nested(key String, value UInt8))"); + client_->Execute("INSERT INTO test_clickhouse_cpp_nested_unflattened VALUES (1, [('a', 1), ('b', 2)]), (2, [])"); + + std::vector rows; + std::string type_name; + client_->Select("SELECT id, pairs FROM test_clickhouse_cpp_nested_unflattened ORDER BY id", [&](const Block& block) { + if (block.GetRowCount() == 0) { + return; + } + type_name = block[1]->GetType().GetName(); + auto pairs = block[1]->AsStrict(); + for (size_t row = 0; row < block.GetRowCount(); ++row) { + rows.push_back(KeyValuePairsOf(pairs->GetAsColumn(row))); + } + }); + + EXPECT_EQ("Array(Tuple(key String, value UInt8))", type_name); + const std::vector expected = {{{"a", 1}, {"b", 2}}, {}}; + EXPECT_EQ(expected, rows); +} + INSTANTIATE_TEST_SUITE_P( Client, ClientCase, ::testing::Values( diff --git a/ut/type_parser_ut.cpp b/ut/type_parser_ut.cpp index 76bedd7a..d59986b8 100644 --- a/ut/type_parser_ut.cpp +++ b/ut/type_parser_ut.cpp @@ -552,3 +552,29 @@ TEST(TypeParser, AggregateFunction) { EXPECT_FALSE(TypeParser("AggregateFunction(argMax, LowCardinality(Nullable(FixedString(4))), DateTime(3, 'UTC'))").Parse(&ast)); } } + +TEST(TypeParserCase, ParseNested) { + TypeAst ast; + ASSERT_TRUE(TypeParser("Nested(key String, value UInt8)").Parse(&ast)); + ASSERT_EQ(ast.meta, TypeAst::Nested); + ASSERT_EQ(ast.name, "Nested"); + ASSERT_EQ(ast.elements.size(), 2u); + + ASSERT_EQ(ast.elements[0].element_name, "key"); + ASSERT_EQ(ast.elements[0].name, "String"); + ASSERT_EQ(ast.elements[0].code, Type::String); + + ASSERT_EQ(ast.elements[1].element_name, "value"); + ASSERT_EQ(ast.elements[1].name, "UInt8"); + ASSERT_EQ(ast.elements[1].code, Type::UInt8); +} + +TEST(TypeParserCase, ParseNested_FieldNamedNested) { + TypeAst ast; + ASSERT_TRUE(TypeParser("Nested(Nested String)").Parse(&ast)); + ASSERT_EQ(ast.meta, TypeAst::Nested); + ASSERT_EQ(ast.elements.size(), 1u); + ASSERT_EQ(ast.elements[0].meta, TypeAst::Terminal); + ASSERT_EQ(ast.elements[0].element_name, "Nested"); + ASSERT_EQ(ast.elements[0].name, "String"); +} From 0a53f999cdda73783fb5ca34bb6a227d9eb6c900 Mon Sep 17 00:00:00 2001 From: Hakaou Date: Sun, 18 Oct 2026 18:35:11 +0200 Subject: [PATCH 2/2] Reject unknown type names inside containers 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. --- clickhouse/types/type_parser.cpp | 33 ++++++++++++++++++++++++++------ ut/CreateColumnByType_ut.cpp | 15 +++++++++++++++ ut/client_ut.cpp | 30 +++++++++++++++++++++++++++++ ut/type_parser_ut.cpp | 24 +++++++++++++++++++++++ 4 files changed, 96 insertions(+), 6 deletions(-) diff --git a/clickhouse/types/type_parser.cpp b/clickhouse/types/type_parser.cpp index 84b3df4b..1b8b686c 100644 --- a/clickhouse/types/type_parser.cpp +++ b/clickhouse/types/type_parser.cpp @@ -31,6 +31,7 @@ bool TypeAst::operator==(const TypeAst & other) const { static const std::unordered_map kTypeCode = { { "Void", Type::Void }, + { "Nothing", Type::Void }, { "Int8", Type::Int8 }, { "Int16", Type::Int16 }, { "Int32", Type::Int32 }, @@ -153,14 +154,34 @@ static TypeAst::Meta GetTypeMeta(const StringView& name) { return TypeAst::Terminal; } +static bool IsKnownVoidName(const std::string& name) { + return CompateStringsCaseInsensitive(name, std::string_view("void")) == 0 + || CompateStringsCaseInsensitive(name, std::string_view("nothing")) == 0; +} + +static bool ElementsAreTypes(TypeAst::Meta meta) { + return meta == TypeAst::Array || meta == TypeAst::Nullable || meta == TypeAst::Tuple + || meta == TypeAst::Map || meta == TypeAst::LowCardinality || meta == TypeAst::Nested; +} + bool ValidateAST(const TypeAst& ast) { - // Void terminal that is not actually "void" produced when unknown type is encountered. - if (ast.meta == TypeAst::Terminal - && ast.code == Type::Void - && CompateStringsCaseInsensitive(ast.name, std::string_view("void")) != 0) - //throw UnimplementedError("Unsupported type: " + ast.name); + if (ast.name.empty()) { + return true; + } + if (ast.meta == TypeAst::Terminal && ast.code == Type::Void && !IsKnownVoidName(ast.name)) { return false; - + } + if (ast.meta == TypeAst::SimpleAggregateFunction) { + return !ast.elements.empty() && ValidateAST(ast.elements.back()); + } + if (!ElementsAreTypes(ast.meta)) { + return true; + } + for (const auto& element : ast.elements) { + if (!ValidateAST(element)) { + return false; + } + } return true; } diff --git a/ut/CreateColumnByType_ut.cpp b/ut/CreateColumnByType_ut.cpp index 4abfaf60..68e54d94 100644 --- a/ut/CreateColumnByType_ut.cpp +++ b/ut/CreateColumnByType_ut.cpp @@ -345,3 +345,18 @@ TEST(CreateColumnByTypeDeathTest, DeepNestedDoesNotCopyAst) { EXPECT_EXIT(create_within_peak_budget(), ::testing::ExitedWithCode(EXIT_SUCCESS), ""); #endif } + +TEST(CreateColumnByType, UnknownTypeInsideContainer) { + ASSERT_EQ(nullptr, CreateColumnByType("Array(Int256)")); + ASSERT_EQ(nullptr, CreateColumnByType("Array(Nested(key String, value Dynamic))")); +} + +TEST(CreateColumnByType, Nothing) { + ASSERT_NE(nullptr, CreateColumnByType("Array(Nothing)")); + ASSERT_NE(nullptr, CreateColumnByType("Nullable(Nothing)")); +} + +TEST(CreateColumnByType, ParameterisedJSON) { + ASSERT_NE(nullptr, CreateColumnByType("JSON(max_dynamic_paths=1)")); + ASSERT_NE(nullptr, CreateColumnByType("JSON(a Int256, SKIP b)")); +} diff --git a/ut/client_ut.cpp b/ut/client_ut.cpp index aca0ac21..6c0ff554 100644 --- a/ut/client_ut.cpp +++ b/ut/client_ut.cpp @@ -1934,6 +1934,36 @@ TEST_P(ClientCase, Nested_UnflattenedColumn) { EXPECT_EQ(expected, rows); } +TEST_P(ClientCase, EmptyArrayAndNullDecode) { + size_t rows = 0; + client_->Select("SELECT [] AS empty, NULL AS null_value, [NULL] AS nulls", [&](const Block& block) { + rows += block.GetRowCount(); + }); + EXPECT_EQ(1u, rows); +} + +TEST_P(ClientCase, ParameterisedJSONDecodes) { + const auto & server_info = client_->GetServerInfo(); + if (versionNumber(server_info) < versionNumber(25, 3)) { + GTEST_SKIP() << "Test is skipped since server '" << server_info << "' does not support the JSON type" << std::endl; + } + + std::vector documents; + client_->Select("SELECT CAST('{\"a\":1}', 'JSON(max_dynamic_paths=1)') AS document " + "SETTINGS output_format_native_write_json_as_string = 1", [&](const Block& block) { + for (size_t row = 0; row < block.GetRowCount(); ++row) { + documents.emplace_back(block[0]->AsStrict()->At(row)); + } + }); + + const std::vector expected = {"{\"a\":1}"}; + EXPECT_EQ(expected, documents); +} + +TEST_P(ClientCase, UnknownTypeInsideArrayIsReported) { + EXPECT_THROW(client_->Select("SELECT [CAST(1, 'Int256')] AS wide", [](const Block&) {}), UnimplementedError); +} + INSTANTIATE_TEST_SUITE_P( Client, ClientCase, ::testing::Values( diff --git a/ut/type_parser_ut.cpp b/ut/type_parser_ut.cpp index d59986b8..5ddbc250 100644 --- a/ut/type_parser_ut.cpp +++ b/ut/type_parser_ut.cpp @@ -578,3 +578,27 @@ TEST(TypeParserCase, ParseNested_FieldNamedNested) { ASSERT_EQ(ast.elements[0].element_name, "Nested"); ASSERT_EQ(ast.elements[0].name, "String"); } + +class TypeNameValidation : public ::testing::TestWithParam> {}; + +TEST_P(TypeNameValidation, Parse) { + const auto & [type_name, accepted] = GetParam(); + TypeAst ast; + EXPECT_EQ(accepted, TypeParser(std::string(type_name)).Parse(&ast)) << type_name; +} + +INSTANTIATE_TEST_SUITE_P(UnknownNames, TypeNameValidation, ::testing::Values( + std::make_tuple("Array(Int256)", false), + std::make_tuple("Nullable(Variant(UInt8, String))", false), + std::make_tuple("Tuple(a Dynamic)", false), + std::make_tuple("Map(String, IntervalDay)", false), + std::make_tuple("LowCardinality(BFloat16)", false), + std::make_tuple("Nested(key String, value Dynamic)", false), + std::make_tuple("Array(Nothing)", true), + std::make_tuple("Nullable(Nothing)", true), + std::make_tuple("SimpleAggregateFunction(anyLast, LowCardinality(String))", true), + std::make_tuple("JSON(max_dynamic_paths=1)", true), + std::make_tuple("JSON(a Int256, SKIP b)", true), + std::make_tuple("Enum8('a' = 1)", true), + std::make_tuple("DateTime64(3, 'UTC')", true) +));