Skip to content

feat(ir): add summary coverage metadata for time and population - #567

Draft
zzylol wants to merge 16 commits into
mainfrom
feat/summary-coverage-contract
Draft

zzylol wants to merge 16 commits into
mainfrom
feat/summary-coverage-contract

Conversation

@zzylol

@zzylol zzylol commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

Design doc: the schema and summary-semantics design for this PR (problem, design considerations, per-operator examples, key interfaces) is in #573, docs/design_docs/proposals/asap-primitive-schema.md, reviewed separately against main. This PR contains the code, tests and the ScanSelection doc renames only.

Closes #571.

Problem: a #535 schema says what a summary is, not what it summarizes

#535 gives every ASAP edge one Schema. For a summary edge it records the field layout and the committed state type:

pub struct Schema {
    pub fields: Vec<Field>,            // e.g. job: Plain(Utf8), state: Sketch(KLL{k=200}, PerSubpopulationInstance)
    pub time_index: Option<ColumnId>,  // position of a timestamp column, not a time range
    pub unique_keys: Vec<Vec<ColumnId>>,
    pub closed: bool,
}

Nothing in it says which time range or which population (label values) the state was built from. #535 left this out on purpose ("filters, reduction/group keys, … execution timing, window framework … are not additional Schema or Field members"). Composition is where this becomes a gap: once the planner combines existing summary states (#560 SummaryMerge, reuse of ingested panes, sub-DAG sharing in #537), the state's origin is no longer visible in its producer, and the schema is all that is left. Every example below uses two states with byte-for-byte equal schemas:

Schema(job: Plain(Utf8), state: Sketch(KLL{k=200}, PerSubpopulationInstance)), result_kind = State

Example 1: time. Equal schemas, different answers

Input A Input B Merging A and B is…
latency, [00:00, 00:01) latency, [00:01, 00:02) correct: p99 over [00:00, 00:02)
latency, [00:00, 00:02) latency, [00:01, 00:03) wrong: every observation in [00:01, 00:02) is counted twice, which skews the quantile and doubles counts or frequencies
latency, [00:00, 00:01) latency, [00:02, 00:03) correct only for [0,1) ∪ [2,3); wrong if the result is used for the continuous window [00:00, 00:03)

time_index is a column position. A KLL state has no timestamp column at all, so time_index is None in all three rows, and the schema cannot tell these cases apart.

Example 2: population (label values). Equal schemas, different answers

Input A Input B Merging A and B is…
region='us' region='eu' correct: p99 for us ∪ eu within each job
region='us' tier='premium' wrong: premium US requests are counted in both inputs
region='us' region='us' wrong: everything is counted twice

region is a filter label, not an output column, so it never appears in the schema. The job field only says the state is grouped by job. It does not say which jobs or which rows contributed.

Example 3: time and population together

A = us × [0,1) and B = eu × [1,2). The merged state covers exactly those two blocks. Describing it as {us,eu} × [0,2) (the result of storing a time range and a label set separately) would claim EU data for [0,1) and US data for [1,2) that was never read. The metadata has to keep time and population paired per region.

Example 4: answering a query from a stored state

Query: p99(latency) WHERE region='us' AND ts IN [10:00, 10:05) GROUP BY job. A stored state with the matching schema could hold US data for 10:00–10:05, EU data, or US data for only 10:00–10:03. All three have the same schema. Today the planner can confirm that the state type fits, but not that the contents fit.

Conclusion. Schema equality is necessary but not sufficient for composing or reusing summaries. Without time/population metadata, the planner must either refuse every composition or accept silent double counting and missing data.

What this PR adds

Schema stays the layout contract from #535 and does not describe coverage. Coverage is a sibling field on the node, next to schema:

