Skip to content

fix(promql): count observations with unit sketch weights - #406

Merged
zzylol merged 2 commits into
mainfrom
fix/promql-count-semantics
Sep 13, 2026
Merged

fix(promql): count observations with unit sketch weights#406
zzylol merged 2 commits into
mainfrom
fix/promql-count-semantics

Conversation

@zzylol

@zzylol zzylol commented Sep 13, 2026

Copy link
Copy Markdown
Contributor

PromQL counts must not vary with sample values. This fixes both count lowering paths and extends the existing frequency-update rule to CMS/CountSketch counts.

Before this PR: count(up) lowered to value cardinality; three series with {1,1,1} expressed a distinct result of 1. count_over_time already lowered to Count, but its CMS/CountSketch plans carried the sample-value column as a weight, allowing value sums and signed CMS updates.

After this PR: count(up) lowers to Count (three vector elements in either {1,1,1} or {1,1,0}), and count sketches carry an explicit item plus constant +1 weight with a UnitCount proof. Ten observations contribute ten updates regardless of whether values are 1, 0, 3, or negative. SQL distinct counting and the existing rejection of unproven signed value-weighted CMS remain unchanged.

Verification: Regression tests failed on the original code for both defects. Added query-text → summary-selection → executable-DAG coverage; corrected existing tests that asserted the erroneous cardinality mapping. All 611 tests passed with cargo +1.98.0 test -p asap-frontend-promql -p asap-aware-mapping --locked. Focused all-target Clippy (-D warnings), workspace formatting, and git diff --check passed. Workspace-wide SQL/integration tests are left to CI.

Reported fixtures: Added numerical planner-contract tests for count(up) on {1,1,1} and {1,1,0} (both 3), ten scrapes of 1/0/3 (each 10), all-negative/mixed-sign series, and CMS update totals for signed/zero values. All five count-planning tests pass, as do focused Clippy and formatting checks. Re-running against the pre-fix implementation makes the new target-count and CMS-total regressions fail. The fixture interpreter covers selected observations and emitted aggregate/update contracts; it is not a backend or sketch-library runtime.

Scope: Planner semantics and executable contracts. Production-process execution was not run; ASAPQuery needs to adopt the fixed revision and replace its hand-built count/HLL oracles as part of ProjectASAP/ASAPQuery-backend#503 and ProjectASAP/ASAPQuery-backend#489. This PR does not claim the broader PromQL rollout is E2E-complete.

Addresses #405.

@zzylol
zzylol merged commit e96c231 into main Sep 13, 2026
4 checks passed
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