Skip to content

test(sql): pin array_agg's rejection as a decision, not a gap (#111) - #122

Merged
zzylol merged 1 commit into
mainfrom
test/111-array-agg-wontfix
Jul 10, 2026
Merged

zzylol merged 1 commit into
mainfrom
test/111-array-agg-wontfix

Conversation

@zzylol

@zzylol zzylol commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Follow-up to the array_agg WONTFIX decision on #111.

Why

array_agg had no test anywhere in the repo. That means "we decided not to support this" and "nobody has gotten to it yet" are indistinguishable from the code — the next person to read unsupported aggregate: array_agg has no way to tell which it is.

This adds the assertion, with the reasoning inline:

  • AggIntent is the vocabulary plan::bind / plan::boundary bind sketches and mergeable accumulators to. array_agg pre-aggregates nothing (output is O(input rows)), has no bounded-memory approximate form, and its partial state is the data — so "mergeable" carries no information.
  • plan::boundary::realize is deliberately an exhaustive match ("adding a variant without an explicit realization is a compile error"). An AggIntent::ArrayAgg would force boundary, bind and schema derivation to each handle a variant whose every answer is PassThrough.
  • It appears in neither SQL corpus. It reached SQL: subquery-valued predicates (IN/EXISTS/scalar), DISTINCT ON, and median/array_agg unsupported #111 because I probed 36 hand-written shapes against lower_sql, not because a workload asked for it.

Contrast median from the same bucket in #111: it is Quantile { q: 0.5 } and feeds the sketch path directly, so it was implemented (#117) rather than rejected.

Scope

Test only — no source change. Reopen the decision if a real workload needs array_agg at L3; the argument is cost/benefit, not impossibility.

Verification

cargo test --workspace      # 380 passed, 0 failed
cargo clippy --all-targets  # clean

🤖 Generated with Claude Code

`array_agg` has no test, so "deliberately rejected" and "not implemented yet"
look identical in the code. Assert the rejection, with the reasoning inline.

AggIntent is the vocabulary the planner binds sketches and mergeable
accumulators to. array_agg pre-aggregates nothing (output is O(input rows)),
has no bounded-memory approximate form, and its partial state is the data
itself. Since plan::boundary::realize is an exhaustive match, an
AggIntent::ArrayAgg would force boundary, bind and schema derivation to each
handle a variant whose every answer is PassThrough.

Contrast median (#117), from the same bucket in #111: it *is* Quantile{q:0.5}
and feeds the sketch path, so it was implemented rather than rejected.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@zzylol
zzylol merged commit ec5176f into main Jul 10, 2026
1 check passed
@zzylol
zzylol deleted the test/111-array-agg-wontfix branch July 10, 2026 02:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant