Skip to content

refactor(stage0, types): memo hash-consing and candidate storage (#1207 PR 2) - #1230

Draft
zzylol wants to merge 2 commits into
refactor/1207-propertiesfrom
refactor/1207-memo
Draft

zzylol wants to merge 2 commits into
refactor/1207-propertiesfrom
refactor/1207-memo

Conversation

@zzylol

@zzylol zzylol commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

Merge unit. This is PR 2 of #1207's merge unit (PRs 0–9). It is stacked on PR 1 (#1227, refactor/1207-properties), which is stacked on PR 0b (#1229) and PR 0 (#1226). The PRs merge into main together, after PR 9. This PR fills in only the Memo methods that PR 0 left as todo!() and rebuilds Stage 0 on them. Planning still panics at the first todo!() of Pass 1 (PR 3); see "Test status".

Why

The concepts doc (section 3) says that rules add groups only through Memo::find_or_add_group and candidates only through Memo::add_candidate. Rules match group expressions, never add the same expression/property pair twice, and never copy consumers to make a new producer visible. Pass 1 (PR 3) and Pass 2 (PR 4) are built on these two methods, and PR 0 left both as todo!().

Stage 0 had its own node-level hash-consing (cse.rs). Its equality compared schema, result kind, guarantee and children. A group lookup by expression would have been a second equality, and the two could disagree. #1207 asks for one equality.

What

  • Memo::find_or_add_group(expression) hash-conses on the operator, its arguments and its input group ids. If an equal expression already has a group, it returns that group. Otherwise it adds a group whose logical node is the operator over the input groups' logical nodes, with the derived schema, and lists the new group as a parent of each input group. θ is not an argument of any operator (PR 0 removed it from FieldDataType::Sketch), so builds of one family over one input that differ only in θ are one group.
  • Memo::add_candidate(group, candidate) appends the candidate to MemoGroup::candidates and returns CandidateForMemoGroupId { group, index_in_group }. If the group already has a candidate with the same expression and the same logical properties, it returns that candidate's id and drops the new one. A new candidate makes group a parent of each group it reads. The group's queries are added to those groups and to every group below them.
  • Memo::candidate(id) is unchanged (PR 1 filled it in).
  • Stage 0 (asap-stage0-memo): memo_of_queries now visits each node of the queries' DAGs once, children first, and calls find_or_add_group on its operator over its children's groups. It then lists, for each group, the queries that reach it. cse.rs (share_common_sub_dags, its intern table and hash caches) is deleted, together with the crate's test-support feature.
  • Dead test helpers deleted: share_identical and distinct_nodes in integration-tests/tests/planner_layering_common (#[allow(dead_code)], no caller).

Decisions and pending items

Item Decision Status
What makes two expressions one group Equal operator and arguments (typed PartialEq, confirmed by equal JSON, so signed zeros stay apart and a NaN never matches) over the same input groups at the same positions. Schema, result kind and guarantee are not compared: the schema is derived from the expression, and the guarantee is a candidate's (from θ), not the group's. pending
An expression whose copies may return different rows (unordered LIMIT, Dedup on some columns, row-position window functions; NonASAPOp::is_shareable_when_equal) It always gets a new group and is never returned for another expression. This is Stage 0's legality rule, kept. pending (rule unchanged from Stage 0)
A shareable parent over such children Two groups, since its input groups differ (DISTINCT over two unordered LIMIT 10s). Stage 0's old node equality compared children by value and made them one node. Both are correct: the new rule shares less, and never gives a wrong answer. pending
A duplicate candidate Same expression (the same equality as groups) and same logical properties: θ compared like an argument, window layout by ==. The duplicate's input_requirements, fused_groups and origin are not compared, and are dropped with it. pending
What parents holds The groups that read the group through their own expression (find_or_add_group, Stage 0) or through one of their candidates (add_candidate), each listed once. fused_groups are not reads. pending
What queries holds Ascending. Stage 0 sets it from each query's root through child groups. add_candidate extends it from the candidate's group to the groups the candidate reads and everything below them, through child groups and candidates' inputs. fused_groups get no queries. pending
An expression with no output schema find_or_add_group panics (# Panics in its doc). The doc's signature returns MemoGroupId, not a Result, and no rule may build such an expression. No frontend DAG in the test suites hits it (see "Test status"). pending
Group logical node Operator over the input groups' logical nodes, schema derived (OperatorNode::new), no guarantee. The frontends build their nodes the same way (none calls with_schema), so Stage 0's nodes are unchanged except that a guarantee is no longer kept. The frontends set none. pending

New abstractions not in #1207 or the doc (pending approval)

Item Where Why it is needed
private fields Memo::groups_by_expression_hash: HashMap<u64, Vec<MemoGroupId>> and Memo::indexed_group_count: usize, and private Memo::index_groups_pushed_directly types/src/stages/candidates.rs Lookup in O(1) expected per call, not a scan of every group. groups stays public as in the doc, so a group pushed onto it directly is indexed at the next find_or_add_group. Because of these fields, Memo can no longer be built with a struct literal outside its module; the one test that did (physical/memo_candidates.rs) now starts from Memo::default().
private module stages::hash_consing: same_expression, expression_hash, same_value, shareable_when_equal types/src/stages/hash_consing.rs The one equality. Groups and candidates both use it. It is the approved module name, but it now compares expressions over group ids (Operator<MemoGroupId>), not nodes, so it needs no node table or pointer caches.
pub(super) fn MemoGroup::expression(&self) -> Operator<MemoGroupId> types/src/stages/memo.rs A group's expression rebuilt from logical_node and child_groups, for comparing against an indexed group. The doc keeps the group as node + child list (a simplification it does not adopt).
private add_parent, same_expression_and_logical_properties, Memo::add_queries_below types/src/stages/candidates.rs Keep parents and queries up to date; find a duplicate candidate.
private GroupOfNode, group_of stage0-memo/src/lib.rs Visit each node of the queries' DAGs once, children first.
test helpers scan, kll_state, quantile_readout, unordered_limit, candidate, kll_k (memo_tests); node, scan, quantile_agg(_over), compare, memo_of, root, one_group, scalar_of_vector, summary_agg, unordered_limit, literal (Stage 0 tests) test modules Fixtures. candidate builds LogicalProperties directly because the doc example's k = 200 and k = 345 are not in today's KLL list.

How

  • Lookup. find_or_add_group first indexes any groups pushed directly. It then builds the expression's logical node (needed for the schema and for the legality check, which reads a LIMIT's child Sort or a Dedup's child schema). If the node is shareable, it hashes the operator's arguments (as JSON, input groups replaced by ()) together with the input group ids, and compares the groups in that bucket with same_expression. On a miss, it pushes the group, adds it to its inputs' parents, and indexes it only if it is shareable.
  • Candidates. add_candidate scans the group's own candidates for a duplicate; a group has few candidates. On a miss, it pushes the candidate, calls add_parent for each input group, and walks downward from the inputs with a worklist, adding the group's queries in sorted position. The walk does not descend from a group that already had every query, because the groups below it have them too.
  • Stage 0. group_of maps each original Rc<OperatorNode> by address to its group, so a diamond is visited once. It recurses into the children through Operator::map_children and calls find_or_add_group. Each query then adds its index to every group reachable from its root groups, in query order, so the lists come out ascending without sorting.

Major algorithm changes

Stage 0 hash-consing.

  • Problem. There were two equalities: Stage 0's node equality (cse::same_node: schema, result kind, guarantee, own fields, children by pointer and then by value) and the group lookup the rules need. A Pass 1 rule adding SummaryAgg[KLL](G1) could fail to find Stage 0's equal group whenever the two equalities disagreed, for example on the guarantee.
  • Before.
    1. share_common_sub_dags interns nodes bottom-up into an Rc table (SipHash of the node's JSON, then same_node).
    2. number assigns group ids to the interned nodes.
    3. pair maps the original nodes to those groups.
  • After.
    1. group_of recursively calls Memo::find_or_add_group(operator over child groups).
    2. The queries are listed per group.
  • Correctness. Two expressions are one group only if their arguments are equal (typed equality, confirmed by JSON) and their input groups are the same groups, so equal groups compute equal values by induction. The legality gate is unchanged. Tests: the 12 Stage 0 tests below, carried over from cse.rs at the memo level.
  • Complexity. Before: O(N) hashing, made linear by pointer caches, plus same_node per bucket hit. After: O(N) expected time for N nodes. Each lookup serializes only the node's own arguments, since its children are u32 ids, and needs no caches. Comparing against a bucket entry is O(arguments) and never recurses: the old 24-level diamond guard test has nothing left to guard.
  • Limits. A shareable parent over unshareable children is no longer shared (see the decisions table).

Key code interfaces

Bodies are elided as ;. Only the signatures marked // new or // removed changed; the others are PR 0's with bodies filled in.

crates/types/src/stages/candidates.rs

/// The memo, Stage 0's and Stage 1's one type (stays).
#[derive(Debug, Clone, Default)]
pub struct Memo {
    /// The groups, by id. // unchanged
    pub groups: Vec<MemoGroup>,
    /// Each query's root over groups, by index in the workload. // unchanged
    pub query_roots: Vec<QueryRoot<MemoGroupId>>,
    /// The shareable groups by the hash of their expression, for find_or_add_group. // new (private)
    groups_by_expression_hash: HashMap<u64, Vec<MemoGroupId>>,
    /// How many of `groups` are indexed; later ones were pushed directly. // new (private)
    indexed_group_count: usize,
}

impl Memo {
    /// The group of `expression` (operator, arguments, input groups), added
    /// if new; panics if the expression has no output schema.
    pub fn find_or_add_group(&mut self, expression: Operator<MemoGroupId>) -> MemoGroupId; // body was todo!()

    /// Adds `candidate` to `group`, or returns the id of the candidate there
    /// with the same expression and logical properties; maintains parents and queries.
    pub fn add_candidate(
        &mut self,
        group: MemoGroupId,
        candidate: CandidateForMemoGroup,
    ) -> CandidateForMemoGroupId; // body was todo!()

    /// The candidate `id`. // unchanged (PR 1)
    pub fn candidate(&self, id: CandidateForMemoGroupId) -> &CandidateForMemoGroup;
}

crates/types/src/stages/memo.rs

/// One memo group (stays; only field docs changed).
pub struct MemoGroup {
    /// Its expression's operator over its child groups' logical nodes, schema derived. // unchanged
    pub logical_node: Rc<OperatorNode>,
    /// Its child groups, in the node's child order. // unchanged
    pub child_groups: Vec<MemoGroupId>,
    /// The groups that read it through their expression or a candidate's. // unchanged (now maintained by the Memo methods)
    pub parents: Vec<MemoGroupId>,
    /// The queries whose roots reach it, ascending. // unchanged (now extended by add_candidate)
    pub queries: Vec<usize>,
    /// Its candidates: empty after Stage 0. // unchanged
    pub candidates: Vec<CandidateForMemoGroup>,
}

impl MemoGroup {
    /// Its expression: the logical node's operator over `child_groups`. // new (pub(super))
    pub(super) fn expression(&self) -> Operator<MemoGroupId>;
}

crates/stage0-memo/src/lib.rs, crates/stage0-memo/Cargo.toml

/// Stage 0: the memo groups of the queries' roots, without candidates. // unchanged signature
pub fn memo_of_queries(roots: &[QueryRoot]) -> Memo;

mod cse; // removed (share_common_sub_dags, InternTable, structural_hash)
pub mod test_support { pub use crate::cse::share_common_sub_dags; } // removed, with feature `test-support`

Usage

Concepts doc, section 1, G1 and G2. Planning past Stage 0 still stops at PR 3's todo!(), so this flow is the one memo_tests::kll_builds_with_k_200_and_k_345_share_one_group and Stage 0's tests run.

// `kll_build` and `readout_candidate` stand for the test's `candidate(operator, θ)` helper.
// The user's queries: A = p95 of latency[1m]; Stage 0 receives the frontends' DAG.
let memo: Memo = memo_of_queries(&roots); // G1 = Scan latency[1m], G3 = A's root; no candidates yet
// Pass 1 (PR 3) adds the KLL state group over G1: θ is not in the expression.
let g2 = memo.find_or_add_group(kll_state(g1)); // SummaryAgg[KLL](G1) → G2 (a new group; G2 is a parent of G1)
// One build candidate per listed θ, here the doc's k = 200 and k = 345.
let k200 = memo.add_candidate(g2, kll_build(g1, /* k */ 200)); // CandidateForMemoGroupId { group: G2, index_in_group: 0 }
let same = memo.find_or_add_group(kll_state(g1)); // == g2: one group for both sizes
let k345 = memo.add_candidate(same, kll_build(g1, /* k */ 345)); // { group: G2, index_in_group: 1 }
memo.add_candidate(g2, kll_build(g1, 200)); // == k200: duplicate, not added
// A's readout candidate in G3 reads G2: G2 and G1 now list A's query, and G2 lists G3 as a parent.
memo.add_candidate(g3, readout_candidate(g2, /* q */ 0.95));

Before / After

KLL example (doc section 1): G1 = Scan latency[1m], G2 = SummaryAgg[KLL](G1) with k = 200 and k = 345.

Before (PR 1) After (this PR)
Adding G2 find_or_add_group panics (todo!("#1207 PR 2")) G2 is added once; the second call returns G2
k = 200 and k = 345 add_candidate panics two candidates of G2: index_in_group 0 and 1
k = 200 added twice panics the first id is returned; G2 keeps 2 candidates
G2's parents/queries after A's readout reads it not maintained parents [G3], queries [A] (and G1 queries [A])
Stage 0's equality node-level cse::same_node (schema, result kind, guarantee, children), a second equality find_or_add_group's, the only one

Test status

New tests, each stating the behavior it checks in its doc line:

  • asap-types, stages::candidates::memo_tests (8): find_or_add_group_returns_the_group_of_an_equal_expression, …_separates_equal_operators_over_different_inputs, …_never_returns_an_unshareable_group, …_lists_the_new_group_among_its_inputs_parents, …_finds_a_group_pushed_directly, kll_builds_with_k_200_and_k_345_share_one_group, add_candidate_returns_the_existing_candidate_for_a_duplicate, add_candidate_maintains_parents_and_queries.
  • asap-stage0-memo (12): two_queries_sharing_a_sub_expression_get_one_group (also checks parents and queries), a_query_s_repeated_sub_expression_is_one_group, an_already_shared_node_is_one_group, aggregates_that_differ_in_an_argument_are_different_groups, equal_keyless_nodes_that_return_the_same_rows_are_one_group, differing_accuracy_targets_are_two_groups_over_one_scan_group, a_scalar_referenced_vector_is_one_group_across_queries, readouts_share_their_state_group_but_not_each_other, dedup_is_one_group_only_on_every_column, unordered_limits_and_their_parents_are_different_groups, limits_over_a_sort_with_their_partitions_are_one_group, signed_zero_and_non_finite_literals_are_different_groups.

cse.rs's 24 tests are not carried over one for one:

  • The 4 structural_hash cache tests are dropped, because the caches are gone.
  • shared_root_value_is_unchanged, asap_nodes_share_without_a_unique_key and asap_nodes_with_distinct_parameters_are_shared_but_distinct_guarantees_are_not are dropped, because guarantees are no longer part of a group. θ sharing is now tested in kll_builds_with_k_200_and_k_345_share_one_group.
  • shared_diamond_does_not_expand_during_comparison is dropped, because comparison no longer recurses.
  • The rest are merged into the 12 tests above.

None of these tests depends on another stage crate; asap-stage0-memo has no dev-dependencies.

Crate / suite PR 1 (ba60db8) PR 2 (this PR)
asap-types 209 passed 217 passed (+8 memo_tests)
asap-accuracy 79 passed 79 passed
asap-stage0-memo 24 passed, but only with --features asap-types/test-support; cargo test -p asap-stage0-memo alone did not compile (fixtures called Schema::has_unique_key) 12 passed, alone
crate_graph (asap-integration-tests --test integration crate_graph) 6 passed 6 passed
asap-planner, asap-executor, asap-devtools, asap-integration-tests (all targets) 378 ok / 199 failed / 10 ignored identical, test by test (diffed). The failures are the same todo!()s: PR 3 (Pass 1, 81), PR 4 (45 + 13), PR 6 (36 + 13 + 7), … No failure is in Stage 0 or find_or_add_group.

Checks: cargo check --workspace --all-targets, cargo clippy --workspace --all-targets -D warnings and cargo fmt --all --check are clean.

@zzylol zzylol added optimizer Workload optimization and plan selection refactor labels Nov 6, 2026
@zzylol
zzylol force-pushed the refactor/1207-properties branch 2 times, most recently from 60a16ee to 107a77b Compare November 6, 2026 19:12
… PR 2)

Memo::find_or_add_group hash-conses on a group's expression (operator,
arguments, input groups; stages::hash_consing) and stores the schema and
result kind the node rules derive; Memo::add_candidate checks a candidate by
the same rules, requires the group's output, drops a duplicate (same
expression and logical properties) and keeps parents and queries up to date;
Memo::add_query and Memo::logical_node_trees are filled in. Stage 0 builds
its groups bottom-up through find_or_add_group and adds each query with
add_query.
@zzylol
zzylol force-pushed the refactor/1207-memo branch from 9e7e404 to 7ea9152 Compare November 6, 2026 19:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

optimizer Workload optimization and plan selection refactor

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant