Skip to content

Check declared summary coverage population against subtree filters #570

Description

@zzylol

Problem

#567 adds SummaryCoverage for summary nodes. with_coverage checks the declared input and reduction against the producing SummaryAgg. The declared population is trusted: nothing checks it against SummaryAgg.filter, Filter nodes, or Scan.predicates in the subtree. So a wrong declaration passes validation:

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

Proposed check

Collect every predicate between the SummaryAgg and its Scan: SummaryAgg.filter, any Filter nodes, and Scan.predicates. Require the declared population to equal the collected column = literal map exactly. A subset is not enough, because it errs in both directions:

  • Fewer conditions declared than the filter applies overstate the data. filter: region='us' AND tier='free' declared as {region: us} claims all US data.
  • More conditions declared than the filter applies understate the data. No filter, declared as {region: us}, lets a merge with {region: eu} pass and double-counts EU rows.

Any other predicate shape on that path fails with a new CoverageError::UnsupportedPredicate. That covers >, OR, IN, regex matchers, function calls, and NULL or float literals. Keys are field names in the schema the predicate is evaluated against (predicates use positional ColumnIds).

Operators allowed between SummaryAgg and Scan

Project or Join can rename columns or change rows, which breaks the column-to-name mapping. Start strict: walk only through Filter and TimeRange (which restricts time, not population), and stop at Scan. Any other operator on the path fails with UnsupportedPredicate. Widen the list once a real SQL or PromQL plan needs it, for example a column-preserving Project.

Out of scope

Time bounds. TimeRange stores a relative Duration, so declared absolute time_ms bounds cannot be checked in the logical IR and remain trusted.

Follow-up to #567.

🤖 Generated with Claude Code

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions