Skip to content

Commit a83fc91

Browse files
committed
feat(ir): require disjoint coverage for summary merges
1 parent 5499f24 commit a83fc91

3 files changed

Lines changed: 95 additions & 4 deletions

File tree

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

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -213,6 +213,29 @@ impl ASAPOp {
213213
}
214214
}
215215

216+
/// Derive joint observation coverage; unknown or overlapping inputs fail closed.
217+
pub fn merged_extent(
218+
&self,
219+
) -> Result<super::observation_extent::ObservationExtent, SchemaDerivationError> {
220+
let ASAPOp::SummaryMerge { children } = self else {
221+
return Err(SchemaDerivationError::InvalidScalarSignature(
222+
"coverage merge requires SummaryMerge".into(),
223+
));
224+
};
225+
let inputs = children
226+
.iter()
227+
.map(|child| {
228+
child.observation_extent.clone().ok_or_else(|| {
229+
SchemaDerivationError::InvalidScalarSignature(
230+
"summary merge requires known observation coverage".into(),
231+
)
232+
})
233+
})
234+
.collect::<Result<Vec<_>, _>>()?;
235+
super::observation_extent::ObservationExtent::merge_disjoint(&inputs)
236+
.map_err(|error| SchemaDerivationError::InvalidScalarSignature(error.to_string()))
237+
}
238+
216239
/// Output schema derived from the operator and its children. Summary
217240
/// planning may retain a more specific schema (evaluation column naming)
218241
/// through [`OperatorNode::with_schema`]; all structural metadata must
@@ -483,6 +506,7 @@ impl ASAPOp {
483506
));
484507
}
485508
}
509+
self.merged_extent()?;
486510
Ok(())
487511
}
488512
SummaryEstimate {

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

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,11 @@ impl OperatorNode {
110110
/// ASAP operator, ...).
111111
pub fn new(operator: Operator) -> Result<Self, SchemaDerivationError> {
112112
let schema = operator.output_schema()?;
113-
Ok(Self::with_schema(operator, schema))
113+
let mut node = Self::with_schema(operator, schema);
114+
if let Some(op @ ASAPOp::SummaryMerge { .. }) = node.asap() {
115+
node.observation_extent = Some(op.merged_extent()?);
116+
}
117+
Ok(node)
114118
}
115119

116120
/// Build a node with caller-supplied output names and qualifiers. For
@@ -320,6 +324,13 @@ impl OperatorNode {
320324
None if node.requires_coverage() => return Err(CoverageError::Missing.into()),
321325
None => {}
322326
}
327+
if let Some(op @ ASAPOp::SummaryMerge { .. }) = node.asap() {
328+
if node.observation_extent.as_ref() != Some(&op.merged_extent()?) {
329+
return Err(SchemaDerivationError::InvalidScalarSignature(
330+
"retained merge coverage disagrees with input union".into(),
331+
));
332+
}
333+
}
323334
node.operator.validate_inputs()?;
324335
if node.result_kind != node.operator.output_kind() {
325336
return Err(SchemaDerivationError::InvalidScalarSignature(

‎crates/types/tests/summary_merge_structure.rs‎

Lines changed: 59 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ fn state(k: u32) -> Rc<OperatorNode> {
1515
schema: Schema::new(vec![Field::plain("value", DataType::Float64, false)]),
1616
}))
1717
.unwrap();
18-
OperatorNode::new_shared(Operator::ASAP(ASAPOp::SummaryAgg {
18+
let summary = OperatorNode::new(Operator::ASAP(ASAPOp::SummaryAgg {
1919
child: scan,
2020
family: FieldDataType::Sketch(
2121
SketchKind::new(SketchAlgorithm::Kll, SketchParams::Kll { k }),
@@ -26,17 +26,38 @@ fn state(k: u32) -> Rc<OperatorNode> {
2626
grouping: GroupingStrategy::default(),
2727
filter: None,
2828
}))
29-
.unwrap()
29+
.unwrap();
30+
std::rc::Rc::new(
31+
summary
32+
.with_observation_extent(asap_types::ir::observation_extent::ObservationExtent {
33+
source: "latencies:timestamp".into(),
34+
revision: "fixture-1".into(),
35+
input: SummaryUpdate::column(ColumnRef::SampleValue),
36+
grouping: Reduction::by(vec![]),
37+
multiplicity:
38+
asap_types::ir::observation_extent::ObservationMultiplicity::OncePerObservation,
39+
regions: vec![asap_types::ir::observation_extent::ExtentRegion {
40+
start_ms: 0,
41+
end_ms: 1,
42+
population: Default::default(),
43+
}],
44+
})
45+
.unwrap(),
46+
)
3047
}
3148
/// Two KLL panes compose into one typed logical state without timing assignment.
3249
#[test]
3350
fn compatible_panes_merge_structurally() {
3451
let root = OperatorNode::new_shared(Operator::ASAP(ASAPOp::SummaryMerge {
35-
children: vec![state(200), state(200)],
52+
children: vec![state(200), shifted_state(200, 1, 2)],
3653
}))
3754
.unwrap();
3855
root.validate_structure().unwrap();
3956
assert_eq!(root.schema.fields.len(), 1);
57+
assert_eq!(
58+
root.observation_extent.as_ref().unwrap().regions[0].end_ms,
59+
2
60+
);
4061
}
4162
/// An empty merge, raw rows and differently sized state cannot masquerade as compatible panes.
4263
#[test]
@@ -51,3 +72,38 @@ fn incompatible_merge_inputs_fail() {
5172
);
5273
}
5374
}
75+
76+
fn shifted_state(k: u32, start: i64, end: i64) -> Rc<OperatorNode> {
77+
let mut node = (*state(k)).clone();
78+
let region = &mut node.observation_extent.as_mut().unwrap().regions[0];
79+
region.start_ms = start;
80+
region.end_ms = end;
81+
Rc::new(node)
82+
}
83+
/// Schema equality cannot authorize overlapping or unknown observation coverage.
84+
#[test]
85+
fn unsafe_coverage_merge_is_rejected() {
86+
let mut unknown = (*state(200)).clone();
87+
unknown.observation_extent = None;
88+
for children in [
89+
vec![state(200), state(200)],
90+
vec![state(200), Rc::new(unknown)],
91+
] {
92+
assert!(
93+
OperatorNode::new_shared(Operator::ASAP(ASAPOp::SummaryMerge { children })).is_err()
94+
);
95+
}
96+
}
97+
98+
/// Gapped time coverage remains disconnected, and forged output metadata is rejected.
99+
#[test]
100+
fn merge_derives_coverage_and_validates_retained_metadata() {
101+
let root = OperatorNode::new_shared(Operator::ASAP(ASAPOp::SummaryMerge {
102+
children: vec![state(200), shifted_state(200, 2, 3)],
103+
}))
104+
.unwrap();
105+
assert_eq!(root.observation_extent.as_ref().unwrap().regions.len(), 2);
106+
let mut forged = (*root).clone();
107+
forged.observation_extent.as_mut().unwrap().regions[0].end_ms = 2;
108+
assert!(Rc::new(forged).validate_structure().is_err());
109+
}

0 commit comments

Comments
 (0)