From 277e3cba7666dd9ae2d9945e7726ca723d30f280 Mon Sep 17 00:00:00 2001 From: zz_y Date: Fri, 11 Sep 2026 10:28:14 -0600 Subject: [PATCH 1/3] fix: finalize exact state before maintained value consumers --- crates/asap-aware-mapping/src/replacement.rs | 23 ++++++- .../tests/promql_to_post_asap.rs | 68 +++++++++++++++++++ .../src/post_asap/execution_data_state.rs | 3 +- 3 files changed, 92 insertions(+), 2 deletions(-) diff --git a/crates/asap-aware-mapping/src/replacement.rs b/crates/asap-aware-mapping/src/replacement.rs index f2c14749..bc073e0b 100644 --- a/crates/asap-aware-mapping/src/replacement.rs +++ b/crates/asap-aware-mapping/src/replacement.rs @@ -1817,6 +1817,14 @@ fn realize_binary( fn finalize_exact_accumulator( node: Rc, logical_output: &QueryExpr, +) -> Result, ImplementError> { + finalize_exact_accumulator_at(node, logical_output, ExecutionTiming::ReadTime) +} + +fn finalize_exact_accumulator_at( + node: Rc, + logical_output: &QueryExpr, + timing: ExecutionTiming, ) -> Result, ImplementError> { let is_exact_state = matches!( node.expr, @@ -1838,7 +1846,7 @@ fn finalize_exact_accumulator( expr: SummaryExpr::ValueOperation { child: node, operation: ValueOperation::FinalizeExactAccumulator, - timing: ExecutionTiming::ReadTime, + timing, }, schema, guarantee, @@ -2228,6 +2236,11 @@ fn construct_summary_agg( } let bound_child = realize_child_with(&input.child, models, child_target)?; + // A maintained parent consumes finalized values, never the child's + // accumulator representation. Keep the read boundary explicit even when + // an exact scalar accumulator currently stores its value directly. + let bound_child = + finalize_exact_accumulator_at(bound_child, &input.child, ExecutionTiming::MaintenanceTime)?; // ── Guarantee (issue #172) ────────────────────────────────────────── // Derived *before* the node exists, so an illegal composition is never @@ -7954,6 +7967,14 @@ mod tests { family, SummaryFamilyType::Sketch(kind, _) if kind.algorithm() == &SketchAlgorithm::Kll )); + let SummaryExpr::ValueOperation { + child, + operation: ValueOperation::FinalizeExactAccumulator, + timing: ExecutionTiming::MaintenanceTime, + } = &child.expr + else { + panic!("expected explicit maintenance readout"); + }; let SummaryExpr::SummaryAgg { family: inner_family, child: leaf, diff --git a/crates/integration-tests/tests/promql_to_post_asap.rs b/crates/integration-tests/tests/promql_to_post_asap.rs index 79c298b6..6892c16a 100644 --- a/crates/integration-tests/tests/promql_to_post_asap.rs +++ b/crates/integration-tests/tests/promql_to_post_asap.rs @@ -744,6 +744,15 @@ fn promql_quantile_of_rate_binds_kll_over_rate_accumulator() { ) ); + let SummaryExpr::ValueOperation { + child, + operation: ValueOperation::FinalizeExactAccumulator, + timing: asap_types::post_asap::ExecutionTiming::MaintenanceTime, + } = &child.expr + else { + panic!("rate needs a maintenance readout"); + }; + // The rate: exact counter-reset-aware accumulator, per-series (labels // and time axis preserved), no estimate wrapper. `rate(...)` has no // grouping concept at all — every entity stays its own summary. @@ -875,3 +884,62 @@ fn promql_sum_of_count_over_time_is_composed_by_default_search() { if range.as_secs() == 300 && matches!(child.as_ref(), QueryExpr::Scan { .. }) )); } + +#[test] +fn nested_summary_explicitly_finalizes_exact_child_at_maintenance_time() { + // Real workload selection must expose the state-to-value edge; an outer + // sketch must not interpret exact accumulator bytes as input samples. + let pre = Rc::new( + lower_promql( + "quantile(0.9, sum_over_time(m[1m]))", + AccuracyTarget::Epsilon(0.05), + ) + .unwrap(), + ); + let space = search_workload(vec![("query", pre)]); + let selected = space.global_selection(&DefaultCostModel); + let plan = selected.materialize(&space.roots[0].1).unwrap().unwrap(); + let SummaryExpr::SummaryEstimate { summary_input, .. } = &plan.expr else { + panic!("expected selected quantile summary"); + }; + let SummaryExpr::SummaryAgg { child, .. } = &summary_input.expr else { + panic!("expected maintained outer summary"); + }; + let SummaryExpr::ValueOperation { + child: source, + operation, + timing, + } = &child.expr + else { + panic!( + "missing explicit accumulator finalization: {:?}", + child.expr + ); + }; + assert!(matches!( + operation, + ValueOperation::FinalizeExactAccumulator + )); + assert_eq!( + *timing, + asap_types::post_asap::ExecutionTiming::MaintenanceTime + ); + assert!(matches!( + source.expr, + SummaryExpr::SummaryAgg { + family: SummaryFamilyType::ExactAggregate(ExactKind::Sum, _), + .. + } + )); + assert!(child + .schema + .fields + .iter() + .all(|field| matches!(field.dtype, SummaryFamilyType::Plain(_)))); + assert!(child + .schema + .fields + .iter() + .any(|field| matches!(field.dtype, SummaryFamilyType::Plain(DataType::Float64)))); + compile_executable_dag(&plan).expect("explicit boundary is a valid executable DAG"); +} diff --git a/crates/types/src/post_asap/execution_data_state.rs b/crates/types/src/post_asap/execution_data_state.rs index cff0c039..93d9c075 100644 --- a/crates/types/src/post_asap/execution_data_state.rs +++ b/crates/types/src/post_asap/execution_data_state.rs @@ -421,7 +421,8 @@ fn visit( ExecutionTiming::ReadTime => ExecutionDataState::READ_ROWS, }; let s = produced_data_state(&child.expr).unwrap_or(required); - let exact_readout = *timing == ExecutionTiming::ReadTime + let exact_readout = (*timing == ExecutionTiming::ReadTime + || matches!(operation, ValueOperation::FinalizeExactAccumulator)) && s == ExecutionDataState::MAINTENANCE_SUMMARY && is_exact_accumulator_state(&child.schema).is_ok(); if s != required && !exact_readout { From 19e7dd845af11436b1b2b7d2ec9d5e0a1ab4a6b1 Mon Sep 17 00:00:00 2001 From: zz_y Date: Fri, 11 Sep 2026 10:36:40 -0600 Subject: [PATCH 2/3] test: assert explicit nested accumulator read boundaries --- crates/integration-tests/tests/exact_composition.rs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/crates/integration-tests/tests/exact_composition.rs b/crates/integration-tests/tests/exact_composition.rs index 6540ffcc..fe32e568 100644 --- a/crates/integration-tests/tests/exact_composition.rs +++ b/crates/integration-tests/tests/exact_composition.rs @@ -226,7 +226,7 @@ fn names(node: &SummaryNode) -> Vec<&str> { // ── step 1: pin every already-supported exact-accumulator nesting ─────── #[test] -fn every_exact_accumulator_nests_directly_under_an_outer_sketch() { +fn every_exact_accumulator_is_finalized_before_an_outer_sketch() { use std::time::Duration; let cases: Vec<(Rc, ExactKind)> = vec![ ( @@ -297,12 +297,20 @@ fn every_exact_accumulator_nests_directly_under_an_outer_sketch() { let SummaryExpr::SummaryAgg { child, .. } = &summary_input.expr else { panic!("expected outer SummaryAgg"); }; + let SummaryExpr::ValueOperation { + child, + operation: asap_types::post_asap::ValueOperation::FinalizeExactAccumulator, + timing: ExecutionTiming::MaintenanceTime, + } = &child.expr + else { + panic!("{kind:?}: missing maintenance finalization"); + }; assert!( matches!( &child.expr, SummaryExpr::SummaryAgg { family: SummaryFamilyType::ExactAggregate(k, _), .. } if *k == kind ), - "{kind:?}: expected the exact accumulator directly under the outer sketch, got {:?}", + "{kind:?}: expected the exact accumulator under its finalization, got {:?}", child.expr ); validate_execution_data_states(root).expect("accumulator state composes under maintenance"); From d80a2ebc96951585dc2eac4fe7577a3a33259a0e Mon Sep 17 00:00:00 2001 From: zz_y Date: Fri, 11 Sep 2026 10:42:05 -0600 Subject: [PATCH 3/3] docs: clarify explicit maintenance finalization edge --- crates/types/src/post_asap/execution_data_state.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/types/src/post_asap/execution_data_state.rs b/crates/types/src/post_asap/execution_data_state.rs index 93d9c075..8f5bfb14 100644 --- a/crates/types/src/post_asap/execution_data_state.rs +++ b/crates/types/src/post_asap/execution_data_state.rs @@ -23,7 +23,7 @@ //! | `SummaryEstimate.summary_input` | `MAINTENANCE_SUMMARY` (any family). Produces `READ_ROWS`. | //! | `SummaryJoin.outer/inner` | `MAINTENANCE_ROWS` or `MAINTENANCE_SUMMARY`; never a read-time data_state. | //! | `SummarySubtract`/`SummaryDelete`/`SummaryMerge` | `MAINTENANCE_SUMMARY`. | -//! | `ValueOperation.child` with `MaintenanceTime` | `MAINTENANCE_ROWS`. Produces `MAINTENANCE_ROWS`. | +//! | `ValueOperation.child` with `MaintenanceTime` | `MAINTENANCE_ROWS`; explicit `FinalizeExactAccumulator` also accepts exact accumulator state. Produces `MAINTENANCE_ROWS`. | //! | `ValueOperation.child` with `ReadTime` | `READ_ROWS`. Produces `READ_ROWS`. | //! //! ## `KeepPreAsap` declares its data_state through the derivation