Fix CreateColumnByType for SimpleAggregateFunction with non-terminal inner type

CreateColumnByType returned nullptr for a SimpleAggregateFunction whose
value type is a non-terminal wrapper such as LowCardinality, Nullable,
Array, Tuple, Map or Enum (e.g. SimpleAggregateFunction(anyLast,
LowCardinality(String))), which made reading any block containing such a
column fail with "unsupported column type".

The factory handled the SimpleAggregateFunction meta by calling
CreateTerminalColumn on the inner type, but that only covers terminal type
codes and falls through to nullptr for wrapper metas. Every other wrapper
(Array, Nullable, Tuple, Map, LowCardinality) recurses through
CreateColumnFromAst; SimpleAggregateFunction was the only one restricted to
terminals. Recurse through CreateColumnFromAst instead (passing settings so
low_cardinality_as_wrapped_column is honored). A SimpleAggregateFunction is
transparent on the wire, so the created column matches its inner type.

Fixes: #540

Author: polyglotAI-bot

clickhouse/columns/factory.cpp

@@ -252,7 +252,7 @@ static ColumnRef CreateColumnFromAst(const TypeAst& ast, CreateColumnByTypeSetti
             }
         }
         case TypeAst::SimpleAggregateFunction: {
-            return CreateTerminalColumn(GetASTChildElement(ast, -1));
+            return CreateColumnFromAst(GetASTChildElement(ast, -1), settings);
         }
 
         case TypeAst::Map: {

ut/CreateColumnByType_ut.cpp

@@ -8,6 +8,8 @@
 
 #include <gtest/gtest.h>
 
+#include <tuple>
+
 namespace {
 using namespace clickhouse;
 }
@@ -20,6 +22,33 @@ TEST(CreateColumnByType, CreateSimpleAggregateFunction) {
     ASSERT_NE(nullptr, col->As<ColumnInt32>());
 }
 
+// SimpleAggregateFunction is transparent on the wire: the created column must
+// match its value (inner) type. The inner type may itself be a wrapper such as
+// LowCardinality, Nullable, Array or Map, which previously produced a nullptr
+// because only terminal inner types were handled (issue #540).
+class CreateColumnBySimpleAggregateFunctionType
+    : public ::testing::TestWithParam<std::tuple<const char* /*type*/, const char* /*expected inner type name*/>>
+{};
+
+TEST_P(CreateColumnBySimpleAggregateFunctionType, CreateColumnByType) {
+    const auto & [type_name, expected_inner_name] = GetParam();
+    const auto col = CreateColumnByType(type_name);
+    ASSERT_NE(nullptr, col) << "CreateColumnByType returned nullptr for " << type_name;
+    EXPECT_EQ(expected_inner_name, col->GetType().GetName());
+}
+
+INSTANTIATE_TEST_SUITE_P(InnerType, CreateColumnBySimpleAggregateFunctionType, ::testing::Values(
+    // Terminal inner type — handled before the fix; must stay unchanged.
+    std::make_tuple("SimpleAggregateFunction(sum, UInt64)", "UInt64"),
+    // Non-terminal (wrapper) inner types — returned nullptr before the fix.
+    std::make_tuple("SimpleAggregateFunction(anyLast, LowCardinality(String))", "LowCardinality(String)"),
+    std::make_tuple("SimpleAggregateFunction(anyLast, Nullable(String))", "Nullable(String)"),
+    std::make_tuple("SimpleAggregateFunction(groupArrayArray, Array(UInt64))", "Array(UInt64)"),
+    std::make_tuple("SimpleAggregateFunction(sumMap, Map(String, UInt64))", "Map(String, UInt64)"),
+    std::make_tuple("SimpleAggregateFunction(anyLast, Enum8('a' = 1, 'b' = 2))", "Enum8('a' = 1, 'b' = 2)"),
+    std::make_tuple("SimpleAggregateFunction(anyLast, Tuple(UInt64, String))", "Tuple(UInt64, String)")
+));
+
 TEST(CreateColumnByType, UnmatchedBrackets) {
     // When type string has unmatched brackets, CreateColumnByType must return nullptr.
     ASSERT_EQ(nullptr, CreateColumnByType("FixedString(10"));

ut/client_ut.cpp

@@ -728,6 +728,44 @@ TEST_P(ClientCase, SimpleAggregateFunction) {
     EXPECT_EQ(EXPECTED_ROWS, total_rows);
 }
 
+TEST_P(ClientCase, SimpleAggregateFunctionLowCardinality) {
+    const auto & server_info = client_->GetServerInfo();
+    if (versionNumber(server_info) < versionNumber(19, 9)) {
+        GTEST_SKIP() << "Test is skipped since server '" << server_info << "' does not support SimpleAggregateFunction" << std::endl;
+    }
+
+    // A SimpleAggregateFunction column whose value type is a non-terminal
+    // wrapper (here LowCardinality(String)) must be readable: the column
+    // factory previously returned nullptr for such a type, so reading any
+    // block that contained it failed (#540).
+    client_->Execute("DROP TEMPORARY TABLE IF EXISTS test_clickhouse_cpp_saf_lc");
+    client_->Execute(
+            "CREATE TEMPORARY TABLE IF NOT EXISTS test_clickhouse_cpp_saf_lc "
+            "(saf SimpleAggregateFunction(anyLast, LowCardinality(String)))");
+
+    const std::vector<std::string> data{"foo", "bar", "foo", "baz"};
+    client_->Execute(
+            "INSERT INTO test_clickhouse_cpp_saf_lc (saf) VALUES ('foo'),('bar'),('foo'),('baz')");
+
+    size_t total_rows = 0;
+    client_->Select("SELECT saf FROM test_clickhouse_cpp_saf_lc", [&total_rows, &data](const Block & block) {
+        if (block.GetRowCount() == 0)
+            return;
+
+        total_rows += block.GetRowCount();
+        ASSERT_EQ(1U, block.GetColumnCount());
+
+        auto col = block[0]->As<ColumnLowCardinalityT<ColumnString>>();
+        ASSERT_NE(nullptr, col);
+        ASSERT_EQ(data.size(), col->Size());
+        for (size_t r = 0; r < col->Size(); ++r) {
+            EXPECT_EQ(data[r], (*col)[r]) << " at index: " << r;
+        }
+    });
+
+    EXPECT_EQ(data.size(), total_rows);
+}
+
 TEST_P(ClientCase, Cancellable) {
     /// Create a table.
     client_->Execute(