Conversation
zzylol
force-pushed
the
stack/528-12-latency
branch
from
October 3, 2026 04:08
ec66d76 to
cc8794b
Compare
zzylol
force-pushed
the
stack/528-13-summary-merge
branch
from
October 3, 2026 04:08
5f2b829 to
5391107
Compare
This was referenced Oct 3, 2026
zzylol
marked this pull request as draft
October 3, 2026 19:28
Contributor
Author
|
Parked as draft: PR priorities changed (see #528). Order is now (A) finish #511 operator sharing, (B) the #572 crate/module reorganization, (C) #509 end-to-end stages. This PR sits on the old 🤖 Generated with Claude Code |
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.
Problem: the window-composition rule needs a merge node, but
SummaryMergeis reserved#509 §Pass 2: ASAP-aware common-subexpression elimination defines the window-composition rule:
and the tumbling-window case: "A longer query window is answered by merging the tumbling windows it covers … a 5-min window evaluated every 1 min uses 1-min tumbling windows and merges exactly 5 of them." #509 Example 4, Pattern B then gives two physical plans for the same merge DAG: B1 stores the 1-min KLLs at ingestion time and merges the latest 5 at query time; B2 rebuilds all 5 from raw samples at query time and merges them.
#511 §1.1 Unified
Operatortype listsSummaryMerge { children: Vec<Rc<OperatorNode>> }under "Reserved operations; semantics and support require further design", and says "Reserved ASAP variants require further semantic and capability design before use." #511 §2.3 adds the timing constraint any such operator must keep: "ingestion-time work cannot depend on query-time results".Before this PR, the B1/B2 DAG could not be built:
This was so even though a wire payload and a native merge kernel already existed.
Scope. This PR covers the merge node of #509's window-composition rule, for state inputs that already exist in the DAG: schema derivation, input checks, timing validation at both phases, and export to the native compiler. It leaves out: generating window candidates automatically in Pass 2, building Exponential Histograms, coverage (which time range / population each input holds; added by #567/#560 on the new stack), and the other reserved operators (
SummarySubtract,SummaryDelete,SummaryJoin,Extension).Proposed method
SummaryMergestops being reserved and gets the same contracts as the other ASAP operators. All changes are in the unified IR (crates/types); no new runtime code.ASAPOp::validate_inputs, run during node construction):childrenmust not be empty.Plainfield (the state column). Plain grouping fields may accompany it.result_kind == State.schemamust equal the first child's schema. This covers state family, algorithm and parameters (KLLk=200vsk=300fails), grouping field positions and types, names, and schema metadata.output_schema): run the input checks, then return a clone of the first child's schema.output_kindis alreadyStateforSummaryMerge.produced_state): return thedtypeof the first child's non-plain field.is_unimplemented):SummaryMergeis removed from the reserved list.validate_asapinir/timing.rs): every child must haveprimitive == SummaryState. If the merge runs at ingestion time, every child must also run at ingestion time. A query-time merge may read ingestion-time (stored) or query-time (rebuilt) children. Violations returnIllegalChildDataState { edge: "SummaryMerge.children", child }.apply_lifecycle_timings+compile_post_asap_dagaccept the node, and the native compiler compiles it with the existing merge kernel.Pipeline position: the node is a Pass 2 (window-composition) building block. Its timing is assigned in physical materialization, and the timing check runs on the physical DAG.
Key code interfaces
crates/types/src/ir/asap.rscrates/types/src/ir/timing.rs(privatevalidate_asap, reached fromvalidate_defaultand lifecycle timing):Usage (from
crates/integration-tests/tests/planner_layering_merge.rs):Also changed: the doc comment on
ExecutionDataStateError::UnimplementedOperatorno longer listsSummaryMerge; a unit test insummary_maintenance_cost/model.rsthat usedSummaryMergeas its example of a reserved child now usesSummarySubtract.Fields
ASAPOp::SummaryMergechildrenVec<Rc<OperatorNode>>result_kind == State; all child schemas equal; the shared schema has exactly one non-PlainfieldOutput of a
SummaryMergenode:schemachildren[0].schemaresult_kindOperatorResultKind::Stateproduced_state()&dtypeof the first child's non-plain fieldSummaryState. Ingestion-time merge ⇒ children at ingestion time. Query-time merge ⇒ children at either phase.Methods:
SummaryMergeis_unimplemented()false. StilltrueforSummarySubtract,SummaryDelete,SummaryJoin,Extension.produced_state()output_schema()validate_inputs, then clones the first schemavalidate_inputs()SchemaDerivationError::InvalidScalarSignaturewith: "summary merge requires at least one state input", "summary merge requires exactly one state column", "SummaryMerge requires summary state as input, got …", or "summary merge inputs must have identical state and grouping schemas"ExecutionDataStateError::IllegalChildDataState { edge, child }(existing variant):edge = "SummaryMerge.children";childis the offending child'sExecutionDataState(primitive and timing).Examples
Five 1-min KLLs → one 5-min p99, rebuilt and materialized. Test
five_minute_quantile_merges_five_one_minute_statesincrates/integration-tests/tests/planner_layering_merge.rs(#509 Examples 3B / 4B):Scans (pane_0…pane_4), each feedingSummaryAggwith KLLk = 200onSampleValueand no grouping; oneSummaryMergeover the five;SummaryEstimatep99 on top. Each pane gets 20 rows, valuespane*20 + i, so the merged input is 0 … 99.validate_structure, export to wire,wire.validate(), nativecompile. Then two runs:evaluation_time_ms: 300_000).cut_candidateat the fiveSummaryAggnodes; run the precompute part at ingestion time (window[0, 300_000)), then feed the stored states to the query part.98.0.Contract tests.
crates/types/tests/summary_merge.rs:compatible_panes_merge_and_export: two KLLk=200states merge; output schema has 1 field; timed export validates;validate_defaultpasses at bothIngestionTimeandQueryTime, andplanned_data_state(...).primitive == SummaryState.incompatible_merge_inputs_fail: construction fails for each case below.childrenk=200statesk=200panes[]k=200+ KLLk=300Scan(the child of aSummaryAgg)IllegalChildDataState(rule; not in these tests)Structural compatibility does not prove the inputs cover disjoint time ranges or populations. Two overlapping panes with equal schemas are accepted here. #567/#560 add that check on the new stack.
Out of scope
SummarySubtract,SummaryDelete,SummaryJoin,Extensionstay reserved.Stack and validation
Legacy physical stack: … ← #553 ← #554 ← #555 ← #556 … · Base: #554 (
stack/528-12-latency) · Next: #556 · Tracker: #528. Continues the stack above #554, which is stacked on #543 through #551–#553.Validation: type and mapping suites (745 tests/doctests passed); the new native E2E test passed; formatting; Clippy for types, mapping and integration tests with warnings denied. The regression failed before the implementation and passes afterward.
🤖 Generated with Claude Code