feat(sql): lower ClickHouse argMax/argMin to AggIntent::Extension

[zzylol]

Summary

argMax(arg, val) — "return arg's value from the row where val is maximal" — appears 3 times in the bgp_jan2024_workload corpus and had no AggIntent representation, unlike every existing reducer (Sum/Min/Max/Avg/...) which folds one column to a value derived from itself. argMax/argMin are two-column, row-selecting aggregates: they return a different column's value, selected by which row extremizes a second column. No existing AggIntent shape fits.

Design decision: AggIntent::Extension, not a new core ArgMax/ArgMin variant

Issue #232 asked for research before picking between (1) a first-class AggIntent::ArgMax/ArgMin core variant, or (2) AggIntent::Extension — the crate's own escape hatch, whose doc comment says "core only grows for intents ≥2 deployment models actually use."

Research performed: a repo-wide grep (grep -rniE 'arg[_ ]?max|arg[_ ]?min' over .rs/.md/.toml, plus a full-repo scan for argmax/argmin) across the PromQL front end, other SQL dialects, and docs found zero other uses or discussion of an arg-max/arg-min concept anywhere in the codebase — only the 3 corpus occurrences this PR closes.

More importantly, crates/sql-function-catalog/src/lib.rs's own KNOWN_UNMAPPED_NATIVE_FUNCTIONS table already documents this exact gap shape twice, independently:

• first_value/last_value/nth_value: "no AggIntent variant models 'the value from a particular row' as a reduction"
• corr/covar/regr_*: "every AggIntent value reducer takes one input column (reducer_col...), so these have no home yet"

argMax/argMin are exactly the union of both gaps (a two-column reducer and a "value from a particular row" selector). Structurally, AggIntent's input_col()/resolve_agg_intent/aggregate_output_schema/collect_referenced_columns machinery is single-column by construction throughout crates/types/src/pre_asap/; giving a new variant a real second column would mean widening that machinery for a shape exactly one deployment model (this ClickHouse dialect) needs today — precisely what the "≥2 deployment models" bar exists to gate against.

Given that, this lowers argMax/argMin to AggIntent::Extension { ext_kind: "arg_max" | "arg_min", payload }. This required zero changes to crates/types — Extension already has complete plumbing (resolve_agg_intent, output_column, requires/is_per_series, and downstream PassThrough handling in asap-aware-mapping's boundary.rs/bind.rs/cost_model.rs), which is itself evidence the Extension escape hatch is doing its intended job here.

Implementation

• crates/sql-function-catalog: new RewriteKind::PassThrough variant for a CLICKHOUSE_BUILTINS entry with no native DataFusion aggregate shape to rewrite to at all — a new pattern alongside the existing CountDistinct/CountIfToSum rewrite targets (which do rewrite to an already-handled native shape). New entries: argmax/argmin, arity Exact(2).
• crates/frontend-sql/src/sql/mod.rs:
  • ClickHouseBuiltinRewrite::rewrite treats PassThrough as a no-op (Transformed::no) — the call survives to lower_agg_intent under its own ClickHouse name, still routed through the existing stub-AggregateUDF registration loop in build_context (so DataFusion's planner accepts the name at all).
  • New lower_arg_selector() recognizes "argmax"/"argmin", validates both arguments as bare columns (generalizing reducer_col's existing "no expression arguments" rule from issue L3: AggIntent::Quantile / Cardinality / TopK drop their input column — distinct aggregates compare equal #115 to 2 args), and builds the Extension node directly. Core never resolves Extension's payload, so both columns are kept as ColumnRefs (JSON) rather than being run through resolve_agg_intent's positional ColumnId binding — consistent with Extension's documented opaque-to-core contract.
• Known, documented (not fixed) scope gap, matching issue SQL: generalize the ClickHouse-builtin stub-registration mechanism from AggregateUDF to ScalarUDF #230's precedent of leaving splitByChar's array-indexing gap undone: DerivedCols::rewrite_agg only materializes/passes through an aggregate's first argument when deriving columns beneath an Aggregate node. If some other aggregate in the same query forces a Project to be inserted, argMax's second argument (val) could be silently dropped from it. This does not affect the corpus today — all 3 argMax occurrences have both arguments as bare columns and no such Project gets inserted for them. Documented in lower_arg_selector's doc comment.

Scope boundary

Purely structural, per the precedent issue #230 already set: getting argMax/argMin to lower to a structurally correct AggIntent node. No SummaryKind/sketch-selection or real runtime binding for Extension's arg_max/arg_min payload in asap-aware-mapping — it defaults to Implementation::PassThrough, same as every other unrecognized Extension kind today.

Verification

• crates/frontend-sql/tests/sql_lowering.rs: 4 new unit tests — argMax/argMin each lower to their own Extension { ext_kind, .. }, the payload preserves both column names correctly, and a non-column argument is rejected (not silently dropped).

• bgp_jan2024_workload corpus tally (corpus_lowering_matches_the_pinned_aggregate_tally):

  Category	Before	After
  Lowered	145	148
  Plan	40	37
  everything else	unchanged	unchanged

  All 3 corpus argMax occurrences move cleanly from Plan ("unknown function: argmax") to Lowered, with no companion gap.

• cargo build --workspace --all-targets, cargo test --workspace, cargo fmt --all -- --check, cargo clippy --workspace --all-targets --all-features -- -D warnings — all clean.

Closes #232