Skip to content

Commit c7353a4

Browse files
zzylolclaude
andcommitted
refactor(ir): follow derived summary coverage in timing and physical export
Coverage is derived from the node (#646), so timed copies no longer carry it and PhysicalASAPDAGNode drops its coverage field. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W7qG9aFyPij5uWsyAJCxDW
1 parent 40b7093 commit c7353a4

3 files changed

Lines changed: 17 additions & 36 deletions

File tree

‎crates/types/src/ir/physical_export.rs‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -67,8 +67,6 @@ pub struct PhysicalASAPDAGNode {
6767
pub output_state: ExecutionDataState,
6868
pub output_schema: Schema,
6969
pub guarantee: Option<ResultGuarantee>,
70-
#[serde(default)]
71-
pub coverage: Option<super::summary_coverage::SummaryCoverage>,
7270
}
7371

7472
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
@@ -413,7 +411,6 @@ pub fn compile_physical_asap_workload_with_node_ids(
413411
output_state,
414412
output_schema: flat_node.schema,
415413
guarantee: flat_node.guarantee,
416-
coverage: flat_node.coverage,
417414
});
418415
}
419416
let dag = PhysicalASAPDAG {

‎crates/types/src/ir/timing.rs‎

Lines changed: 10 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -264,15 +264,11 @@ fn write(
264264
if let Some(e) = error {
265265
return Err(e);
266266
}
267-
let timed = Rc::new(OperatorNode {
268-
operator,
269-
result_kind: node.result_kind,
270-
schema: node.schema.clone(),
271-
guarantee: node.guarantee.clone(),
272-
timing: Some(timing),
273-
// Timing copies the same logical sub-DAG; its observations are unchanged.
274-
coverage: node.coverage.clone(),
275-
});
267+
let timed = Rc::new(
268+
OperatorNode::with_schema(operator, node.schema.clone())
269+
.with_guarantee(node.guarantee.clone())
270+
.with_timing(Some(timing)),
271+
);
276272
memo.done.insert(Rc::as_ptr(node), Rc::clone(&timed));
277273
Ok(timed)
278274
}
@@ -618,14 +614,11 @@ pub fn split_shared_by_phase(
618614
rebuilt
619615
});
620616
let out = if changed {
621-
Rc::new(OperatorNode {
622-
operator,
623-
result_kind: node.result_kind,
624-
schema: node.schema.clone(),
625-
guarantee: node.guarantee.clone(),
626-
timing: node.timing,
627-
coverage: node.coverage.clone(),
628-
})
617+
Rc::new(
618+
OperatorNode::with_schema(operator, node.schema.clone())
619+
.with_guarantee(node.guarantee.clone())
620+
.with_timing(node.timing),
621+
)
629622
} else {
630623
Rc::clone(node)
631624
};

‎crates/types/tests/physical_export.rs‎

Lines changed: 7 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
1-
//! A timed plan exports as a PhysicalASAPDAG that keeps timing and coverage.
1+
//! A timed plan exports as a PhysicalASAPDAG that keeps timing.
22
use asap_types::ir::physical_export::{
33
compile_physical_asap_dag, PhysicalASAPDAGDocument, PhysicalASAPDAGValidationError,
44
};
5-
use asap_types::ir::summary_coverage::{CoverageRegion, SummaryCoverage};
65
use asap_types::ir::{
76
apply_lifecycle_timings, ASAPOp, LifecycleAssignment, NonASAPOp, Operator, OperatorNode,
87
TimingMemo,
@@ -13,11 +12,10 @@ use std::rc::Rc;
1312

1413
/// Scan(t) → SummaryAgg(sum by key) → FinalizeExactAccumulator, untimed.
1514
fn plan() -> Rc<OperatorNode> {
16-
let source = Source::Table {
17-
table_ref: "t".into(),
18-
};
1915
let scan = OperatorNode::new_shared(Operator::NonASAP(NonASAPOp::Scan {
20-
source: source.clone(),
16+
source: Source::Table {
17+
table_ref: "t".into(),
18+
},
2119
predicates: vec![],
2220
schema: Schema::new(vec![
2321
Field::plain("key", DataType::Utf8, false),
@@ -33,14 +31,6 @@ fn plan() -> Rc<OperatorNode> {
3331
grouping: Default::default(),
3432
filter: None,
3533
}))
36-
.unwrap()
37-
.with_coverage(SummaryCoverage {
38-
source,
39-
regions: vec![CoverageRegion {
40-
time_ms: Some(0..60_000),
41-
population: Default::default(),
42-
}],
43-
})
4434
.unwrap();
4535
OperatorNode::new_shared(Operator::ASAP(ASAPOp::FinalizeExactAccumulator {
4636
child: Rc::new(state),
@@ -50,7 +40,7 @@ fn plan() -> Rc<OperatorNode> {
5040

5141
/// Default lifecycle: the summary is maintained at ingestion time and read at query time.
5242
#[test]
53-
fn timed_plan_exports_with_timing_and_coverage() {
43+
fn timed_plan_exports_with_timing() {
5444
let timed = apply_lifecycle_timings(
5545
&plan(),
5646
&LifecycleAssignment::default_maintained(),
@@ -70,7 +60,8 @@ fn timed_plan_exports_with_timing_and_coverage() {
7060
ExecutionTiming::QueryTime
7161
]
7262
);
73-
assert!(dag.nodes[1].coverage.is_some());
63+
// Timing keeps what the summary covers.
64+
assert!(timed.children()[0].coverage().is_some());
7465

7566
let decoded: PhysicalASAPDAGDocument =
7667
serde_json::from_str(&serde_json::to_string(&document).unwrap()).unwrap();

0 commit comments

Comments
 (0)