(superset) 01/01: test(clickhouse): cover GROUP BY ALL against a real instance (#40482)

[ru...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

rusackas pushed a commit to branch test/clickhouse-group-by-all-40482
in repository https://gitbox.apache.org/repos/asf/superset.git

commit 90215cd68a8413ae32752693fbb0eb323d6ae45f
Author: Evan Rusackas <ev...@rusackas.com>
AuthorDate: Thu Oct 1 18:19:14 2026 -0700

    test(clickhouse): cover GROUP BY ALL against a real instance (#40482)
    
    #40482 reports "GROUP BY ALL" not working in SQL Lab against ClickHouse,
    with no error text or repro steps attached, and has sat without
    confirmation since. Superset's own AST-based query mutation already
    round-trips this syntax cleanly (sqlglot parses ClickHouse's GROUP BY ALL
    as Group(all=True), and SQLStatement.set_limit_value operates on that
    AST node directly, so LIMIT injection is unaffected by the grouping
    shorthand) -- verified directly, not just by inspection.
    
    The one layer that can't be verified by reading code is the real
    driver/server combination Superset actually ships
    (clickhouse-connect over HTTP), so this adds a testcontainers
    regression executing GROUP BY ALL against a live ClickHouse instance
    and asserting the grouped/summed results are correct. A green run here
    is strong evidence the reported issue is stale; a red one would
    isolate the bug to the driver/server layer specifically.
    
    Co-Authored-By: Evan Rusackas <ev...@preset.io>
    Co-Authored-By: Claude Sonnet 5 <no...@anthropic.com>
---
 .../db_engine_specs/test_clickhouse.py             | 43 ++++++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/tests/testcontainers/db_engine_specs/test_clickhouse.py b/tests/testcontainers/db_engine_specs/test_clickhouse.py
index de574d1c08d..d66d800114a 100644
--- a/tests/testcontainers/db_engine_specs/test_clickhouse.py
+++ b/tests/testcontainers/db_engine_specs/test_clickhouse.py
@@ -47,10 +47,13 @@ import pytest
 from sqlalchemy import (
     Column,
     create_engine,
+    insert,
     inspect,
     Integer,
     MetaData,
+    String,
     Table as SATable,
+    text,
 )
 from sqlalchemy.engine import Engine
 
@@ -128,3 +131,43 @@ def test_get_columns_maps_native_types(engine: Engine) -> None:
         assert spec is not None
         assert spec.generic_type == GenericDataType.NUMERIC
         assert isinstance(spec.sqla_type, Integer)
+
+
+def test_group_by_all_groups_correctly(engine: Engine) -> None:
+    """
+    ClickHouse's `GROUP BY ALL` shorthand (group by every non-aggregated
+    SELECT column) executed for real over clickhouse-connect's HTTP driver,
+    the one Superset actually ships (apache/superset#40482 reported it
+    failing in SQL Lab, with no error text or repro steps attached).
+    Superset's own AST-based query mutation (sqlglot parse + LIMIT
+    injection) already round-trips this syntax cleanly -- this covers the
+    one layer that can't: the real driver/server actually executing it.
+    """
+    metadata = MetaData()
+    t = SATable(
+        "pilot_group_by_all",
+        metadata,
+        Column("id", Integer, primary_key=True, autoincrement=False),
+        Column("category", String(16)),
+        Column("amount", Integer),
+        MergeTree(order_by="id"),
+    )
+    metadata.create_all(engine)
+    with engine.begin() as conn:
+        conn.execute(
+            insert(t),
+            [
+                {"id": 1, "category": "a", "amount": 10},
+                {"id": 2, "category": "a", "amount": 5},
+                {"id": 3, "category": "b", "amount": 7},
+            ],
+        )
+
+        rows = conn.execute(
+            text(
+                "SELECT category, sum(amount) AS total "
+                "FROM pilot_group_by_all GROUP BY ALL ORDER BY category"
+            )
+        ).fetchall()
+
+    assert [tuple(row) for row in rows] == [("a", 15), ("b", 7)]