Skip to content

Commit 5dfe095

Browse files
zzylolclaude
andcommitted
feat(ir): check merge update/reduction on producers; test documented coverage examples
SummaryCoverage no longer repeats input/reduction, so SummaryMerge compares them through OperatorNode::summary_update. summary_coverage_examples.rs builds each example in docs/develop_docs/summary-coverage.md as a SummaryAgg -> SummaryMerge plan. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent ef19aea commit 5dfe095

4 files changed

Lines changed: 289 additions & 3 deletions

File tree

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -498,6 +498,14 @@ impl ASAPOp {
498498
));
499499
}
500500
}
501+
// Coverage records only time and population; what each state
502+
// summarizes and how it is grouped come from the producers.
503+
let update = first.summary_update();
504+
if update.is_none() || children.iter().any(|c| c.summary_update() != update) {
505+
return Err(SchemaDerivationError::InvalidScalarSignature(
506+
"summary merge inputs must share update expression and reduction".into(),
507+
));
508+
}
501509
self.merged_coverage()?;
502510
Ok(())
503511
}

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,12 @@ use serde::{Deserialize, Serialize};
1010

1111
use super::asap::ASAPOp;
1212
use super::non_asap::NonASAPOp;
13+
use super::operator_properties::Reduction;
1314
use super::summary_coverage::{CoverageError, SummaryCoverage};
1415
use crate::ir::SchemaDerivationError;
1516
use crate::post_asap::execution_data_state::ExecutionTiming;
1617
use crate::post_asap::guarantee::ResultGuarantee;
18+
use crate::post_asap::SummaryUpdate;
1719
use crate::pre_asap::schema::Schema;
1820

1921
/// The output category of an operator, derived from the operation and its
@@ -163,6 +165,18 @@ impl OperatorNode {
163165
Ok(self)
164166
}
165167

