Skip to content

feat: lower and compile PromQL histogram_quantile - #493

Merged
zzylol merged 1 commit into
mainfrom
feat/histogram-quantile
Oct 1, 2026
Merged

zzylol merged 1 commit into
mainfrom
feat/histogram-quantile

Conversation

@zzylol

@zzylol zzylol commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Stacked on #494.

Rebuilt into the linear stack on main. New integrate: commit(s) fold in integration-branch resolutions this PR needs on top of the earlier stack: "bind the histogram input layout in series_labels". Conflict resolutions are recorded in the messages of: "feat(physical): compile classic histogram_quantile in PromQL fallbacks"; "docs: record classic histogram_quantile coverage".

Why

compile rejected PromQL histogram_quantile (coverage row 10). The IR was Aggregate{by (), [HistogramQuantile{q}]}, so the compiler could not tell which series form one histogram, and the output lost its labels.

What

  • IR: AggIntent::HistogramQuantile { q, le: C } names the bucket-bound column. PromQL lowers the classic form to Aggregate{Reduce(without([le])), [HistogramQuantile{q, le}]}. The without keys seed le into the selector schema even when no matcher names it. The SQL asap_histogram_quantile bridge keeps its single-histogram by () and now names le.
  • Types fix: without output schemas no longer keep a nested aggregate's renamed value (e.g. sum) as a label.
  • Physical: a new SeriesHistogramQuantile operator. It groups rows by every label except le (keeping __name__, as Prometheus does), applies bucketQuantile, and drops le and __name__. Result label sets that become equal are an error, as in Prometheus. It works on series-identity rows and plain label columns, so it covers rate(x_bucket[5m]), sum by (le, job) (…) and bare selectors.
  • bucket_quantile now matches Prometheus:
    • almost.Equal for the small-delta fix, so an infinite count is no longer merged into a finite one.
    • Divide-then-scale interpolation.
    • A NaN rank finds no bucket.
    • le values that ParseFloat rejects as out of range are skipped.
  • Selection: candidate search still keeps this intent as one exact KeepPreAsap subtree for exact and approximate targets. No sketch candidates were added.

Before this PR

histogram_quantile(0.5, sum by (le, job) (rate(x_bucket[5m]))) → compile error: "vector aggregate has no native lowering". Even if it had compiled, the IR described one quantile over all buckets with no labels.

After this PR

With buckets {1: 2, 2: 6, 4: 8, +Inf: 10} per job=a, the query compiles through search and selection and returns {job="a"} 1.75. The hand-computed Prometheus result is 1 + (2-1)·((5-2)/4). A per-series rate(x_bucket[5m]) argument keeps {inst, job}.

Known limits:

  • An argument that provably lacks le (sum by (job) (…)) is rejected at lowering. Prometheus returns an empty vector for it.
  • histogram_quantiles and aggregating over the result still don't compile. See the coverage doc.

Validation

  • cargo fmt --all -- --check, cargo clippy --workspace --all-targets -- -D warnings, and cargo test --workspace --no-fail-fast pass (1401 tests).
  • New end-to-end tests in asap-physical-operators/tests/promql_fallback.rs check hand-computed results for:
    • interpolation and the +Inf bucket;
    • q<0, q>1 and NaN q;
    • a missing +Inf bucket, fewer than two buckets, and zero observations;
    • unsorted buckets, unparsable and out-of-range le values, duplicate bounds and non-monotonic counts;
    • a lowest bucket bound ≤ 0;
    • rate and sum shapes;
    • duplicate output label sets;
    • search and selection.
  • There are also frontend lowering and unit tests.
  • Each fix has a regression test that failed before the fix.
  • An independent agent reviewed the change. Its NaN-rank, ParseFloat-overflow and SQL-schema findings are fixed here.

🤖 Generated with Claude Code

zzylol added a commit that referenced this pull request Sep 30, 2026
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@zzylol
zzylol force-pushed the fix/series-labels-duplicates branch from 17d1c12 to ad45318 Compare September 30, 2026 18:27
zzylol added a commit that referenced this pull request Sep 30, 2026
A without (le) HistogramQuantile aggregate compiles to a series operator
that groups rows by every label except le, applies bucketQuantile, and
drops le and __name__, rejecting equal result label sets.

Conflicts with earlier stack changes resolved to the integration tree:
- crates/asap-physical-operators/src/operators/mod.rs: 2bea2f6 Merge #493 classic histogram quantile coverage
- series_labels.rs and tests/promql_fallback.rs: placement from 2bea2f6
  without the later c39156e/15ab72b changes of this PR

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
zzylol added a commit that referenced this pull request Sep 30, 2026
Conflicts with earlier stack changes resolved to the integration tree:
- docs/develop_docs/physical-compile-coverage.md: 2bea2f6 Merge #493
  classic histogram quantile coverage, without the paragraph that 6d895b7
  in this PR adds later. The comparison section from #492 stays, the
  Remaining list drops the entries both PRs now cover, and the totals
  count both PRs.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@zzylol
zzylol force-pushed the feat/histogram-quantile branch from 6d895b7 to fbeddda Compare September 30, 2026 18:27
@zzylol
zzylol changed the base branch from fix/series-labels-duplicates to fix/reject-promql-fill September 30, 2026 18:28
@zzylol
zzylol force-pushed the fix/reject-promql-fill branch from feade00 to 6eb8a84 Compare September 30, 2026 20:24
zzylol added a commit that referenced this pull request Sep 30, 2026
A without (le) HistogramQuantile aggregate compiles to a series operator
that groups rows by every label except le, applies bucketQuantile, and
drops le and __name__, rejecting equal result label sets.

Conflicts with earlier stack changes resolved to the integration tree:
- crates/asap-physical-operators/src/operators/mod.rs: 2bea2f6 Merge #493 classic histogram quantile coverage
- series_labels.rs and tests/promql_fallback.rs: placement from 2bea2f6
  without the later c39156e/15ab72b changes of this PR

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
zzylol added a commit that referenced this pull request Sep 30, 2026
Conflicts with earlier stack changes resolved to the integration tree:
- docs/develop_docs/physical-compile-coverage.md: 2bea2f6 Merge #493
  classic histogram quantile coverage, without the paragraph that 6d895b7
  in this PR adds later. The comparison section from #492 stays, the
  Remaining list drops the entries both PRs now cover, and the totals
  count both PRs.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@zzylol
zzylol force-pushed the feat/histogram-quantile branch from fbeddda to 9b53ea7 Compare September 30, 2026 20:24
@zzylol
zzylol changed the base branch from fix/reject-promql-fill to feat/kernel-codec-accessors October 1, 2026 22:48
@zzylol
zzylol force-pushed the feat/histogram-quantile branch from 9b53ea7 to 50c9d6d Compare October 1, 2026 22:48
@zzylol
zzylol force-pushed the feat/kernel-codec-accessors branch from b8440f7 to a9db8d7 Compare October 1, 2026 23:06
@zzylol
zzylol changed the base branch from feat/kernel-codec-accessors to main October 1, 2026 23:14
@zzylol
zzylol force-pushed the feat/histogram-quantile branch from 50c9d6d to ae5f517 Compare October 1, 2026 23:14
@zzylol
zzylol merged commit deb3904 into main Oct 1, 2026
3 of 6 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