Goal
Under #509, logical planning decides what to compute. Physical planning decides materialization, i.e. when: ingestion time or query time. On main, ExecutionTiming is still part of logical node payloads and of node identity. Move it to the physical side, then rename the post-ASAP DAG family to #509's names.
Blocked on
Implementation of #511 (unified operator graph: OperatorNode with common metadata, no KeepPreAsap wrappers). Do not start before #511 is implemented.
#511 places timing as common node metadata: OperatorNode { timing: Option<ExecutionTiming> }, left None on logical candidates and required on executable physical candidates (operator-sharing.md ~:65, :349, :358, :507-520). Settle with #511 whether timing is that field or a physical-side per-node map.
Current state (main @107ab28d)
- Defined:
ExecutionTiming / ExecutionDataState (types/src/post_asap/execution_data_state.rs:60-116). There are timing fields on BinaryOp, ValueOperation and SummaryMerge (expr.rs:140,151,263). It is serialized in PostAsapDagNode.output_state and in edge data_state (post_asap_dag.rs:93-114; POST_ASAP_DAG_WIRE_VERSION = 6).
- Stamped in Pass 1: about 10
QueryTime and 3 IngestionTime sites in replacement.rs. exact_composition.rs:199-205 maps placement to timing, which creates separate ValueOperationAtQueryTime / AtIngestionTime candidates.
- Read in logical validation and CSE:
validate_execution_data_states (execution_data_state.rs:288-590) uses timing to give nodes their meaning. For example, an ingestion-time BinaryOp must be identity-aligned single-column arithmetic, and checked division is query-time only. cse.rs includes timing in node identity.
- Overridden after lifecycle selection:
execution_timed_dag() (summary_maintenance_lifecycle.rs:236-300) re-stamps every node through PostAsapDag::with_execution_phases.
- Consumed by the physical planner: the cut frontier (
candidates.rs:96-123) and lowering (mod.rs:466,649; precompute.rs:228; promql_rows.rs:277).
Latent bug: under an Ephemeral lifecycle, a BinaryOp that Pass 1 validated as ingestion-time arithmetic is re-stamped QueryTime, so it is lowered differently from how it was validated.
Proposed (recommendations; revisit against #511)
PR1: move timing out (wire 6→7)
- Remove
timing from the logical payloads. Express the ingestion-time BinaryOp's meaning on the operator itself, e.g. an identity_aligned flag on BinaryOperator, and use it in both validation and lowering. This also fixes the latent bug; add a test for it.
- Keep the maintenance and read variants only as different graph shapes, with no timing label.
- Have
execution_timed_dag return the phase map it already builds (BTreeMap<NodeId, ExecutionTiming>). compile, frontier_from_timing and precompute take that map. Delete with_execution_phases, and move its query-feeds-ingestion check into frontier_from_timing.
- Drop timing from CSE identity, so sharing ignores placement.
- DAG node and edge states keep only
DataPrimitive.
Estimate: about 20 src and 15 test files, roughly 600-900 lines.
PR2: pure rename (no wire bump; serde does not write type names, and the dag-viewer kind contract checks only SummaryExpr/QueryExpr kinds)
| Current |
New |
PostAsapDag / PostAsapDagNode / PostAsapDagEdge / PostAsapDagDocument / PostAsapDagValidationError / PostAsapDagCompilation |
LogicalASAPDAG / …Node / …Edge / …Document / …ValidationError / …Compilation |
PostAsapNodeId / PostAsapNodeIdentityMap / PostAsapOperatorPayload |
LogicalNodeId / LogicalNodeIdentityMap / LogicalOperatorPayload |
compile_post_asap_dag[_with_node_ids], POST_ASAP_DAG_WIRE_VERSION |
compile_logical_asap_dag[_with_node_ids], LOGICAL_ASAP_DAG_WIRE_VERSION |
About 41 files. This follows #514, which renamed PlanSpace → CandidateLogicalASAPDAGs and PhysicalCandidate → PhysicalASAPDAG.
Open questions
Goal
Under #509, logical planning decides what to compute. Physical planning decides materialization, i.e. when: ingestion time or query time. On main,
ExecutionTimingis still part of logical node payloads and of node identity. Move it to the physical side, then rename the post-ASAP DAG family to #509's names.Blocked on
Implementation of #511 (unified operator graph:
OperatorNodewith common metadata, noKeepPreAsapwrappers). Do not start before #511 is implemented.#511 places timing as common node metadata:
OperatorNode { timing: Option<ExecutionTiming> }, leftNoneon logical candidates and required on executable physical candidates (operator-sharing.md~:65, :349, :358, :507-520). Settle with #511 whether timing is that field or a physical-side per-node map.Current state (main @107ab28d)
ExecutionTiming/ExecutionDataState(types/src/post_asap/execution_data_state.rs:60-116). There aretimingfields onBinaryOp,ValueOperationandSummaryMerge(expr.rs:140,151,263). It is serialized inPostAsapDagNode.output_stateand in edgedata_state(post_asap_dag.rs:93-114;POST_ASAP_DAG_WIRE_VERSION = 6).QueryTimeand 3IngestionTimesites inreplacement.rs.exact_composition.rs:199-205maps placement to timing, which creates separateValueOperationAtQueryTime/AtIngestionTimecandidates.validate_execution_data_states(execution_data_state.rs:288-590) uses timing to give nodes their meaning. For example, an ingestion-time BinaryOp must be identity-aligned single-column arithmetic, and checked division is query-time only.cse.rsincludes timing in node identity.execution_timed_dag()(summary_maintenance_lifecycle.rs:236-300) re-stamps every node throughPostAsapDag::with_execution_phases.candidates.rs:96-123) and lowering (mod.rs:466,649;precompute.rs:228;promql_rows.rs:277).Latent bug: under an
Ephemerallifecycle, a BinaryOp that Pass 1 validated as ingestion-time arithmetic is re-stampedQueryTime, so it is lowered differently from how it was validated.Proposed (recommendations; revisit against #511)
PR1: move timing out (wire 6→7)
timingfrom the logical payloads. Express the ingestion-time BinaryOp's meaning on the operator itself, e.g. anidentity_alignedflag onBinaryOperator, and use it in both validation and lowering. This also fixes the latent bug; add a test for it.execution_timed_dagreturn the phase map it already builds (BTreeMap<NodeId, ExecutionTiming>).compile,frontier_from_timingand precompute take that map. Deletewith_execution_phases, and move its query-feeds-ingestion check intofrontier_from_timing.DataPrimitive.Estimate: about 20 src and 15 test files, roughly 600-900 lines.
PR2: pure rename (no wire bump; serde does not write type names, and the dag-viewer kind contract checks only
SummaryExpr/QueryExprkinds)PostAsapDag/PostAsapDagNode/PostAsapDagEdge/PostAsapDagDocument/PostAsapDagValidationError/PostAsapDagCompilationLogicalASAPDAG/…Node/…Edge/…Document/…ValidationError/…CompilationPostAsapNodeId/PostAsapNodeIdentityMap/PostAsapOperatorPayloadLogicalNodeId/LogicalNodeIdentityMap/LogicalOperatorPayloadcompile_post_asap_dag[_with_node_ids],POST_ASAP_DAG_WIRE_VERSIONcompile_logical_asap_dag[_with_node_ids],LOGICAL_ASAP_DAG_WIRE_VERSIONAbout 41 files. This follows #514, which renamed
PlanSpace→CandidateLogicalASAPDAGsandPhysicalCandidate→PhysicalASAPDAG.Open questions
Optionfield set by physical planning?QueryLifecyclePlan?