Skip to content

docs(adr): aggregate placement, proposed on two queries of 22 - #216

Merged
Hor911 merged 1 commit into
mainfrom
docs/adr-aggregate-placement
Oct 21, 2026
Merged

Hor911 merged 1 commit into
mainfrom
docs/adr-aggregate-placement

Conversation

@Hor911

@Hor911 Hor911 commented Oct 21, 2026

Copy link
Copy Markdown
Contributor

Summary

A new ADR, Proposed, for computing a grouped aggregate on one side of the join below it (Yan and Larson,
SIGMOD 1994). No code: this is the design, the survey behind it and the gate, following the pattern ADR 0029
and ADR 0030 set — a Proposed ADR with numbered items, each shipping as its own PR, and a final docs(adr) item
that accepts or defers with the measured numbers.

The case. Q13 is a textbook instance and the worst query of the 22 against DuckDB by peak memory: antb1
builds a hash table on 148,451,295 order rows, probes 15,000,000 customers to emit 153,451,474 rows, then groups
those back down to 15,000,000. Moving the aggregate below the join, at SF 100 on four threads, interleaved and
rotated:

as written pre-aggregated
time 4.581 s 2.263 s (0.494x)
peak memory 14,325 MiB 1,554 MiB (0.108x)

Answers identical over all 48 rows, and the ratio held across four independent measurements taken days apart
(0.547x, 0.546x, 0.494x). The memory figure is the stronger half of the case — the difference between a query
needing 14 GB and one fitting in a container.

The survey found the shape in 14 of the 22 and worth doing in 2, and the reason so few qualify is structural
rather than incidental: the required pre-grouping key is the join key together with every grouping column from
the pre-grouped side, and in a star-like schema that union is usually a key of that side, making the reduction
1.00:1 by construction. Three queries are refused on that alone with no cardinality estimate needed.

The natural gate does not work, and the ADR says so. "Reduce a lot and discard little" rates Q9 best in the
whole survey on both axes while doing provably nothing to it — its aggregate consumes 319,404 rows and emits 175
groups in both forms, so the ratio is read off a quantity the push leaves unchanged — and it does not even
order Q10 (wins, 3.02:1) against Q3 (loses, 3.9:1). The Decision carries a six-condition gate instead, two of
whose conditions are the only signals that classify Q18 and Q21 correctly and are absent from the natural rule
entirely.

Closes #

Type of change

  • feat: new SQL, CLI or engine capability
  • fix: bug fix
  • perf: performance improvement
  • refactor, test, docs, build, ci or chore
  • Breaking change (CLI, output format or semantics); also add the breaking-change label

Verification

$ pixi run ci                 # unchanged tree, so this is a control
100% tests passed out of 2813

$ pixi run lint
every hook Passed except shellcheck: 37 pre-existing SC2218 findings in scripts/ and tools/github/, on files
this PR does not touch; `git stash && pixi run lint` fails identically on main and CI's lint leg passes. markdownlint,
typos and check_repo.py all pass on the new file and the index row.

Docs only — no behaviour changes, so there is nothing to measure here beyond the numbers the ADR records. Those
were measured at SF 100 on four threads with /usr/bin/time -l for peak RSS, arms interleaved and rotated.
TPC-H-derived data generated locally; not TPC-H Benchmark results.

Checklist

  • pixi run check passes locally (lint + clang Debug -Werror + hermetic tests) — not ticked: check
    stops at the pre-existing shellcheck findings above. Both halves were run separately and pass.
  • Tests cover the change (unit tests under src/<module>/tests/, or why none are needed) — none needed:
    no code changes. The tests the work itself will need are specified in the Plan's G1 and G2.
  • Docs updated where behavior, commands or architecture changed (AGENTS.md, docs/, an ADR), or not needed
  • No ClickBench-derived data is committed: no Parquet files, query answers or values from hits (ADR-0006)
  • Nothing derived from TPC-H is committed: no query text or fragments, data, answers or TPC tools (ADR-0006)
  • Changes to governance paths (see .github/CODEOWNERS) were agreed with a maintainer

The open checklist item is the whole PR: /docs/adr/ is CODEOWNERS and a new ADR is a status decision, which
AGENTS.md puts on the "Ask a human first" list. This is the ask. Nothing is built; if you would rather it were
Rejected than Proposed, the Context already carries the evidence for that reading — two queries of 22, one of
them the known case — and only the Status line and the Plan would change.

On ADR 0006

Every cardinality, ratio and timing is permitted. No query text, no fragment, no data value and no EXPLAIN output
appears: query shapes are described in words and queries named by number only. I checked this deliberately,
having violated it once on #211.

What is refused rather than deferred

  • The mirror direction (pulling an aggregate up). Q20's decorrelated plan already places one below a join
    where 99% of its groups are discarded, and grouping after the join measured 0.881x and then 0.778x — both
    arm after arm
    . Interleaved and rotated over six repeats it is 1.010x; the apparent win was the second arm
    running on a warmer page cache each time. Peak memory does improve 0.920x. Recorded as measured and refused.
  • A guard on the join shape instead of a gate on the rewrite: penalising a join whose only edge has a tiny
    key domain fires on 9 of the 22, every one a dimension lookup whose small side's key is its primary key,
    including the join ADR 0026's C5 rule exists to protect. The fix belonged at the estimator's input and landed
    there in fix(plan): an unknown key domain must not read as the largest #215.
  • Raising KeyFilter::kMaxBytes 16 → 32 MiB so a published filter could rescue the Q3-like cases: measured
    1.001x over the 22, and Q3 still receives no filter, because what blocks it is ADR 0029's uncarried offer
    through a pushing Filter, not the cap.

A prerequisite, already merged

The first attempt at this rewrite produced a plan the search ranked cheapest at 150,000 rows that produced
6,697,748,923 — 145.2 s where the same query with that one join ordered differently runs 140.6 ms. That was the
estimator reading a derived table's join four orders of magnitude low, fixed in #215. The ADR states plainly that
this item could not have been built before that, because the gate would have been fed the same numbers.

AI assistance

  • No AI assistance
  • AI-assisted. Tools and what they did: Claude Code (Opus 5) ran the survey (nine subagents over the 22
    queries, each writing and verifying a rewrite at SF 1), wrote the ADR, and took the SF 100 measurements. I
    re-measured every number the ADR quotes myself, interleaved and rotated, which is how the mirror direction's
    apparent win turned out to be a measurement artifact — I had reported it twice before catching it.
  • Accountable human (has read and understands the whole diff): @Hor911

A grouped aggregate that reads one side of the join below it can be
computed on that side first (Yan and Larson, SIGMOD 1994). Q13 is a
textbook instance and the worst query of the 22 against DuckDB by peak
memory: it builds a hash table on 148 M order rows, probes 15 M customers
to emit 153 M rows, and groups those back to 15 M. Moving the aggregate
below the join is 0.494x and takes peak memory from 14,325 MiB to 1,554,
answers identical, the ratio holding across four measurements days apart.

A survey of all 22 found the shape in 14 and worth doing in 2. The reason
so few qualify is structural and outlives this benchmark: the required
pre-grouping key is the join key together with every grouping column from
the pre-grouped side, and in a star-like schema that union is usually a
key of that side, so the reduction is 1.00:1 by construction.

The natural gate does not work. "Reduce a lot and discard little" rates Q9
best in the survey on both axes while doing provably nothing to it -- its
aggregate consumes 319,404 rows and emits 175 groups in both forms -- and
it does not even order Q10, which wins at 3.02:1, against Q3, which loses
at 3.9:1. So the Decision carries a six-condition gate instead, of which
the two that classify Q18 and Q21 correctly are absent from the natural
rule entirely.

The mirror direction is refused rather than deferred. Q20's decorrelated
plan already places an aggregate below a join where 99% of its groups are
discarded, and grouping after the join measured 0.881x and then 0.778x --
both arm after arm. Interleaved and rotated over six repeats it is 1.010x,
the apparent win being the second arm's warmer page cache each time.

Two alternatives were measured and refused: a guard on the join shape,
which fires on 9 of the 22 including the join ADR 0026's C5 rule exists to
protect, and raising KeyFilter's cap, which is 1.001x over the 22.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@Hor911
Hor911 requested a review from lll-phill-lll as a code owner October 21, 2026 23:38
@claude

claude Bot commented Oct 21, 2026 •

Copy link
Copy Markdown

Claude finished @Hor911's task in 17s —— View job


Claude review: no problems found

  • Docs-only PR (new ADR 0032 plus its index row). I found no P0 or P1 problems.
  • I found no ClickBench data, TPC-H query text, answers or tools. Queries are named by number only, and the ADR doesn't present any timing as a TPC-H Benchmark result.
  • I did not check the SF 1 and SF 100 measurements. I can't reproduce them in this run.
  • "Ask a human first" paths changed: docs/adr/ (CODEOWNERS). The PR adds a new ADR with status Proposed, so a maintainer should confirm it was agreed.

@Hor911
Hor911 merged commit 41aaad3 into main Oct 21, 2026
14 checks passed
@Hor911
Hor911 deleted the docs/adr-aggregate-placement branch October 21, 2026 23:42
@github-actions

Copy link
Copy Markdown

Coverage

Module Lines Floor Branches Floor Status
common 98.07% (203/207) 92.9% 92.44% (110/119) 90.8% ok
sql 99.08% (3244/3274) 97.3% 97.64% (2190/2243) 95.6% ok
plan 95.60% (8821/9227) 93.5% 90.13% (4728/5246) 89.5% ok
io 94.30% (1241/1316) 93.2% 90.11% (720/799) 86.6% ok
exec 97.05% (11191/11531) 96.1% 88.81% (6323/7120) 85.1% ok
engine 95.98% (478/498) 93.0% 88.52% (239/270) 85.1% ok
cli 94.59% (647/684) 93.4% 87.08% (283/325) 86.4% ok
total 96.59% (25825/26737) 90.52% (14593/16122)

Coverage gate: PASS (floors: tools/ci/coverage_thresholds.json; they only go up).

Clang source-based coverage of the hermetic tests over src/<module>/ (module unit tests excluded); N/A: nothing to cover. Reproduce with pixi run coverage (details: build/coverage/report.txt).

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