Repository navigation
refactor(stage0, types): memo hash-consing, candidate storage and group-coverage tests (#1207 PR 2) - #1230
Draft
zzylol wants to merge 2 commits into
Draft
refactor(stage0, types): memo hash-consing, candidate storage and group-coverage tests (#1207 PR 2)#1230zzylol wants to merge 2 commits into
zzylol wants to merge 2 commits into
Conversation
zzylol
force-pushed
the
refactor/1207-properties
branch
2 times, most recently
from
November 6, 2026 19:12
60a16ee to
107a77b
Compare
zzylol
force-pushed
the
refactor/1207-memo
branch
from
November 6, 2026 19:14
9e7e404 to
7ea9152
Compare
zzylol
force-pushed
the
refactor/1207-properties
branch
2 times, most recently
from
November 6, 2026 21:38
c7a0bc9 to
9e5ce62
Compare
zzylol
force-pushed
the
refactor/1207-memo
branch
from
November 6, 2026 21:38
54d8b30 to
60c5cfe
Compare
zzylol
force-pushed
the
refactor/1207-properties
branch
from
November 6, 2026 22:11
9e5ce62 to
96d3457
Compare
zzylol
force-pushed
the
refactor/1207-memo
branch
from
November 6, 2026 22:11
60c5cfe to
4915a82
Compare
zzylol
force-pushed
the
refactor/1207-properties
branch
from
November 6, 2026 22:46
96d3457 to
b250852
Compare
zzylol
force-pushed
the
refactor/1207-memo
branch
from
November 6, 2026 22:46
4915a82 to
e710906
Compare
This was referenced Nov 6, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
The concepts doc (section 3) says that rules add groups only through
Memo::find_or_add_groupand candidates only throughMemo::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. PR 0 left them,Memo::add_queryand Stage 0'smemo_of_queriesastodo!(). PR 0 also madeGroupProperties(schema, result kind, summary coverage, sharing) derived from the input groups by the existing rules (derive_group_properties,OperatorInput,MemoGroupAsInput); this PR stores them.Before PR 0, Stage 0 had its own node-level hash-consing (
cse.rs). Its equality compared schema, result kind, guarantee and children, so a group lookup by expression would have been a second equality, and the two could disagree. #1207 asks for one equality, and (decided 2026-11-06) a group is its expression.What
Memo::find_or_add_group(expression) -> Result<MemoGroupId, SchemaDerivationError>hash-conses on the expression: the operator, its arguments and its input group ids.GroupPropertieswithderive_group_properties(PR 0): the existing node rules (schema derivation, input validation, merge check) over the input groups'group_propertiesandexpression, never a node tree. An expression that breaks them returns the error, and the memo is unchanged.MemoGroup { expression, group_properties, .. }and lists the new group as a parent of each input group.Memo::add_candidate(group, candidate) -> Result<CandidateForMemoGroupId, CandidateError>derives the candidate's properties the same way (CandidateError::Schema). It then requires the derived schema and result kind to equal the group's (CandidateError::OutputDiffersFromGroup). Both checks run before the dedup and before any change, so a rejected candidate leaves the memo unchanged.MemoGroup::candidates.groupbecomes a parent of each group it reads, and the group's queries are added to those groups and every group below them.Memo::add_query(root) -> usizeappends the root toquery_rootsand lists the query on every group the root reaches.types/tests/memo_summary_coverage.rschecks PR 0's group-based coverage throughfind_or_add_group, on the worked example ofasap-primitive-schema.md§4.2.2 (see "Test status").docs/design_docs/proposals/asap-primitive-schema.md§4.2.2 describes the derivation over memo groups (SummaryCoverage::derive_from_inputs, stored inGroupProperties::summary_coverage), with the worked example G1–G10 and its three cases, andunlifted_operators: Vec<Operator<Option<MemoGroupId>>>(a scalar subquery names its group, so such definitions compare); §4.2.3 ("When it runs") and §6.6 follow. The node-tree notationcoverage()in §5's examples and §6.1 is not rewritten here.Memo::candidate(id)is unchanged (PR 1).asap-stage0-memo):memo_of_queries(roots) -> Result<Memo, SchemaDerivationError>visits each node of the queries' DAGs once, children first. It callsfind_or_add_groupon the node's operator over its children's groups, thenadd_queryper root.run_planningwith?, devtools'run_stages_untilasPlanningStagesError::Validation, andsketch_coveragecounts that root as rejected (PR 0).same_output_contractandoutput_contract_satisfiedexist only on the closed Logical memo candidate as (one operator over groups, logical properties θ) with a requirement per input; no private nodes #1203 branch.add_candidate's output check replaces them, so PR 4 (Pass 2 rules) and PR 6 (realizer) must not port them.csemodule now point atMemo::find_or_add_group(schema_support.rs,canonicalize.rs).Decisions and pending items
MemoGroup { expression, group_properties, parents, queries, candidates }. Hash-consing compares and hashesexpressiondirectly: equal operator and arguments (typedPartialEq, confirmed by equal JSON, so signed zeros stay apart andNaNnever matches) over the same input groups at the same positions.Result; one set of node rules for groups and candidatesderive_group_properties(PR 0): the existing rules overMemoGroupAsInputs. A candidate's schema and result kind must equal the group's (==).GroupProperties,SummaryStateDefinition { input_group, unlifted_operators },OperatorInput,MemoGroupAsInput,derive_group_properties, coverage only in the memoInputOrTemporaryBuild/TemporaryBuild(a build a rule constructs only while it derives, never stored in the memo);unlifted_operators: Vec<Operator<Option<MemoGroupId>>>(None= an operator's main input,Some(g)= a group a scalar subquery reads), so states whose definitions read subqueries merge; no node-treemerged_window_buildwrapper (accuracy's merged-windows guarantee istodo!()until PR 4)input_group: Option<MemoGroupId>,OperatorInputpub+Clone,derive_from_inputsoverMemoGroupAsInputonlyLIMIT,Dedupon some columns, row-position window functions;GroupProperties::shareable_when_equal)DISTINCTover two unorderedLIMIT 10s). Stage 0's old node equality compared children by value and made them one node. Both are correct; the new rule shares less.==. The duplicate'sinput_requirements,fused_groupsandoriginare not compared, and are dropped with it.parentsholdsfind_or_add_group) or through one of their candidates (add_candidate), each listed once.fused_groupsare not reads.queriesholdsadd_querylists the query on its root's groups and everything below them;add_candidateextends the candidate's group's queries to the groups the candidate reads and everything below them. "Below" follows expressions and candidates' inputs.fused_groupsget no queries.memo_of_queriesreturns the first node-rule error (Result<Memo, SchemaDerivationError>). The facade validates the roots first, so it signals a frontend bug.New abstractions not in #1207 or the doc
Memo::groups_by_expression_hash: HashMap<u64, Vec<MemoGroupId>>types/src/stages/candidates.rsO(1)expected per call. Groups are private and added only throughfind_or_add_group, so the index covers every group.Memo::groupswithMemo::groups() -> &[MemoGroup](PR 0)find_or_add_group.CandidateError::{Schema(SchemaDerivationError), OutputDiffersFromGroup { group }}(PR 0)add_candidaterejects a candidate; both are bugs in the rule that built it.Memo::add_query(&mut self, root) -> usize(PR 0)groupsprivate, Stage 0 cannot setqueries; this adds a root and lists the query on what it reaches.stages::hash_consing:same_expression,expression_hash,same_valuetypes/src/stages/hash_consing.rsGroupProperties::shareable_when_equal.add_parent,same_expression_and_logical_properties,Memo::add_queries_belowtypes/src/stages/candidates.rsparentsandqueriesup to date; find a duplicate candidate.GroupOfNode,group_ofstage0-memo/src/lib.rsscan,kll_state,quantile_readout,unordered_limit,project_first_column,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);scan,job_is,a_plus_b_above_10,filter,range,kll,merge,kll_of_last_minute_where,coverage,kll_unlifted,jobs,last_minute(memo_summary_coverage.rs)candidatebuildsLogicalPropertiesdirectly, because the doc example's k = 200 and k = 345 are not in today's KLL list.Deleted (earlier revisions):
MemoGroup::logical_node,MemoGroup::child_groups,MemoGroup::expression(),Memo::indexed_group_count, and the previous revision of this PR'sMemo::logical_node_trees,logical_node_treeandnode_over_groups(rebuilt node trees; the rules now read groups throughMemoGroupAsInput). The input groups areexpression.children().How
derive_group_properties(PR 0) maps each input group id to aMemoGroupAsInput { memo, group_id }and runs the generic rules on the expression:output_schema,validate_inputs,output_kind,is_shareable_when_equal, andSummaryCoverage::derive_from_inputs(an error rejects aSummaryMerge; any other operator without coverage getsNone). An input is read through its storedgroup_propertiesand itsexpression; nothing is rebuilt.find_or_add_groupderives the properties, then hashes the operator's arguments (as JSON, input groups replaced by()) together with the input group ids, and compares the groups in that bucket withsame_expression. Only shareable expressions are looked up and indexed. On a miss it pushes the group with itsgroup_properties, and adds it to its inputs'parents.add_candidateruns the node rules on the candidate and compares the output with the group's before touching the memo. It then scans the group's own candidates for a duplicate; a group has few candidates. On a miss it appends the candidate, callsadd_parentper input group, and walks downward from the inputs with a worklist, inserting 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.group_ofmaps each originalRc<OperatorNode>by address to its group, so a diamond is visited once. It interns the children first, then callsfind_or_add_groupon the node's operator over their groups.add_querythen adds each root in workload order, so every group's query list comes out ascending.Major algorithm changes
Stage 0 hash-consing.
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 addingSummaryAgg[KLL](G1)could fail to find Stage 0's equal group whenever the two disagreed, for example on the guarantee.share_common_sub_dagsinterns nodes bottom-up into anRctable (SipHash of the node's JSON, thensame_node).numberassigns group ids.pairmaps the original nodes to the groups.group_ofcallsMemo::find_or_add_group(operator over child groups)bottom-up.add_queryis called per root.Memotests and 7 coverage tests below.O(its arguments), since its children areu32ids. The rules read only the input groups' stored properties and, where a rule looks below an input (aLIMITover aSort, a merge of a chain, the coverage walk throughFilter/TimeRange/TimeShift/Project), their expressions:O(arguments + the few groups such a rule walks)per call,O(N)for Stage 0 overNnodes. Coverage is derived once per group and a merge reads its inputs' stored coverage.Key code interfaces
Bodies are elided as
;. The signatures are PR 0's; this PR fills in their bodies.crates/types/src/stages/candidates.rscrates/types/src/stages/memo.rscrates/stage0-memo/src/lib.rsUsage
Concepts doc, section 1, G1 and G2. Planning past Stage 0 still stops at PR 3's
todo!(), so this flow is the onememo_tests::kll_builds_with_k_200_and_k_345_share_one_group,add_candidate_maintains_parents_and_queriesand Stage 0's tests run.kll_buildandreadout_candidatestand for the tests'candidate(operator, θ)helper.Before / After
KLL example (doc section 1): G1 =
Scan latency[1m], G2 =SummaryAgg[KLL](G1)with k = 200 and k = 345.memo_of_queriespanics (todo!)find_or_add_group; queries listed byadd_queryfind_or_add_grouppanicsgroup_properties(state schema, coverage); the second call returns G2add_candidatepanicsindex_in_group0 and 1Err(OutputDiffersFromGroup { group: G2 }), memo unchangedcse::same_node(deleted in PR 0)find_or_add_group's expression equality, the only oneTest status
New tests, each stating the behavior it checks in its doc line:
asap-types,stages::candidates::memo_tests(10):find_or_add_group_returns_the_group_of_an_equal_expressionfind_or_add_group_separates_equal_operators_over_different_inputsfind_or_add_group_never_returns_an_unshareable_groupfind_or_add_group_lists_the_new_group_among_its_inputs_parentsfind_or_add_group_returns_an_error_for_an_expression_with_no_schema(and adds nothing)kll_builds_with_k_200_and_k_345_share_one_groupadd_candidate_returns_the_existing_candidate_for_a_duplicateadd_candidate_maintains_parents_and_queries(alsoadd_query)add_candidate_rejects_a_candidate_whose_output_differs_from_its_group(memo unchanged)add_candidate_rejects_a_candidate_whose_inputs_fail_validation: a readout of raw rows, whose schema derives and which onlyvalidate_inputsrejects; memo unchanged.asap-stage0-memo(13):two_queries_sharing_a_sub_expression_get_one_group(also checks parents and queries)a_query_s_repeated_sub_expression_is_one_groupan_already_shared_node_is_one_groupaggregates_that_differ_in_an_argument_are_different_groupsequal_keyless_nodes_that_return_the_same_rows_are_one_groupdiffering_accuracy_targets_are_two_groups_over_one_scan_groupa_scalar_referenced_vector_is_one_group_across_queriesreadouts_share_their_state_group_but_not_each_otherdedup_is_one_group_only_on_every_columnunordered_limits_and_their_parents_are_different_groupslimits_over_a_sort_with_their_partitions_are_one_groupsigned_zero_and_non_finite_literals_are_different_groupsmemo_of_queries_returns_the_error_of_a_node_that_breaks_the_node_rulesasap-types,types/tests/memo_summary_coverage.rs(8), onasap-primitive-schema.md§4.2.2's memo (G1Scan latency; G4/G7/G10 KLL over the last minute ofjob = "api",job = "web",job = "api" ∧ a + b > 10):group_properties_come_from_input_groups: scan schema, state kind and column, readout value column,shareable_when_equal(false for an unorderedLIMIT), coverage only on the state.a_summary_agg_over_a_job_filter_lifts_it_into_the_selection: G4 hasinput_group: Some(G1),unlifted_operators: [SummaryAgg[KLL]], selectionjob ∈ {api}, time(−60 s, 0].states_with_equal_definitions_and_disjoint_selections_merge: G4 and G7 have one definition anddefinition_id;SummaryMerge(G4, G7)is a group coveringjob ∈ {api, web}.states_with_overlapping_selections_do_not_merge:SummaryMerge(G4, G4)isInputsOverlap { Disjoint }; no group is added.a_filter_that_is_no_selection_stays_unlifted: G10 keepsFilter(a + b > 10)inunlifted_operators;SummaryMerge(G4, G10)isDefinitionMismatch.states_whose_definitions_read_the_same_subquery_merge: two states overFilter(job = … ∧ a > (SELECT threshold))keepFilter(a > subquery(Some(threshold)), None)inunlifted_operators; their definitions are equal, their selections (job ∈ {api},job ∈ {web}) disjoint, andSummaryMergeof them coversjob ∈ {api, web}.scan_predicates_lift_and_their_states_merge: PromQL matchers in theScan(latency{job="api"},latency{job="web"}) are lifted (input_group: None, the bareScanlast); their states merge, as before Tracking: implement the optimization formulation (memo candidates, properties, edges y_{u,c,i}, MILP) as named code per stage #1207.a_tumbling_window_chain_covers_its_whole_window: a 5 × 1 min chain over G1 has the coverage ofKLL(TimeRange[5m](G1)).delivers_on_follows_the_alignment_tablebuilds its memo throughfind_or_add_groupandadd_candidatesincegroupsbecame private. It fails on PR 1 and passes here.None of these tests depends on another stage crate;
asap-stage0-memohas no dev-dependencies.cargo test -p <crate> --all-features --no-fail-fastat the head (e710906). Every failure panics at a later PR'stodo!()(or is the stale fixture, PR 9). Planning now passes Stage 0 and stops at Pass 1's target discovery (PR 3).asap_plannersubprocess that stops at atodo!(): 2; label a group by its expression (PR 9): 1Checks:
cargo check --workspace --all-targets,cargo clippy --workspace --all-targets -D warningsandcargo fmt --all --checkare clean.