OperatorNode
├── schema: Schema                    what each output row looks like   (#535)
└── coverage: Option<SummaryCoverage> which observations the state holds (this PR)

coverage is required on summary nodes: validate_structure rejects a SummaryAgg (and, in #560, a SummaryMerge) whose coverage is None with CoverageError::Missing. Plain relational nodes leave it None; the field is an Option only because all operators share OperatorNode.

Coverage cannot go inside Schema. #560 only allows a merge when the input schemas are equal, and the inputs of a useful merge ([0,1) + [1,2)) always have different coverage.

pub struct SummaryCoverage {
    pub source: Source,               // as named by Scan: Table { table_ref } or TimeSeries { metric }
    pub regions: Vec<CoverageRegion>, // union of time × population blocks
}
pub struct CoverageRegion {
    pub time_ms: Option<Range<i64>>,            // half-open; None = no time restriction
    pub population: BTreeMap<String, String>,   // label = value AND …; empty = all
}

// OperatorNode
pub coverage: Option<SummaryCoverage>;   // required on summary nodes
pub fn requires_coverage(&self) -> bool;
pub fn with_coverage(self, c: SummaryCoverage) -> Result<Self, SchemaDerivationError>;
// SummaryCoverage
pub fn merge_disjoint(inputs: &[Self]) -> Result<Self, CoverageError>;
// SchemaDerivationError
Coverage(CoverageError)

Every observation in a region is assumed to contribute once to the state.

Removing duplication

Given the new definition, coverage holds only what no other type records: time × population. This PR also removes the duplication that the first draft introduced or exposed:

  • input and reduction dropped from coverage. They copied SummaryAgg.input / SummaryAgg.reduction on the same node, and with_coverage needed a ProducerMismatch check to keep the copies in sync. feat(ir): define compatible logical summary merges #560's SummaryMerge now compares them on its producers through OperatorNode::summary_update().
  • source is a Source, not a String. It uses the same type as Scan.source, so one table cannot have two spellings.
  • SourceCoverage renamed to ScanSelection (asap-aware-mapping, about 100 call sites plus docs). It names the rows a physical scan reads for cost comparison, a different concept from SummaryCoverage.
  • revision and multiplicity were removed earlier, as a deployment concern and a single-variant enum respectively.

How the examples come out under merge_disjoint:

Case Result
[0,1) + [1,2), same population accepted, coalesced to one region [0,2)
[0,1) + [2,3) accepted, two regions (gap kept)
[0,2) + [1,3) PossibleOverlap
region=us + region=eu, same time accepted, two regions
region=us + region=us PossibleOverlap
region=us + tier=premium PossibleOverlap. Different label names prove nothing.
us×[0,1) + eu×[1,2) accepted, two regions, never widened to {us,eu}×[0,2)
different source SourceMismatch
different update expression or reduction rejected by SummaryMerge (#560)

Rules:

  • A summary node without coverage fails validate_structure. Some with regions = [] means known empty.
  • time_ms: None is for sources without a time column (plain tabular data). Such a region overlaps every region it is not population-disjoint from.
  • with_coverage validates the declaration and requires State output (CoverageError::NotState). validate_structure re-checks it.
  • Rewriting a node's inputs clears the coverage, like other assessed metadata, so the rewriter must declare it again with with_coverage.
  • Declarations come from trusted composition rules or catalogs. Nothing is inferred from arbitrary SQL predicates.

Population and time bounds are trusted

Coverage is declared by the composition rule or catalog that built the subtree. Nothing checks population against SummaryAgg.filter, Filter nodes or Scan.predicates. Time bounds can't be checked either, because TimeRange stores a relative Duration. So a wrong declaration passes:

A = SummaryAgg(filter: region='us', input: latency, reduction: by job)
    declared coverage: {region: eu} × [0,1)        ← wrong; the state holds US data
B = SummaryAgg(filter: region='us', input: latency, reduction: by job)
    declared coverage: {region: us} × [0,1)

merge_disjoint(A, B)  → accepted ("eu" ≠ "us" proves disjoint)
actual merged state   → every US observation in [0,1) counted twice
a query for region='eu' could also be answered from A, which holds no EU data

Follow-up #570 adds the check: the declared population must exactly equal the column = literal predicates collected between the SummaryAgg and its Scan, and any other predicate shape fails closed. It starts strict about which operators may sit on that path (only Filter and TimeRange), because Project or Join can rename columns or change rows. The list is widened when a real SQL or PromQL plan needs it.

Out of scope

  • Checking that coverage contains a requested query window or population (Example 4). That is a later query-relative check; this PR supplies the data it needs.
  • Checking declared population against subtree predicates: Check declared summary coverage population against subtree filters #570.
  • Predicates beyond non-null equality conjunctions, idempotent set-union families.
  • Runtime merge kernels, accuracy, storage, or execution timing.

Stack and validation

Order: #567 → #560 (SummaryMerge requires and derives coverage) → #537 (logical transport and CSE preserve them) → #539 → #540 → #561.

Tests in crates/types/tests/summary_coverage.rs cover adjacency, gaps, population disjointness, joint regions, overlap, regions without time bounds, source mismatch, serde round-trip, invalid declarations, required coverage, and clearing after rewrites. Each example in this body and the doc is also built as a real SummaryAgg → SummaryMerge plan in #560's summary_coverage_examples.rs. The design document is reviewed separately in #573.

🤖 Generated with Claude Code

@zzylol zzylol changed the title feat(ir): define summary observation coverage contract feat(ir): define summary observation extent contract Oct 3, 2026
…ts fields

Name the metadata coverage to match #560 and the docs; drop the single-variant
multiplicity and deployment-specific revision; rename grouping to reduction to
match SummaryAgg; report failures through SchemaDerivationError::Coverage; revert
the unrelated PaneCoverageError rename.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@zzylol zzylol changed the title feat(ir): define summary observation extent contract feat(ir): add summary coverage metadata for time and population Oct 3, 2026
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
zzylol and others added 2 commits October 3, 2026 17:19
…me bounds

validate_structure rejects a SummaryAgg without coverage (CoverageError::Missing).
CoverageRegion time bounds become optional so tabular sources without a time
column can declare coverage. Population stays trusted; #570 tracks checking it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Given required coverage on summary nodes, input and reduction duplicated the
producing SummaryAgg fields; drop them along with ProducerMismatch. Type source
as Source, matching Scan.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
SourceCoverage names the rows a physical scan reads for cost comparison, not
which observations a summary state holds; rename it so it is not confused with
SummaryCoverage.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@zzylol

zzylol commented Oct 3, 2026

Copy link
Copy Markdown
Contributor Author

TODO: should write a design doc for it, not developer doc, human written.

zzylol and others added 4 commits October 3, 2026 19:32
Move it to docs/design_docs/proposals with problem and motivation,
requirements, design, alternatives and key code interfaces.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@zzylol
zzylol marked this pull request as draft October 3, 2026 20:01
The design document and the docs it consolidates are reviewed separately on
main. This PR keeps code, tests and the ScanSelection rename in docs.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Selvomega

Copy link
Copy Markdown
Collaborator
image I suggest you change this problem headline: This is misleading. Encoding "what is summarized" is not schema's job

@Selvomega

Selvomega commented Oct 4, 2026 •

Copy link
Copy Markdown
Collaborator

According to this PR doc, I still don't think we are solving the right problem here.
You cannot ask PR to encode that much semantic information of data.
For example, consider two SQL queries:
SELECT sum_1 AS SUM(val) FROM table WHERE time in [Jan, Feb)
SELECT sum_2 AS SUM(val) FROM table WHERE time in [Feb, Mar)
Each SQL gives you a sum and now, say, you want to get SUM(val) from Jan to Mar. You naturally want to merge sum_1 and sum_2 but find the schema of the aggregation node cannot encode the sum range. (since schema only tells you that after this aggregate operator there is only one column with type, say, float)

This is indeed annoying but it is not schema problem. Since the responsibility of schema is mostly to encode data structure, not data semantic. To determine if the above two sums need to be merged, we can either

  • Go deep into the tree and analyze the SQL semantic
  • Maintain some side-cart data structure holding the semantic of the subtree / subDAG below some certain operator.
    But we should not do that in schema. For example, when we implemented CSE analysis, we also did not resort to schema, cuz that is not the correct place to go to. :)

@zzylol

zzylol commented Oct 4, 2026

Copy link
Copy Markdown
Contributor Author

This is indeed annoying but it is not schema problem. Since the responsibility of schema is mostly to encode data structure, not data semantic. To determine if the above two sums need to be merged, we can either

  • Go d

Yes, this is not a schema problem, I will change the PR problem description title. Actually in the code implementation, the "what is summarized" is a field in the Node, in parallel with the field "schema" in the node struct.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Summary states lack time/population coverage metadata; equal schemas can't prove a merge or reuse is safe

2 participants