Skip to content

Add plan parent-reduction rule API - #9196

Draft
joseph-isaacs wants to merge 13 commits into
vortex-planfrom
vortex-plan-rules
Draft

Add plan parent-reduction rule API#9196
joseph-isaacs wants to merge 13 commits into
vortex-planfrom
vortex-plan-rules

Conversation

@joseph-isaacs

@joseph-isaacs joseph-isaacs commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Summary

  • add a typed PlanParentReduceRule API for child-driven physical-plan rewrites
  • add a type-erased adapter for collecting heterogeneous typed rules
  • add an ordered static PlanParentRuleSet whose first successful rewrite wins
  • define rules as one-step rewrites while leaving recursive traversal to the optimizer
  • verify in debug builds that rewrites preserve the parent row count and dtype
  • expose the framework through vortex_layout::plan::optimizer

Stack

This is part 2 of the plan-native scan stack. It is based on #9142 and is followed by #9166.

Scope

This PR only introduces the rule framework and API contract. It registers no concrete rules and
does not change optimization behavior.

Checks

  • cargo test -p vortex-layout
  • cargo clippy -p vortex-layout --all-targets --all-features -- -D warnings
  • cargo +nightly fmt --all -- --check
  • git diff --check

@codspeed-hq

codspeed-hq Bot commented Aug 5, 2026

Copy link
Copy Markdown

Merging this PR will degrade performance by 0.46%

⚡ 4 improved benchmarks
❌ 3 regressed benchmarks
✅ 1927 untouched benchmarks
⏩ 51 skipped benchmarks1

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
Simulation decode_varbin[(1000, 2)] 61.5 µs 78.4 µs -21.51%
Simulation compress_fsst[(1000, 64, 8)] 1 ms 1.2 ms -11.67%
Simulation compress_fsst[(10000, 64, 4)] 8.6 ms 9.7 ms -10.57%
Simulation take[small_m/shuffled/primitive/nonnull/chunks=16384/indices=16] 1.3 ms 1.2 ms +12.41%
Simulation decompress[u64, (1000, 16)] 72.3 µs 64.4 µs +12.2%
Simulation compact_sliced[(4096, 90)] 1.9 µs 1.7 µs +11.69%
Simulation compact_sliced[(16384, 90)] 2 µs 1.8 µs +10.81%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.


Comparing vortex-plan-rules (825351d) with vortex-plan (c351747)2

Open in CodSpeed

Footnotes

  1. 51 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

  2. No successful run was found on vortex-plan (55e7a8c) during the generation of this report, so 31bd037 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

@joseph-isaacs
joseph-isaacs force-pushed the vortex-plan-rules branch 3 times, most recently from 3675240 to 79b24a6 Compare August 6, 2026 20:40
@joseph-isaacs
joseph-isaacs force-pushed the vortex-plan-rules branch 3 times, most recently from 29488db to 6c8e44e Compare August 7, 2026 15:33
joseph-isaacs and others added 11 commits August 7, 2026 16:57
Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk>
Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk>
Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk>
Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk>
Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk>
Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk>
Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk>
Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk>
Replace the per-node `Plan::optimize` recursion with `children()` +
`with_children()`, which lets `PlanRef` implement
`vortex_array::expr::traversal::Node`. Optimization rules now live in
`plan/optimize.rs` as a `NodeRewriter` and are written once against the
generic tree instead of inside every plan kind.

`PlanRef` becomes a newtype over `Arc<dyn Plan>` so the foreign `Node`
trait can be implemented for it, matching `ScalarFnRef`. It derefs to
`dyn Plan`, so call sites are unchanged.

Children are now stored uniformly and built eagerly, which removes
`LazyPlanChildren` and its per-slot closures. Absent optional slots are
omitted from `children()` rather than reported as `None`, so out-of-bounds
access has one behaviour across every plan kind instead of the previous
mix of `Ok(None)` and an error.

Building the tree eagerly means a layout with no plan implementation now
fails in `new_plan` rather than when its child is first accessed. The two
deferral tests are replaced by tests asserting that eager failure.

Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk>

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012obBhJ8oPZoBbKyeS79yMv
Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk>
Introduce `PlanVTable` plus a `Plan<V>` container and a `DynPlan` erased
trait, matching how `Layout<V>` and `ScalarFn<V>` are built. Common fields
— dtype, row count, children — live in the container and are written once;
operator-specific state lives in `V::PlanData` and is reached typed through
`Deref` rather than by downcasting.

Plan operators no longer hold a layout handle. Each carries everything it
needs, so a rewrite can reason about a plan's shape alone:

  FlatPlan       -> SegmentScan  read one segment
  ChunkedPlan    -> Concat       concatenate children row-wise
  StructPlan     -> Pack         assemble a struct from field children
  DictPlan       -> Take         index values by codes
  ListPlan       -> ListPack     assemble a list from elements and offsets
  ExpressionPlan -> Eval         apply an expression to its child
  RowIdxPlan     -> RowIdx       offset row numbers

Naming operators for what they compute rather than for what produced them
is what lets one rule cover every case: `Concat` of `Concat` flattens on
shape alone, and `Take` over `SegmentScan` is the dictionary pushdown, no
matter which layout each was lowered from.

`new_plan` becomes `lower` in `plan/lower.rs`, the only module that knows
about layouts. The dependency runs one way: lowering imports plans, plans
never import layouts.

`PlanVTable` carries `id` and a `Metadata` codec so a registry and a
serialization envelope can be added without reshaping operators. Operators
whose state is recoverable serialize their metadata today; `SegmentScan`
and `Eval` return `None` until a read context and a bound expression have
codecs.

Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk>

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012obBhJ8oPZoBbKyeS79yMv
Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk>
Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk>
Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk>
Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog/feature A new feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant