diff --git a/clickhouse/columns/factory.cpp b/clickhouse/columns/factory.cpp index 2fb0d39f..6c031bac 100644 --- a/clickhouse/columns/factory.cpp +++ b/clickhouse/columns/factory.cpp @@ -150,16 +150,17 @@ static ColumnRef CreateTerminalColumn(const TypeAst& ast) { static ColumnRef CreateColumnFromAst(const TypeAst& ast, CreateColumnByTypeSettings settings) { switch (ast.meta) { case TypeAst::Array: { - return std::make_shared( - CreateColumnFromAst(GetASTChildElement(ast, 0), settings) - ); + if (auto nested = CreateColumnFromAst(GetASTChildElement(ast, 0), settings)) { + return std::make_shared(nested); + } + return nullptr; } case TypeAst::Nullable: { - return std::make_shared( - CreateColumnFromAst(GetASTChildElement(ast, 0), settings), - std::make_shared() - ); + if (auto nested = CreateColumnFromAst(GetASTChildElement(ast, 0), settings)) { + return std::make_shared(nested, std::make_shared()); + } + return nullptr; } case TypeAst::Terminal: { @@ -252,12 +253,11 @@ static ColumnRef CreateColumnFromAst(const TypeAst& ast, CreateColumnByTypeSetti // item and would omit the null item, producing an incorrect nullable // dictionary. // 2. It lets us construct the ColumnNullable with an explicit UInt8 null-map. - return std::make_shared( - std::make_shared( - CreateColumnFromAst(GetASTChildElement(nested, 0), settings), - std::make_shared() - ) - ); + if (auto inner = CreateColumnFromAst(GetASTChildElement(nested, 0), settings)) { + return std::make_shared( + std::make_shared(inner, std::make_shared())); + } + throw UnimplementedError("LowCardinality(Nullable(" + GetASTChildElement(nested, 0).name + ")) is not supported"); default: { // Generic LowCardinality(T): build the inner column and // wrap it. Works for any fixed-size dictionary type that diff --git a/ut/CreateColumnByType_ut.cpp b/ut/CreateColumnByType_ut.cpp index 0f1fbc4f..daac35ad 100644 --- a/ut/CreateColumnByType_ut.cpp +++ b/ut/CreateColumnByType_ut.cpp @@ -64,6 +64,35 @@ TEST(CreateColumnByType, UnmatchedBrackets) { ASSERT_EQ(nullptr, CreateColumnByType("Array(LowCardinality(Nullable(FixedString(10000)))")); } +TEST(CreateColumnByType, UnsupportedNestedType) { + // A wrapper over a type the factory cannot create must not be built around a null nested column. + for (const char* type_name : { + "Nullable(5)", + "Nullable(DateTime64)", + "Array(Nullable(5))", + "Array(Null)", + "Array(DateTime64)", + "Array(5)", + "Array(Time64)", + }) { + EXPECT_EQ(nullptr, CreateColumnByType(type_name)) << type_name; + } + + EXPECT_THROW(CreateColumnByType("LowCardinality(Nullable(5))"), UnimplementedError); + EXPECT_THROW(CreateColumnByType("LowCardinality(Nullable(DateTime64))"), UnimplementedError); + + for (const char* type_name : { + "Nullable(UInt8)", + "Array(String)", + "LowCardinality(Nullable(String))", + "Array(Nullable(DateTime64(3)))", + }) { + const auto col = CreateColumnByType(type_name); + ASSERT_NE(nullptr, col) << type_name; + EXPECT_EQ(type_name, col->GetType().GetName()); + } +} + TEST(CreateColumnByType, LowCardinalityAsWrappedColumn) { CreateColumnByTypeSettings create_column_settings; create_column_settings.low_cardinality_as_wrapped_column = true;