Repository navigation
Precompute factory mis-builds DeltaSetAggregator and SetAggregator as Sum #477
Description
Summary
The precompute engine still mishandles DeltaSetAggregator and SetAggregator configurations generated by the planner. Both aggregation types are implemented in the query engine, but create_accumulator_updater() does not route them to their corresponding keyed accumulators. They therefore fall through to the generic fallback and are built as SumAccumulatorUpdater instances.
The planner emits a DeltaSetAggregator companion for CountMinSketch plans so the query path can enumerate the subpopulation keys represented in the sketch. Losing that key set causes approximate keyed sum/count/avg results to be wrong or empty.
Environment
- Component:
asap-query-engineprecompute engine - Affected code:
asap-query-engine/src/precompute_engine/accumulator_factory.rs - Producer:
asap-plannerwith--streaming_engine precompute - Mode: standalone
query_engine_rust --config-file <engine_config.yaml>with remote-write ingest
Steps to reproduce
-
Plan a workload with an approximate keyed aggregation, for example:
sum by (label) (metric) -
Inspect the generated streaming configuration. It contains a
CountMinSketchand a companionDeltaSetAggregatorentry. -
Run
query_engine_rustwith that configuration and ingest samples for the planned metric. -
Query the approximate keyed aggregation.
Actual behavior
The DeltaSetAggregator/SetAggregator configuration is not recognized by the accumulator factory. It falls through to the generic fallback and constructs a sum updater, so the key-tracking accumulator is never populated. A warning is emitted, but ingest continues with incorrect semantics.
config_is_keyed() also does not classify these aggregation types as keyed, which is inconsistent with the accumulator implementations and makes the worker route them incorrectly.
Expected behavior
The factory should construct the correct keyed accumulator for DeltaSetAggregator and SetAggregator, and config_is_keyed() should agree with the resulting updater's is_keyed() value. Approximate keyed sum/count/avg queries should retain and enumerate the planned subpopulation keys correctly.
Proposed fix
- Add a
DeltaSetAggregatorUpdaterthat records keys inDeltaSetAggregatorAccumulator. - Add a
SetAggregatorUpdaterthat records keys inSetAggregatorAccumulator. - Add both aggregation types to
config_is_keyed(). - Remove the generic silent fallback for unhandled aggregation types; see the triage comment below.
- Add factory tests covering accumulator type, distinct key tracking, and
config_is_keyed()/is_keyed()consistency.
Related work
The separate CountMinSketch depth/width parameter mismatch from the original report was fixed by #483. It is no longer part of this issue.
Affected files
asap-query-engine/src/precompute_engine/accumulator_factory.rsasap-query-engine/src/precompute_operators/delta_set_aggregator_accumulator.rsasap-query-engine/src/precompute_operators/set_aggregator_accumulator.rs
Activity
[-]CMS configuration mismatch between asap-planner-rs and precompute-engine[/-][+]Precompute engine can't ingest planner-generated `CountMinSketch` configs, and silently mis-builds `DeltaSetAggregator` as `Sum`[/+]milindsrivastava1997 commentedon Jun 30, 2026
#483 opened for defect 1
[-]Precompute engine can't ingest planner-generated `CountMinSketch` configs, and silently mis-builds `DeltaSetAggregator` as `Sum`[/-][+]Precompute factory mis-builds DeltaSetAggregator and SetAggregator as Sum[/+]milindsrivastava1997 commentedon Sep 1, 2026
One implementation constraint is important here: we should not retain a generic
_ => default to Sumfallback increate_accumulator_updater().An unhandled
AggregationTypehas different semantics fromSum; silently constructing a sum updater produces incorrect data while only emitting a warning. The factory should either use an exhaustive match so newly added variants fail at compile time, or return a loud configuration error for unsupported types. This also prevents future planner/query-engine aggregation types from being silently degraded.