Skip to content

[backfill: ClickHouse/clickhouse-cpp] CreateColumnByType fails for SimpleAggregateFunction with non-terminal inner type (e.g. LowCardinality) #540

Description

@alex-clickhouse

Target client repo: ClickHouse/clickhouse-cpp

Severity: sev:2 — Visible failure (column factory returns nullptr, leading to an error when the client attempts to read a block containing the column). The affected type — SimpleAggregateFunction(..., LowCardinality(String)) — is a real combination used in AggregatingMergeTree materialized views with low-cardinality string state. No client-side workaround short of changing the table schema or stripping LowCardinality from the column type. Not silent corruption (column creation fails outright), so not sev:3; but more than a trivial flag-flip, so not sev:1.

Description

SimpleAggregateFunction(func, T) columns are handled in the column factory by recursing only into terminal types. When T is a non-terminal type — e.g. LowCardinality(String), Nullable(String), Array(...), Tuple(...), Map(...), etc. — column creation returns nullptr, which blocks the client from constructing a column for that field.

In clickhouse/columns/factory.cpp:

case TypeAst::SimpleAggregateFunction: {
    return CreateTerminalColumn(GetASTChildElement(ast, -1));
}

CreateTerminalColumn switches on ast.code only for terminal codes (UInt*/String/DateTime/Decimal/...). For an inner type whose AST meta is TypeAst::LowCardinality (set in clickhouse/types/type_parser.cpp:118-120), the switch falls through to default: return nullptr;.

Compare with the sibling Nullable/Array/LowCardinality cases in the same CreateColumnFromAst function, which all recurse through CreateColumnFromAst (not CreateTerminalColumn) on their child element. SimpleAggregateFunction is the only wrapper that recurses only via CreateTerminalColumn.

This is the same root cause class as the source bug filed against clickhouse-connect: the inner type of SimpleAggregateFunction is treated as restricted to simple/terminal types rather than as any column type.

ClickHouse server version

Verified against ClickHouse server 26.4.2.10 (locally running). Investigation is code analysis of target-repo at main; not run end-to-end against a live server.

Reproduction

Server-side setup (same as the source issue):

CREATE TABLE default.simple_agg
(
    id UInt32,
    str SimpleAggregateFunction(any, String),
    lc_str SimpleAggregateFunction(any, LowCardinality(String))
) ENGINE = AggregatingMergeTree() ORDER BY id;
INSERT INTO default.simple_agg VALUES (1, 'str', 'lc_str');

Client-side, the bug is visible directly via the column factory used during block deserialization:

#include <clickhouse/columns/factory.h>
#include <cassert>
#include <iostream>

int main() {
    using namespace clickhouse;

    // Works:
    auto ok = CreateColumnByType("SimpleAggregateFunction(any, String)");
    std::cout << "String inner: " << (ok ? "OK" : "nullptr") << "\n";

    // Bug: returns nullptr instead of a LowCardinality<String> column.
    auto bad = CreateColumnByType("SimpleAggregateFunction(any, LowCardinality(String))");
    std::cout << "LowCardinality(String) inner: " << (bad ? "OK" : "nullptr") << "\n";

    assert(ok != nullptr);
    assert(bad != nullptr); // FAILS — bad is nullptr
}

Expected: both calls return a non-null ColumnRef (an ordinary ColumnString and a LowCardinalityT<ColumnString> respectively).

Actual: the second call returns nullptr. During a real Client::Select, this would cause column construction to fail (e.g., UnimplementedError or a null-deref further up the stack when the block is being built).

The same failure mode would also affect SimpleAggregateFunction(..., Nullable(String)), SimpleAggregateFunction(..., Array(...)), etc.

Suggested fix

In clickhouse/columns/factory.cpp (~line 251), replace the terminal-only recursion with the general one so any inner type — LowCardinality, Nullable, Array, Tuple, Map, etc. — is handled the same way it would be at the top level:

case TypeAst::SimpleAggregateFunction: {
    return CreateColumnFromAst(GetASTChildElement(ast, -1), settings);
}

(Do not attempt the fix in this issue — investigation only.)

Source bug

Relayed from ClickHouse/clickhouse-connect#182 (originally reported against clickhouse-connect 0.5.20 / Python). The clickhouse-cpp manifestation is structurally the same — SimpleAggregateFunction's inner-type recursion assumes a terminal type — but surfaces as nullptr from CreateColumnByType rather than a Python parsing exception.

Metadata

Metadata

Labels

No labels
No labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions