Skip to content

compute: reuse MonotonicTop1's output arrangement to elide redundant ArrangeBy#37592

Merged
antiguru merged 7 commits into
MaterializeInc:mainfrom
antiguru:bridge-cse_01Y8Ju5Fw1GS8xSBhDHwQAF6
Jul 14, 2026
Merged

compute: reuse MonotonicTop1's output arrangement to elide redundant ArrangeBy#37592
antiguru merged 7 commits into
MaterializeInc:mainfrom
antiguru:bridge-cse_01Y8Ju5Fw1GS8xSBhDHwQAF6

Conversation

@antiguru

Copy link
Copy Markdown
Member

Motivation

MonotonicTop1 builds a group-key-keyed output arrangement and then discards it
(TODO(database-issues#2288)), so the optimizer inserts a redundant ArrangeBy
whenever a consumer needs the result keyed by the group key, for example a join
on a DISTINCT ON key.

Addresses database-issues#2288.

Description

Lowering now advertises that arrangement via AvailableCollections::new_arranged,
mirroring Reduce, so a group-key consumer reuses it instead of re-arranging.

Render delivers the output in one of two shapes, selected by a new
MonotonicTop1Plan.arranged flag: arrangement-only when lowering advertised it,
or a raw collection for the case where refine_single_time_operator_selection
upgrades a Basic plan into MonotonicTop1 after lowering already advertised
raw. The flag is true only for plans built by create_from (the sole path
lowering advertises) and false for the post-lowering upgrade, so the render
shape always matches the advertisement.

Only MonotonicTop1 is affected. MonotonicTopK and Basic key their
arrangements by (hash, group_key), which a group-key consumer cannot reuse, so
they continue to advertise new_raw().

The top-1-monotonic testdrive goldens change: the ArrangeBy over each
elided dataflow is gone.

🤖 Generated with Claude Code

@antiguru antiguru requested a review from a team as a code owner July 12, 2026 12:57
Comment thread test/sqllogictest/transform/monotonic.slt Outdated
Comment thread test/sqllogictest/transform/monotonic.slt Outdated
@antiguru antiguru force-pushed the bridge-cse_01Y8Ju5Fw1GS8xSBhDHwQAF6 branch from 39ed77f to 6b3facc Compare July 12, 2026 15:17
@antiguru

Copy link
Copy Markdown
Member Author

Rebased onto #37593 (single-time monotonic selection moved into lowering). Because that runs as_monotonic before the arrangement advertisement, every MonotonicTop1 is now advertised, so this PR drops the arranged flag it previously needed and renders MonotonicTop1 unconditionally arrangement-only.

This PR now depends on #37593. Until #37593 merges to main, the diff here also shows #37593's commit (GitHub can't target a fork branch as the PR base); it reduces to just this change once #37593 lands.

@frankmcsherry frankmcsherry left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@antiguru antiguru force-pushed the bridge-cse_01Y8Ju5Fw1GS8xSBhDHwQAF6 branch from 6b3facc to 1289de5 Compare July 13, 2026 19:00
@antiguru

Copy link
Copy Markdown
Member Author

Rebased onto main now that #37593 has merged. The diff is now just this change (arrange the MonotonicTop1 output + reuse it), no longer stacked. Local verification: cargo check/clippy/fmt clean, sqllogictest 515/515 across the affected files.

antiguru and others added 7 commits July 14, 2026 03:54
Mirrors the arity field already present on MonotonicTopKPlan and
BasicTopKPlan, so that render/lowering can later derive the plan's
output arrangement layout from (group_key, arity). Pure plumbing: no
behavior change.
Make `render_top1_monotonic` build its output as a group-key-keyed
arrangement in the layout `permutation_for_arrangement` dictates, and have
`render_topk` return that arrangement in the bundle for the MonotonicTop1
arm, mirroring `Reduce`. The winning row is thinned to the value columns; the
group-key columns live in the key row.

Because lowering does not yet advertise the arrangement, the bundle also
carries a raw collection reconstructed from the arrangement via the
permutation, so consumers planned with `input_key = None` (for example a
Reduce over a DISTINCT ON) still find a collection. A later change advertises
the arrangement and can drop the reconstruction.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Lowering now advertises the group-key arrangement that
`render_top1_monotonic` builds, so a downstream consumer keyed on the
same group key reuses it instead of forcing a redundant `ArrangeBy`.
`MonotonicTopK`/`Basic` are unaffected: they key their arrangements by
(hash, group_key), which a group-key-only consumer can't reuse.

The render side keeps building both the arrangement and a reconstructed
raw collection. Dropping the reconstruction (as originally intended) is
unsound: `refine_single_time_operator_selection` can upgrade a `Basic`
plan into `MonotonicTop1` after lowering has already computed
`AvailableCollections` for the old (raw) variant, so a downstream
consumer can still expect a raw collection directly. Unlike
`Reduce::keys()`, which advertises the same arrangement for every
`Hierarchical` sub-variant, `TopK`'s variants differ in what they
arrange, so render can't tell which case it's in. Confirmed by a
reproduced crash (`keys.raw <= collection.collection.is_some()`
assertion) before adding the defensive raw reconstruction back.

Adds a regression test proving the `ArrangeBy` is elided when two
`DISTINCT ON` queries over the same monotonic source are joined on
their group key.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…not both

Task 3 defensively delivered a dual bundle (arrangement + reconstructed raw
collection) because render couldn't tell whether a MonotonicTop1 plan was
advertised as arranged by lowering, or was a Basic plan upgraded to
MonotonicTop1 after lowering by TopKPlan::as_monotonic (whose advertisement
stays raw). Add an explicit `arranged` flag to MonotonicTop1Plan, set true in
the lowering-time construction and false in the post-lowering upgrade, and
branch render on it to deliver exactly one shape.
`render_topk`'s `MonotonicTop1` arm computed `key` and `permutation` (via
`permutation_for_arrangement`) unconditionally, but they are only used in the
`!arranged` branch to reconstruct the raw collection from the arrangement.
Move the computation into that branch; the `arranged` branch never used them
and `key` duplicated work already done inside `render_top1_monotonic`.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The arranged=true correctness test spun up a bounded LOAD GENERATOR
COUNTER source and waited with mz_sleep, which flakes. The arbitrary
(non-prefix) group-key permutation reconstruction it exercised is
already covered deterministically by the one-shot DISTINCT ON over a
plain table (the arranged=false path, same permutation_for_arrangement
output), and the arranged=true plan is covered by the join-elision
EXPLAIN test.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…he variant

Rebasing on MaterializeInc#37593 (which inlines single-time monotonic operator selection
into lowering, running as_monotonic before the arrangement advertisement)
means every MonotonicTop1 plan, including a one-shot SELECT DISTINCT ON
upgraded during lowering, now advertises its group-key arrangement. That
makes the `arranged` flag on MonotonicTop1Plan always true, so drop the
field from the struct, create_from, as_monotonic, and the limit()
destructure, and make render_topk's MonotonicTop1 arm unconditionally
arrangement-only instead of branching on it. A consumer that needs the raw
collection reconstructs it from the advertised arrangement via its
permutation, same as for an index arrangement.

Regenerated sqllogictest goldens affected by the now-always-arranged
output: an explicit ArrangeBy on the group key is elided where the Top1
render used to require one, and Get/Reduce plans downstream of a
MonotonicTop1 now see an arrangement instead of a raw collection.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@antiguru antiguru force-pushed the bridge-cse_01Y8Ju5Fw1GS8xSBhDHwQAF6 branch from 1289de5 to 3bdc9d1 Compare July 14, 2026 01:58
@antiguru antiguru enabled auto-merge (squash) July 14, 2026 01:58
@antiguru antiguru merged commit f74a121 into MaterializeInc:main Jul 14, 2026
123 checks passed
@antiguru antiguru deleted the bridge-cse_01Y8Ju5Fw1GS8xSBhDHwQAF6 branch July 14, 2026 11:19
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.

2 participants