feat(sql): extract function-name catalog, generalize uniqExact/countIf rewrite (#227)

Pulls the SQL aggregate-function name -> canonical AggIntent mapping out of
frontend-sql::lower_agg_intent's hand-written match and into a new
independent crate, asap-sql-function-catalog (issue #225), structured after
tobilg/polyglot's polyglot-sql-function-catalogs: flat data (name, arity,
canonical semantic), no dependency on the SQL front end or DataFusion.
lower_agg_intent keeps only call-site logic that isn't a function of the
name alone (the DISTINCT rule, the bare-column reducer-argument rule, phi
extraction, the ambient AccuracyTarget).

Also generalizes uniqExact's stub-UDAF + FunctionRewrite mechanism (#221)
so a new ClickHouse-only builtin DataFusion doesn't know at all is a
catalog data entry (name, arity, RewriteKind) rather than a new one-off
Rust type and a new one-off stub-UDAF function. uniqExact and the new
countIf entry both go through the single ClickHouseBuiltinRewrite +
clickhouse_builtin_stub_udaf mechanism now. countIf(cond) rewrites to
sum(CASE WHEN cond THEN 1 ELSE 0 END) rather than count(...) FILTER
(WHERE cond), since AggIntent::Count never consults its argument (it
always means "row count") and would silently drop the filter; summing a
0/1 indicator keeps cond observable through the existing Sum path,
including the general non-column-argument materialization
lower_aggregate already does (#110).

uniqExact's own behavior is unchanged (same signature, same rewrite
target) and its existing corpus tests (bgp_analytics, #221) pass
unmodified. countIf support moves 13 bgp_jan2024_workload corpus queries
out of Category::Plan (12 now lower end to end, one hits a second,
pre-existing NotImplemented gap) -- the pinned tally is updated
accordingly. New direct unit tests cover countIf's derived-column shape
and composition with GROUP BY.

Deliberately out of scope (per the issue): generating the catalog from a
live ClickHouse/DataFusion introspection source (item 3 -- flagged in the
issue as needing a decision on where such tooling would run).

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>

Author: zzylol

Cargo.lock

@@ -1,6 +1,7 @@
 [workspace]
 members = [
     "crates/types",
+    "crates/sql-function-catalog",
     "crates/asap-aware-mapping",
     "crates/frontend-promql",
     "crates/frontend-sql",

Cargo.toml

@@ -4,9 +4,12 @@ version = "0.1.0"
 edition = "2021"
 
 # SQL front end: L1 (parse + plan via DataFusion) → L2 relational, then the
-# shared L2→L3 converter — both in asap-types. Pulls DataFusion only — never promql-parser.
+# shared L2→L3 converter — both in asap-types. Pulls DataFusion, plus the
+# independent SQL function-name catalog (asap-sql-function-catalog, issue
+# #225) it consults when lowering an aggregate call — never promql-parser.
 [dependencies]
 asap-types = { path = "../types" }
+asap-sql-function-catalog = { path = "../sql-function-catalog" }
 datafusion = "43"
 
 [dev-dependencies]

crates/frontend-sql/Cargo.toml

@@ -36,8 +36,10 @@ pub async fn lower_sql(
 /// Lower a single SQL query string under an explicit [`SqlDialect`].
 ///
 /// `ClickhouseSQL` parses via sqlparser's vendored `ClickHouseDialect`
-/// (array-lambda syntax, `arr[-1]` indexing) — it does not teach DataFusion's
-/// planner any ClickHouse-only builtin functions, which still fail to plan.
+/// (array-lambda syntax, `arr[-1]` indexing). It also teaches DataFusion's
+/// planner the ClickHouse-only builtin functions listed in
+/// `asap_sql_function_catalog::CLICKHOUSE_BUILTINS` (`uniqExact`, `countIf`)
+/// — every other ClickHouse-only builtin still fails to plan.
 /// `ElasticSQL` has no vendored parser and always returns `UnsupportedDialect`.
 pub async fn lower_sql_dialect(
     query: &str,

crates/frontend-sql/src/lib.rs

@@ -160,15 +160,20 @@ async fn corpus_lowering_matches_the_pinned_aggregate_tally() {
              gap was added/removed: {tally:?}"
         );
     };
-    expect(Category::Lowered, 85);
-    expect(Category::Plan, 105);
+    // `countIf` support (issue #225 -- `ClickHouseBuiltinRewrite`, catalog-
+    // driven the same way `uniqExact` (#221) already was) clears the
+    // "unknown function: countif" `Plan` failure for 13 queries: 12 now
+    // lower end to end, and a 13th plans far enough to hit a second,
+    // pre-existing gap (map/array index access, `NotImplemented`).
+    expect(Category::Lowered, 97);
+    expect(Category::Plan, 92);
     expect(Category::Schema, 0);
     expect(Category::Parse, 0);
     // One query that used to fail at `uniqExact` (`Plan`) now clears that
-    // hurdle -- `UniqExactRewrite` in `sql/mod.rs` rewrites it to
+    // hurdle -- `ClickHouseBuiltinRewrite` in `sql/mod.rs` rewrites it to
     // `COUNT(DISTINCT ...)` before `lower_plan` runs -- and plans far enough
     // to hit a second, pre-existing gap: map/array index access.
-    expect(Category::NotImplemented, 4);
+    expect(Category::NotImplemented, 5);
     expect(Category::UnsupportedFeature, 6);
     expect(Category::Other, 0);
 }

crates/frontend-sql/src/sql/mod.rs

@@ -5,13 +5,14 @@
 //! the shared `resolve_root` produces the positional, resolved canonical
 //! tree (the same resolver the PromQL path uses).
 
-use asap_frontend_sql::{lower_sql, SqlCatalog};
+use asap_frontend_sql::{lower_sql, lower_sql_dialect, SqlCatalog};
 use asap_types::pre_asap::schema::{Column, DataType, Schema};
 use asap_types::pre_asap::{
     AggIntent, CompareOpKind, GroupKeys, JoinKind, QueryExpr, Reduction, ScalarValue, Source,
     WindowFuncKind,
 };
 use asap_types::types::AccuracyTarget;
+use asap_types::workload::SqlDialect;
 
 fn col(name: &str, dtype: DataType) -> Column {
     Column::new(name, dtype, false)
@@ -1392,3 +1393,76 @@ async fn array_agg_is_deliberately_rejected() {
         "expected a clean UnsupportedAggregate, got {err}"
     );
 }
+
+// ── Issue #225: catalog-driven ClickHouse builtins (countIf, generalizing
+// uniqExact from #221) ───────────────────────────────────────────────────
+
+async fn lower_clickhouse(sql: &str) -> QueryExpr {
+    lower_sql_dialect(
+        sql,
+        &catalog(),
+        SqlDialect::ClickhouseSQL,
+        AccuracyTarget::Exact,
+    )
+    .await
+    .unwrap_or_else(|e| panic!("lower failed for {sql:?}: {e}"))
+}
+
+#[tokio::test]
+async fn count_if_lowers_to_a_sum_over_a_derived_indicator_column() {
+    // ClickHouse's `countIf(cond)` has no DataFusion equivalent at all, so it
+    // goes through the same stub-UDAF + catalog-driven `FunctionRewrite`
+    // mechanism `uniqExact` (#221) does — rewritten, before `lower_agg_intent`
+    // ever runs, to `sum(CASE WHEN cond THEN 1 ELSE 0 END)`. Not a plain
+    // `count(...) FILTER (WHERE cond)`: `AggIntent::Count` never consults its
+    // argument (always a row count), so the filter would be silently dropped;
+    // summing a 0/1 indicator keeps `cond` observable through the ordinary
+    // `Sum` path instead.
+    let qe = lower_clickhouse("SELECT countIf(bytes > 100) AS big FROM metrics").await;
+    let (by, measures) = find_aggregate(&qe).expect("expected an Aggregate");
+    assert!(by.is_empty());
+    assert!(
+        matches!(measures.as_slice(), [AggIntent::Sum { col: Some(_) }]),
+        "expected a Sum bound to the derived indicator column, got {measures:?}"
+    );
+    let (_, materialized) = reducer_input_names(&qe);
+    assert!(
+        materialized,
+        "the indicator expression must be materialized in a Project beneath the Aggregate"
+    );
+}
+
+#[tokio::test]
+async fn two_count_ifs_with_different_conditions_stay_distinct_reducers() {
+    // The corpus pattern (`countIf(operation = 'A'), countIf(operation = 'W')`
+    // in one GROUP BY) needs each call's own condition to survive as its own
+    // derived column, not collapse onto a shared one.
+    let qe = lower_clickhouse(
+        "SELECT service, countIf(bytes > 100) AS big, countIf(bytes <= 100) AS small \
+         FROM metrics GROUP BY service",
+    )
+    .await;
+    let (by, measures) = find_aggregate(&qe).expect("expected an Aggregate");
+    assert_eq!(*by, GroupKeys::by(vec![0]));
+    assert!(
+        matches!(
+            measures.as_slice(),
+            [
+                AggIntent::Sum { col: Some(a) },
+                AggIntent::Sum { col: Some(b) }
+            ] if a != b
+        ),
+        "expected two distinct Sum reducers, got {measures:?}"
+    );
+}
+
+#[tokio::test]
+async fn count_if_composes_with_group_by() {
+    let qe = lower_clickhouse(
+        "SELECT service, countIf(bytes > 100) AS big FROM metrics GROUP BY service",
+    )
+    .await;
+    let (by, measures) = find_aggregate(&qe).expect("expected an Aggregate");
+    assert_eq!(*by, GroupKeys::by(vec![0]));
+    assert!(matches!(measures.as_slice(), [AggIntent::Sum { .. }]));
+}

crates/frontend-sql/tests/bgp_jan2024_workload/bgp_jan2024_workload.rs

@@ -0,0 +1,14 @@
+[package]
+name = "asap-sql-function-catalog"
+version = "0.1.0"
+edition = "2021"
+
+# The (SQL function name, arity) -> canonical AggIntent-shaped semantic
+# catalog `asap-frontend-sql` consults when lowering an aggregate call, plus
+# the ClickHouse-only-builtin -> native-DataFusion-shape rewrite table
+# (issue #225). No internal deps -- like `asap-types`, this is meant to be
+# consultable on its own (a future extraction tool populating it, a test
+# asserting a name is covered, ...) without pulling in DataFusion or the
+# front end that uses it. `asap-frontend-sql` depends on this crate, never
+# the other way around.
+[dependencies]