168+
/// What a summary state is updated with and how it is grouped: the
169+
/// `SummaryAgg` fields, or those shared by a `SummaryMerge`'s inputs.
170+
pub fn summary_update(&self) -> Option<(&SummaryUpdate, &Reduction)> {
171+
match self.asap()? {
172+
ASAPOp::SummaryAgg {
173+
input, reduction, ..
174+
} => Some((input, reduction)),
175+
ASAPOp::SummaryMerge { children } => children.first()?.summary_update(),
176+
_ => None,
177+
}
178+
}
179+
166180
/// Summary nodes whose state can be composed must declare coverage.
167181
pub fn requires_coverage(&self) -> bool {
168182
matches!(
Lines changed: 264 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,264 @@
1+
//! The examples in docs/develop_docs/summary-coverage.md, built as real
2+
//! SummaryAgg -> SummaryMerge plans. Every input has the same schema
3+
//! `(job: Utf8, state: KLL{k=200})`; only coverage differs.
4+
use asap_types::ir::summary_coverage::{CoverageError, CoverageRegion, SummaryCoverage};
5+
use asap_types::ir::{
6+
ASAPOp, ExprSemantics, NonASAPOp, Operator, OperatorNode, Predicate, ScalarExpr,
7+
SchemaDerivationError,
8+
};
9+
use asap_types::post_asap::{
10+
GroupingStrategy, SketchAlgorithm, SketchKind, SketchParams, SummaryUpdate,
11+
};
12+
use asap_types::pre_asap::{
13+
ColumnRef, CompareOpKind, DataType, Field, FieldDataType, Reduction, ScalarValue, Schema,
14+
Source,
15+
};
16+
use std::rc::Rc;
17+
18+
const MIN: i64 = 60_000;
19+
20+
fn requests() -> Source {
21+
Source::Table {
22+
table_ref: "requests".into(),
23+
}
24+
}
25+
26+
fn scan() -> Rc<OperatorNode> {
27+
OperatorNode::new_shared(Operator::NonASAP(NonASAPOp::Scan {
28+
source: requests(),
29+
predicates: vec![],
30+
schema: Schema::new(vec![
31+
Field::plain("job", DataType::Utf8, false),
32+
Field::plain("region", DataType::Utf8, false),
33+
Field::plain("tier", DataType::Utf8, false),
34+
Field::plain("latency", DataType::Float64, false),
35+
Field::plain("size", DataType::Float64, false),
36+
]),
37+
}))
38+
.unwrap()
39+
}
40+
41+
/// `region = value`, evaluated on the scan schema.
42+
fn region_is(value: &str) -> Predicate {
43+
Predicate(ScalarExpr::Compare {
44+
left: Box::new(ScalarExpr::Column(1)),
45+
op: CompareOpKind::Eq,
46+
right: Box::new(ScalarExpr::Literal(ScalarValue::Utf8(value.into()))),
47+
semantics: ExprSemantics::Sql,
48+
})
49+
}
50+
51+
fn region(time_ms: Option<std::ops::Range<i64>>, population: &[(&str, &str)]) -> CoverageRegion {
52+
CoverageRegion {
53+
time_ms,
54+
population: population
55+
.iter()
56+
.map(|(k, v)| (k.to_string(), v.to_string()))
57+
.collect(),
58+
}
59+
}
60+
61+
/// p99-ready KLL over `column`, grouped by job, with declared coverage.
62+
fn kll_over(
63+
column: &str,
64+
filter: Option<Predicate>,
65+
coverage: SummaryCoverage,
66+
) -> Rc<OperatorNode> {
67+
let node = OperatorNode::new(Operator::ASAP(ASAPOp::SummaryAgg {
68+
child: scan(),
69+
family: FieldDataType::Sketch(
70+
SketchKind::new(SketchAlgorithm::Kll, SketchParams::Kll { k: 200 }),
71+
GroupingStrategy::default(),
72+
),
73+
input: SummaryUpdate::column(ColumnRef::Named(column.into())),
74+
reduction: Reduction::by(vec![0]),
75+
grouping: GroupingStrategy::default(),
76+
filter,
77+
}))
78+
.unwrap();
79+
Rc::new(node.with_coverage(coverage).unwrap())
80+
}
81+
82+
fn kll(time_ms: Option<std::ops::Range<i64>>, population: &[(&str, &str)]) -> Rc<OperatorNode> {
83+
kll_over(
84+
"latency",
85+
None,
86+
SummaryCoverage {
87+
source: requests(),
88+
regions: vec![region(time_ms, population)],
89+
},
90+
)
91+
}
92+
93+
fn merge(children: Vec<Rc<OperatorNode>>) -> Result<Rc<OperatorNode>, SchemaDerivationError> {
94+
let schema = children[0].schema.clone();
95+
let merged = OperatorNode::new_shared(Operator::ASAP(ASAPOp::SummaryMerge { children }))?;
96+
merged.validate_structure()?;
97+
// Schema never changes; only coverage does.
98+
assert_eq!(merged.schema, schema);
99+
Ok(merged)
100+
}
101+
102+
fn regions(node: &OperatorNode) -> Vec<CoverageRegion> {
103+
node.coverage.as_ref().unwrap().regions.clone()
104+
}
105+
106+
fn rejected(result: Result<Rc<OperatorNode>, SchemaDerivationError>, expected: CoverageError) {
107+
match result {
108+
Err(SchemaDerivationError::Coverage(actual)) => assert_eq!(actual, expected),
109+
other => panic!("expected {expected:?}, got {other:?}"),
110+
}
111+
}
112+
113+
/// Example 1: adjacent panes coalesce, gaps stay, overlapping windows are rejected.
114+
#[test]
115+
fn example_1_time() {
116+
let adjacent = merge(vec![kll(Some(0..MIN), &[]), kll(Some(MIN..2 * MIN), &[])]).unwrap();
117+
assert_eq!(regions(&adjacent), vec![region(Some(0..2 * MIN), &[])]);
118+
119+
let gapped = merge(vec![
120+
kll(Some(0..MIN), &[]),
121+
kll(Some(2 * MIN..3 * MIN), &[]),
122+
])
123+
.unwrap();
124+
assert_eq!(
125+
regions(&gapped),
126+
vec![
127+
region(Some(0..MIN), &[]),
128+
region(Some(2 * MIN..3 * MIN), &[])
129+
]
130+
);
131+
132+
rejected(
133+
merge(vec![
134+
kll(Some(0..2 * MIN), &[]),
135+
kll(Some(MIN..3 * MIN), &[]),
136+
]),
137+
CoverageError::PossibleOverlap,
138+
);
139+
}
140+
141+
/// Example 2: disjoint label values merge; different labels or equal values are rejected.
142+
#[test]
143+
fn example_2_population() {
144+
let t = Some(0..MIN);
145+
let us_eu = merge(vec![
146+
kll(t.clone(), &[("region", "us")]),
147+
kll(t.clone(), &[("region", "eu")]),
148+
])
149+
.unwrap();
150+
assert_eq!(
151+
regions(&us_eu),
152+
vec![
153+
region(t.clone(), &[("region", "eu")]),
154+
region(t.clone(), &[("region", "us")]),
155+
]
156+
);
157+
for other in [("tier", "premium"), ("region", "us")] {
158+
rejected(
159+
merge(vec![
160+
kll(t.clone(), &[("region", "us")]),
161+
kll(t.clone(), &[other]),
162+
]),
163+
CoverageError::PossibleOverlap,
164+
);
165+
}
166+
}
167+
168+
/// Example 3: time and population stay paired; never widened to {us,eu} × [0,2).
169+
#[test]
170+
fn example_3_joint_regions() {
171+
let joint = merge(vec![
172+
kll(Some(0..MIN), &[("region", "us")]),
173+
kll(Some(MIN..2 * MIN), &[("region", "eu")]),
174+
])
175+
.unwrap();
176+
assert_eq!(
177+
regions(&joint),
178+
vec![
179+
region(Some(MIN..2 * MIN), &[("region", "eu")]),
180+
region(Some(0..MIN), &[("region", "us")]),
181+
]
182+
);
183+
}
184+
185+
/// A table without a time column declares no time bounds.
186+
#[test]
187+
fn tabular_source_without_time_bounds() {
188+
let by_region = merge(vec![
189+
kll(None, &[("region", "us")]),
190+
kll(None, &[("region", "eu")]),
191+
])
192+
.unwrap();
193+
assert_eq!(regions(&by_region).len(), 2);
194+
rejected(
195+
merge(vec![
196+
kll(None, &[("region", "us")]),
197+
kll(Some(0..MIN), &[("region", "us")]),
198+
]),
199+
CoverageError::PossibleOverlap,
200+
);
201+
}
202+
203+
/// Inputs must read the same source and share the producer's update and reduction.
204+
#[test]
205+
fn incompatible_inputs() {
206+
let other_source = kll_over(
207+
"latency",
208+
None,
209+
SummaryCoverage {
210+
source: Source::Table {
211+
table_ref: "other".into(),
212+
},
213+
regions: vec![region(Some(MIN..2 * MIN), &[])],
214+
},
215+
);
216+
rejected(
217+
merge(vec![kll(Some(0..MIN), &[]), other_source]),
218+
CoverageError::SourceMismatch,
219+
);
220+
221+
// Same schema (both Float64 columns), different update expression.
222+
let size = kll_over(
223+
"size",
224+
None,
225+
SummaryCoverage {
226+
source: requests(),
227+
regions: vec![region(Some(MIN..2 * MIN), &[])],
228+
},
229+
);
230+
assert!(matches!(
231+
merge(vec![kll(Some(0..MIN), &[]), size]),
232+
Err(SchemaDerivationError::InvalidScalarSignature(message))
233+
if message.contains("update expression and reduction")
234+
));
235+
}
236+
237+
/// Summary nodes must carry coverage, and merges reject inputs without it.
238+
#[test]
239+
fn coverage_is_required() {
240+
let mut missing = (*kll(Some(MIN..2 * MIN), &[])).clone();
241+
missing.coverage = None;
242+
let missing = Rc::new(missing);
243+
assert!(matches!(
244+
missing.validate_structure(),
245+
Err(SchemaDerivationError::Coverage(CoverageError::Missing))
246+
));
247+
rejected(
248+
merge(vec![kll(Some(0..MIN), &[]), missing]),
249+
CoverageError::UnknownInput,
250+
);
251+
}
252+
253+
/// Trusted declarations: population is not checked against the filter (#570).
254+
/// Both states hold US data, yet the wrong declaration lets them merge.
255+
#[test]
256+
fn wrong_population_declaration_is_accepted_until_570() {
257+
let declared = |value: &str| SummaryCoverage {
258+
source: requests(),
259+
regions: vec![region(Some(0..MIN), &[("region", value)])],
260+
};
261+
let a = kll_over("latency", Some(region_is("us")), declared("eu"));
262+
let b = kll_over("latency", Some(region_is("us")), declared("us"));
263+
assert!(merge(vec![a, b]).is_ok());
264+
}

‎crates/types/tests/summary_merge_structure.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,9 +30,9 @@ fn state(k: u32) -> Rc<OperatorNode> {
3030
std::rc::Rc::new(
3131
summary
3232
.with_coverage(asap_types::ir::summary_coverage::SummaryCoverage {
33-
source: "latencies:timestamp".into(),
34-
input: SummaryUpdate::column(ColumnRef::SampleValue),
35-
reduction: Reduction::by(vec![]),
33+
source: Source::Table {
34+
table_ref: "latencies".into(),
35+
},
3636
regions: vec![asap_types::ir::summary_coverage::CoverageRegion {
3737
time_ms: Some(0..1),
3838
population: Default::default(),

0 commit comments

Comments
 (0)