Skip to content

fix(sql): finalize nested exact aggregate inputs - #379

Merged
zzylol merged 1 commit into
mainfrom
fix/nested-exact-read-boundary
Sep 10, 2026
Merged

zzylol merged 1 commit into
mainfrom
fix/nested-exact-read-boundary

Conversation

@zzylol

@zzylol zzylol commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Why

The nested SQL aggregate rule could pass an ExactAggregate accumulator state directly into a read-time outer aggregate. Strict execution-state validation then rejected ordinary nested sums and backend workload compilation.

What

Finalize an exact accumulator child at the maintenance-to-read boundary before a nested read-time aggregate consumes it.

How

Residual materialization detects exact read-time operations and applies the existing FinalizeExactAccumulator transformation using the child expression's canonical plain output schema. The validator remains unchanged.

Before this PR

sum(rate(...)) and nested SQL sums could fail with exact operator consumes non-plain column.

After this PR

The inner accumulator remains maintained state, its explicit finalize node produces readable rows, and the outer aggregate consumes the typed values.

Verification

  • cargo fmt --all -- --check
  • cargo test -p asap-integration-tests --test sql_to_post_asap — 11 passed
  • cargo test -p asap-integration-tests --test promql_to_post_asap promql_sum_of_count_over_time_is_composed_by_default_search — passed

@zzylol
zzylol merged commit 9f526a9 into main Sep 10, 2026
3 checks passed
@zzylol
zzylol deleted the fix/nested-exact-read-boundary branch September 21, 2026 18:44
Selvomega added a commit that referenced this pull request Sep 26, 2026
Brings per-measure FILTER predicates (#466) plus the five main commits it
is based on (#445, #455, #426, #460, #461). Conflicts resolved toward
main: `ValueOperationAtIngestionTime`, `query_time_nested_sum`, and the
unconditional `finalize_exact_accumulator` from #461 replace dev/dqc's
older #379 shape; the #455 wording in the architecture docs stands; the
moved `accuracy.rs` is dropped in favor of `accuracy/mod.rs`.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
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.

1 participant