Skip to content

Commit e699985

Browse files
zzylolclaude
andcommitted
feat(ir): derive summary coverage as definition + selection
Coverage is derived from the node, never declared. For a SummaryAgg, predicate conjuncts that lift through Filter, a range TimeRange, a TimeShift without @ and direct-column Project items, and are a value set or an interval on one column, form the selection; everything else stays in the definition (the SummaryAgg with the selection removed). A TimeRange(w) over TimeShift(s) gives the relative window (-(s+w), -s]. A SummaryMerge is valid only when its inputs have structurally equal definitions (ignoring timing and guarantee) and pairwise disjoint selections; OperatorNode::new and validate_structure enforce it. Its coverage is the shared definition and the union of the selections, joining adjacent windows and value sets. OperatorNode.coverage becomes a private cache behind coverage(): ignored by equality, skipped by serde, emptied on clone. with_coverage, requires_coverage and CoverageError::Missing are removed, as are the coverage fields of CSE keys and FlatNode. Design: docs/design_docs/proposals/asap-primitive-schema.md §4 (#573). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W7qG9aFyPij5uWsyAJCxDW
1 parent 0ebeb9d commit e699985

8 files changed

Lines changed: 1108 additions & 359 deletions

File tree

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

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,6 @@ pub fn structural_hash(node: &OperatorNode, cache: &mut HashCache) -> u64 {
112112
&node.schema,
113113
&node.guarantee,
114114
node.timing,
115-
&node.coverage,
116115
);
117116
serde_json::to_string(&own)
118117
.unwrap_or_default()
@@ -168,7 +167,6 @@ fn same_node(left: &OperatorNode, right: &OperatorNode, memo: &mut EqMemo) -> bo
168167
&& left.result_kind == right.result_kind
169168
&& left.schema == right.schema
170169
&& left.timing == right.timing
171-
&& left.coverage == right.coverage
172170
&& same_value(&left.guarantee, &right.guarantee)
173171
&& same_value(&own_fields(left), &own_fields(right))
174172
}
@@ -250,14 +248,9 @@ fn intern_bottom_up(
250248
let operator = node
251249
.operator
252250
.map_children(|child| intern_bottom_up(table, visited, child));
253-
let rebuilt = OperatorNode {
254-
operator,
255-
result_kind: node.result_kind,
256-
schema: node.schema.clone(),
257-
guarantee: node.guarantee.clone(),
258-
timing: node.timing,
259-
coverage: node.coverage.clone(),
260-
};
251+
let rebuilt = OperatorNode::with_schema(operator, node.schema.clone())
252+
.with_guarantee(node.guarantee.clone())
253+
.with_timing(node.timing);
261254
let interned = table.intern(rebuilt);
262255
visited.insert(Rc::as_ptr(node), (Rc::clone(node), Rc::clone(&interned)));
263256
interned

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

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ use serde::{Deserialize, Serialize};
1515

1616
use super::node::{Operator, OperatorNode, OperatorResultKind};
1717
use super::query::QueryRoot;
18-
use super::summary_coverage::SummaryCoverage;
1918
use crate::post_asap::execution_data_state::ExecutionTiming;
2019
use crate::post_asap::guarantee::ResultGuarantee;
2120
use crate::pre_asap::schema::Schema;
@@ -31,8 +30,6 @@ pub struct FlatNode {
3130
pub schema: Schema,
3231
pub guarantee: Option<ResultGuarantee>,
3332
pub timing: Option<ExecutionTiming>,
34-
#[serde(default)]
35-
pub coverage: Option<SummaryCoverage>,
3633
}
3734

3835
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
@@ -80,7 +77,6 @@ fn visit(
8077
schema: node.schema.clone(),
8178
guarantee: node.guarantee.clone(),
8279
timing: node.timing,
83-
coverage: node.coverage.clone(),
8480
});
8581
originals.push(Rc::clone(node));
8682
ids.insert(Rc::as_ptr(node), id);

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

Lines changed: 48 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,15 @@
33
//! every traversal needs (output category and schema, accuracy guarantee,
44
//! execution timing).
55
6+
use std::cell::OnceCell;
67
use std::collections::HashSet;
78
use std::rc::Rc;
89

910
use serde::{Deserialize, Serialize};
1011

1112
use super::asap::ASAPOp;
1213
use super::non_asap::NonASAPOp;
13-
use super::summary_coverage::{CoverageError, SummaryCoverage};
14+
use super::summary_coverage::SummaryCoverage;
1415
use crate::ir::SchemaDerivationError;
1516
use crate::post_asap::execution_data_state::ExecutionTiming;
1617
use crate::post_asap::guarantee::ResultGuarantee;
@@ -100,8 +101,33 @@ pub struct OperatorNode {
100101
pub schema: Schema,
101102
pub guarantee: Option<ResultGuarantee>,
102103
pub timing: Option<ExecutionTiming>,
103-
#[serde(default)]
104-
pub coverage: Option<SummaryCoverage>,
104+
/// Cache for [`Self::coverage`], derived from `operator`.
105+
#[serde(skip)]
106+
coverage_cache: CoverageCache,
107+
}
108+
109+
/// A lazily filled [`SummaryCoverage`]. It is not part of a node's value:
110+
/// equality ignores it, serialization skips it, and a clone starts empty so
111+
/// a clone whose operator is then edited cannot read a stale entry.
112+
#[derive(Default)]
113+
struct CoverageCache(OnceCell<Option<SummaryCoverage>>);
114+
115+
impl Clone for CoverageCache {
116+
fn clone(&self) -> Self {
117+
Self::default()
118+
}
119+
}
120+
121+
impl PartialEq for CoverageCache {
122+
fn eq(&self, _: &Self) -> bool {
123+
true
124+
}
125+
}
126+
127+
impl std::fmt::Debug for CoverageCache {
128+
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
129+
f.write_str("CoverageCache")
130+
}
105131
}
106132

107133
impl OperatorNode {
@@ -110,7 +136,9 @@ impl OperatorNode {
110136
/// ASAP operator, ...).
111137
pub fn new(operator: Operator) -> Result<Self, SchemaDerivationError> {
112138
let schema = operator.output_schema()?;
113-
Ok(Self::with_schema(operator, schema))
139+
let node = Self::with_schema(operator, schema);
140+
node.check_merge()?;
141+
Ok(node)
114142
}
115143

116144
/// Build a node with caller-supplied output names and qualifiers. For
@@ -124,7 +152,7 @@ impl OperatorNode {
124152
schema,
125153
guarantee: None,
126154
timing: None,
127-
coverage: None,
155+
coverage_cache: CoverageCache::default(),
128156
}
129157
}
130158

@@ -145,23 +173,22 @@ impl OperatorNode {
145173
self
146174
}
147175

148-
/// Attach caller-established coverage. Required on summary nodes; see
149-
/// [`Self::requires_coverage`].
150-
pub fn with_coverage(
151-
mut self,
152-
coverage: SummaryCoverage,
153-
) -> Result<Self, SchemaDerivationError> {
154-
coverage.validate()?;
155-
if self.result_kind != OperatorResultKind::State {
156-
return Err(CoverageError::NotState.into());
157-
}
158-
self.coverage = Some(coverage);
159-
Ok(self)
176+
/// What this summary state covers; `None` for a node that is not a
177+
/// `SummaryAgg` or a valid `SummaryMerge`. Derived on first use.
178+
pub fn coverage(&self) -> Option<&SummaryCoverage> {
179+
self.coverage_cache
180+
.0
181+
.get_or_init(|| SummaryCoverage::derive(self).ok())
182+
.as_ref()
160183
}
161184

162-
/// Summary nodes whose state can be composed must declare coverage.
163-
pub fn requires_coverage(&self) -> bool {
164-
matches!(self.asap(), Some(ASAPOp::SummaryAgg { .. }))
185+
/// A `SummaryMerge` is valid only over inputs with the same definition
186+
/// and disjoint selections.
187+
fn check_merge(&self) -> Result<(), SchemaDerivationError> {
188+
if matches!(self.asap(), Some(ASAPOp::SummaryMerge { .. })) {
189+
SummaryCoverage::derive(self)?;
190+
}
191+
Ok(())
165192
}
166193

167194
pub fn non_asap(&self) -> Option<&NonASAPOp> {
@@ -315,14 +342,8 @@ impl OperatorNode {
315342
"invalid time or identity column in schema".into(),
316343
));
317344
}
318-
match &node.coverage {
319-
Some(coverage) => {
320-
(*node.as_ref()).clone().with_coverage(coverage.clone())?;
321-
}
322-
None if node.requires_coverage() => return Err(CoverageError::Missing.into()),
323-
None => {}
324-
}
325345
node.operator.validate_inputs()?;
346+
node.check_merge()?;
326347
if node.result_kind != node.operator.output_kind() {
327348
return Err(SchemaDerivationError::InvalidScalarSignature(
328349
"retained result kind disagrees with operation".into(),

0 commit comments

Comments
 (0)