Repository navigation
refactor(types, stages): physical properties off the logical IR (OperatorNode has no timing or lifetime) - #1223
Closed
zzylol wants to merge 2 commits into
Conversation
OperatorNode has no timing and no lifetime: a logical candidate has only its logical properties (#1200), so frontends, Stage 0, Stage 1 and accuracy propagation cannot see physical properties. A node's (timing, lifetime) lives only in PhysicalPropertiesAssignment, keyed by node: Stage 2's choice in, the timing pass's completed assignment out. - apply_physical_properties[_to_roots] return the completed assignment instead of rebuilding timed copies; NodeToTimedNodeCache is deleted. - Export, tumbling-window expansion and with_confirmed_weights take or carry the assignment; PhysicalWorkloadPlan, PlanOutput and WholePlanSelection hold the selected plan's physical_properties, which replace the properties pinned on Stage 3's selected nodes. - An entry with lifetime None fixes a node's timing (what a pin did); the pin conflict and invalid-pin errors are deleted, as are the Stage 0/1/2 comparisons, copies and checks of the two fields. - Logical FlatDag nodes lose their timing/lifetime keys; the DAG viewer and its fixtures change with them. Wire format unchanged (INSTALLABLE_PLAN_WIRE_VERSION 12); corpus and sketch-plan snapshots unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01K1M6br9PL4SDeVS1XpezKJ
Docs: the Pass 1 module doc, post-asap-ir.md and asap-primitive-schema.md no longer describe OperatorNode::timing; operator-sharing.md gets a status note. The facade carries the plan to run as one SelectedPlan (roots + physical properties) and leaves the RealizedPlan as Stage 2 built it; a debug check asserts the assignment lists every node of the roots. bounds.rs and tumbling_window_chain.rs use copy_assignment. Assignment variables are named physical_properties, not timed; the integration-tests helpers are default_physical_properties and physical_properties_at_ingestion, and the logical crate's unused copies are deleted. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01K1M6br9PL4SDeVS1XpezKJ
zzylol
force-pushed
the
refactor/1200-physical-properties-off-logical-ir
branch
from
November 5, 2026 20:21
6500daf to
5ef723d
Compare
Contributor
Author
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.
Stacked on #1214; part of #1200, tracked in #1207
Merge unit. This PR merges into main only together with #1207's steps 1a–6 (#1207, Order): the #1200 stack, #1203 2a and 2b, #1201 and #1204 PR 1.
Representation / rules / enumeration
OperatorNodehas notimingand nolifetime: a logical candidate has only its logical properties, so frontends, Stage 0, Stage 1 (Pass 1 and Pass 2) and accuracy propagation cannot see physical properties. The physical propertiesPhysicalPropertiesAssignment(node →PhysicalProperties): Stage 2's choice in, and the timing pass's completed result out;PhysicalCandidate.properties(unchanged);PhysicalWorkloadPlan.physical_properties,PlanOutput.physical_propertiesandWholePlanSelection.physical_properties(the selected plan's nodes);PhysicalASAPDAGNode { output_state.timing, lifetime }(unchanged wire fields).reuse.rs,scan_labels.rs,window_candidates.rs,state_groups.rs,shared_configuration_readers.rs) no longer compare or copy the two fields; they wereNoneon every logical node, so every comparison gives the same answer as before.(timing, lifetime)alternatives and Stage 3 picks among them. The timing pass places every node by the same rules; it now writes each node's properties into the completed assignment instead of into a rebuilt copy of the node.Why
#1200's formulation gives physical properties$(\tau, \ell)$ only to physical candidates; a logical candidate has $p^{\log}$ only. Before this PR every
OperatorNodecarriedtiming: Option<ExecutionTiming>andlifetime: Lifetime, so logical code had to compare, copy and reject them: Stage 0 CSE hashed them, five Pass 2 rules compared or copied them, Pass 1 rejected an input that had them (AssignedPhysicalAnnotation), and Stage 2 rejected a memo node that had them (AssignedExecution, "assigned execution timing or lifetime"). Physical code also used the fields as a back door: a plan Stage 3 selected left its properties on the nodes ("pins"), and the timing pass honored a pre-set timing, so the same pair was stored on the node and in the assignment and could disagree (PhysicalPropertiesConflictWithFixedTiming,InvalidPhysicalPropertieson a node).What
OperatorNodelosestiming,lifetime,with_timing,with_lifetimeandphysical_properties().PhysicalPropertiesAssignmentis the one place a node's physical properties live in IR form. An entry may have any validPhysicalProperties: a materialized lifetime keeps the node's output (as before); lifetimeNonefixes only its timing (what a pin did).getis public.apply_physical_properties(root, &assignment)/apply_physical_properties_to_roots(roots, &assignment)return the completed assignment: every reachable node with itsPhysicalProperties. Nodes are no longer rebuilt, so node identity is kept andNodeToTimedNodeCachegoes away.compile_physical_asap_dag(root, &timed),compile_physical_asap_workload(roots, &timed); a node it does not list isUntimedNode, as an untimed node was.TumblingWindowChainExpansion::physical_properties;with_confirmed_weights[_at]give a rebuilt node its original's entry;split_shared_by_timingalready did (copy_assignment).PhysicalWorkloadPlan.physical_properties(Stage 2 / realizer),PlanOutput.physical_properties(facade),WholePlanSelection.physical_properties(reference pricer);installable_plan_oftakes them.PlanOutput::physical_asap_dagand installation re-apply the timing pass with them, which gives every node exactly the properties Stage 3 selected (what the pins did).PlanningRun::selected_plan: Option<SelectedPlan>withSelectedPlan { roots, physical_properties }(the roots with Stage 3's weight proofs and their assignment), and leaves theRealizedPlanas Stage 2 built it. In debug buildsStage1ToStage3Passasserts that the assignment lists every node of the roots (PhysicalPropertiesAssignment::lists_every_node_of), so a rewrite that drops an entry cannot silently fall back to the timing pass's default.plan.physical_propertiesinstead of on the node.physical_planner::logical::restoreno longer writestiming/lifetimeinto the IR it rebuilds (nothing read them).FlatDagnodes (the devtools viewer documents' logical DAGs,stage0, logical candidates, Stage 2 pruned operator labels) lose theirtiming: nullandlifetime: "None"keys. The DAG viewer (stages.js) no longer reads them from logical nodes, and its two hand-written memo fixtures drop them.pre-asap-ir.md,post-asap-ir.md,library-api.mddescribe the assignment as the home of physical properties.Side map, not a wrapper node type
A node's properties are kept in a side map keyed by node,
PhysicalPropertiesAssignment, not in a physical node type wrappingOperatorNode. A wrapper would either need its own child references (a second DAG type with its ownmap_children, flattening, export and expansion code) or still look its children's properties up in a map. The side map already existed as Stage 2's assignment, keepsRcidentity (no node is rebuilt to be timed), and encodes each node's pair once: the timing pass's output is the same type as its input, with every node listed. Re-applying a completed assignment is the identity, which replaces the pins.Concepts not in the formulation
PhysicalPropertiesAssignmentthat lists every node reachable from the roots (the timing pass's output). The formulation'sNonefixes a node's timing:Formulation notation → code
PhysicalPropertiesAssignment::get(node) -> Option<PhysicalProperties>OperatorNode::{timing, lifetime}/OperatorNode::physical_properties()PhysicalProperties::not_materialized_at(timing)PhysicalProperties::new(τ, Lifetime::None)PhysicalWorkloadPlan::physical_properties,PlanOutput::physical_properties,WholePlanSelection::physical_propertiesNew abstractions not in #1207 or the doc: decisions (2026-11-05)
Per #1207's interface policy; the user decided each row on 2026-11-05. #1207's steps 1a–6 are one merge unit, so an interim abstraction may stay only when a later PR of the unit deletes it.
PhysicalWorkloadPlan::physical_properties,PlanOutput::physical_properties,WholePlanSelection::physical_properties,TumblingWindowChainExpansion::physical_propertiesPhysicalPropertiesAssignmentwith the planSelectedPlan { roots, physical_properties },SelectedPlan::ofPhysicalPropertiesAssignment::get(nowpub),impl PartialEq,copy_assignmentWholePlanSelection's derive; carry an entry to a rebuilt nodePhysicalPropertiesAssignment::lists_every_node_ofPhysicalProperties::not_materialized_at(τ)PhysicalProperties::new(τ, Lifetime::None)default_physical_properties,physical_properties_at_ingestionSignatures that now take the assignment (not new names):
data_state(node, &assignment),with_confirmed_weights(.., &mut assignment),compile_physical_asap_dag[_with_node_ids](.., &assignment),compile_physical_asap_workload(.., &assignment).Divergences from the doc's section 3: none in what this PR changes; the doc's "logical stages carry no physical properties" now holds for
OperatorNode.How
The timing pass (
crates/types/src/ir/properties/timing.rs) keeps its placement rules.placementnow returns aPhysicalProperties: a node the assignment lists takes its entry (after the same filtered-build and upstream-at-query-time checks the materialized entries had; aSummaryEstimate/EvaluatePopulationentry at ingestion time isUpstreamNotAtIngestion), any other node is(QueryTime, None)for a summary build,SummaryEstimateorEvaluatePopulation, and(consumer, None)otherwise.forces_query_timereads entries the same way.writerecords each node in the output assignment (a node reached at two timings is stillConflictingTiming) andvalidatereads each child's timing from it. The edge rules are unchanged.Every former pin is an assignment entry: Stage 3's selection reaches export as
PlanOutput.physical_properties; tests that pinned a node at ingestion time (placed_at_ingestion,with_timing(Some(..))) now setPhysicalProperties::NOT_MATERIALIZED_AT_INGESTIONfor it. Tests that only checked the pins' conflicts or invalid pins are deleted (see Deleted): those states cannot be represented any more.Key code interfaces added / modified
crates/types/src/ir/operator/node.rscrates/types/src/ir/properties/timing.rscrates/types/src/ir/properties/execution_data_state.rscrates/types/src/ir/physical_export.rscrates/types/src/ir/flat.rscrates/types/src/ir/operator/tumbling_window_chain.rscrates/types/src/physical/properties.rs,crates/types/src/physical/workload_plan.rs,crates/types/src/workload/bounds.rscrates/planner/src/pass.rs,crates/planner/src/installable.rs,crates/plan-selection/src/report.rsLogical-stage errors removed:
PerComputationCandidateError::AssignedPhysicalAnnotation(Pass 1),ConfigurationError::AssignedExecutionand the inventory's "assigned execution timing or lifetime" rejection (Stage 2): a logical node cannot carry physical properties any more.Usage (the facade, unchanged for a caller; what changed is where Stage 3's choice travels):
crates/integration-tests/tests/runtime_handoff.rschecks this flow: the facade's DAG equalscompile_physical_asap_workload(&output.roots(), &output.physical_properties).Major algorithm changes
None. The timing pass's placement and edge rules are unchanged; it records each node's properties in the completed assignment instead of a rebuilt node, so it no longer allocates a copy of the DAG.
Deleted
OperatorNode::{timing, lifetime},OperatorNode::{with_timing, with_lifetime, physical_properties},OperatorNode::validate_execution_timing(test-support) and its testexecution_timing_checks_edges_not_function_names.FlatNode::{timing, lifetime}(thetiming/lifetimekeys of logical DAG documents).NodeToTimedNodeCache,NodeToTimedNodeCache::timed;asap_types::test_support::timed; the logical crate'stest_support::{timed, at_ingestion}(the integration tests' fixtures are nowdefault_physical_propertiesandphysical_properties_at_ingestioninintegration-tests/src/lib.rs).ExecutionDataStateError::PhysicalPropertiesConflictWithFixedTiming,ExecutionDataStateError::InvalidPhysicalProperties; timing.rs'stimed_propertiesandPhysicalPropertiesAssignment::resolve; thechild.timing.is_some()check underEvaluatePopulation(always true after timing).PerComputationCandidateError::AssignedPhysicalAnnotationand its test cases;ConfigurationError::AssignedExecution; the inventory's assigned-timing rejection; Stage 2's "a node fixed at ingestion time cannot be built at query time" filter on shared results.cse.rs(hash, equality, interning),pass2/memo/rules/reuse.rs,rules/scan_labels.rs,window_candidates.rs,state_groups.rs,rules/shared_configuration_readers.rs(including its skip of an "already assigned" build),catalog/factoring.rs,split_by_timing.rs, accuracypropagation.rs(bind_guarantees,with_resized_summaries), plan-selectionaccuracy_positions.rs,workload/bounds.rs,schema_support.rs, the executor'slogical::restore;tumbling_window_chain.rs'stimed_like.fixed_timing_keeps_an_agreeing_assignment_and_rejects_a_conflict(its first half kept asan_entry_gives_a_node_its_timing_and_lifetime),ingestion_fixed_node_rejects_mat_at_query,an_invalid_timed_node_reports_its_error,a_summary_build_over_an_invalid_input_reports_its_error,a_non_asap_node_over_an_invalid_input_reports_its_errorand helpersplaced_at_ingestion,invalid_at_ingestion; split_by_timinga_fixed_timing_conflict_survives_the_split(and its helpersfinalize,exact_sum); summary_coveragetiming_does_not_block_merges; runtime_install'suntime; weighted_topk_binding'sunpinned.PhysicalASAPDAG::untimed_query_roots→query_roots.Before this PR
Every operator node, logical or physical, carried
timingandlifetime. Logical stages carriedNone/Lifetime::Nonethrough every rewrite and compared them in CSE and the sharing rules; Pass 1 and Stage 2 rejected inputs that had them. The timing pass rebuilt the whole DAG to write the pair into each node, and honored a pair already on a node, so Stage 3's selection reached export as pins on the nodes and a pin could conflict with an assignment entry.After this PR
A logical node has no physical properties; only physical types hold them, each node's pair encoded once in a
PhysicalPropertiesAssignment. The timing pass keeps the input nodes and returns the completed assignment; export, installation, cost and Stage 2's realizer read it. Stage 3's selection reaches export throughPlanOutput.physical_properties.Wire format: unchanged.
INSTALLABLE_PLAN_WIRE_VERSIONstays 12;PhysicalASAPDAGNodekeepsoutput_state.timingandlifetime. Only the developer-tool viewer documents change: logical DAG nodes no longer havetiming/lifetimekeys (alwaysnull/"None"before); the viewer and its fixtures change with them.Plans: unchanged.
UPDATE_SNAPSHOTS=1 cargo test --release -p asap-devtools --test corpus_snapshots -- --include-ignoredleavesgit statuswithout snapshot changes.Verification
Speed mode: tests covering the changed code, plus fmt and clippy.
5ef723d9c, rebased on refactor(types, stages): replace Materialization with PhysicalProperties (timing, lifetime) #121464e721439):cargo fmt --all -- --checkandcargo clippy --workspace --all-targets -- -D warningsclean;cargo test -p asap-types(253),-p asap-cost(45),-p asap-accuracy(77),-p asap-logical-asap-optimization(203),-p asap-frontend-promql(228),-p asap-frontend-sql(176),-p asap-physical-asap-optimization(28),-p asap-executor(296),-p asap-planner(89, 8 wall-clock ignored),-p asap-plan-selection --lib(142),-p asap-devtools(108),-p asap-integration-tests(364): 0 failed. The first run's one failure,range_evaluation::stage3_plan_selection_prices_61_evaluations_per_dashboard_request, came from refactor(types, stages): replace Materialization with PhysicalProperties (timing, lifetime) #1214's detail-text rename and is fixed there (rerun after the rebase: ok). The review fixes also undo two bad renames from thetimed→physical_propertiessweep (a rebuilt node in accuracypropagation.rs, a boolean inruntime_install.rs).Earlier round (at
6500daf19):cargo fmt --all -- --check: clean.cargo clippy --workspace --all-targets -- -D warnings: clean.cargo test -p asap-types(143 + 101 + 8 passed),-p asap-cost(36 + 1 + 8 passed),-p asap-accuracy(77 passed),-p asap-logical-asap-optimization --lib(201 passed),-p asap-physical-asap-optimization(28 passed),-p asap-plan-selection --lib(142 passed),-p asap-executor(all targets passed; 2 ignored),-p asap-planner(11 + 78 passed; 8 ignored),-p asap-devtools --lib(18 passed),-p asap-devtools --test devtools(82 passed; 1 ignored),-p asap-integration-tests(364 passed; 26 ignored). Ignored tests are the wall-clock / time-budget ones.node --test stages.test.mjs memo.test.mjs enumeration.test.mjs(8 + 13 + 12 passed) with the updated fixtures.UPDATE_SNAPSHOTS=1 cargo test --release -p asap-devtools --test corpus_snapshots -- --include-ignored(9 passed);git statusshows no snapshot change, so every corpus and sketch-plan snapshot selects the same plan.PhysicalPropertiesAssignmententries (fixed_at_ingestion,apply_fixing) and expect the same errors and timings;runtime_handoffchecks that the facade's DAG equals the selected plan compiled withPlanOutput.physical_properties.🤖 Generated with Claude Code
https://claude.ai/code/session_01K1M6br9PL4SDeVS1XpezKJ