feat(promql): lower histogram_quantiles as a per-φ Merge fan-out (#109) - #121
Merged
Merged
Conversation
`histogram_quantiles(v, "label", φ₀, φ₁, …)` is the experimental multi-quantile form: `histogram_quantile(φᵢ, v)` evaluated once per φ, each output series tagged with `label = φᵢ`. It parsed but was rejected as UnsupportedFunction. Lower it to a `Merge` of one `Relabel`-wrapped quantile branch per φ. Each branch reuses the single-quantile decision — classic `le`-buckets interpolate (`HistogramQuantile`), native histograms and raw samples take the sketch-able `Quantile` (#43 / #79) — so the two functions cannot diverge. The choice is a property of the argument, not of φ, so it is made once. Two details that are easy to get wrong: - Label values use Prometheus's `labels.FormatOpenMetricsFloat`, not Rust's `Display`: `1 → "1.0"`, `0 → "0.0"`, and Go's `%g` exponent form below 1e-4 (`0.00001 → "1e-05"`). - Each branch aliases its value column to `value` instead of taking the intent-keyed name (`quantile_0_5`, `quantile_0_9`, …). `Merge` derives its schema from the first child, so branches that disagree on a column name would make the merged schema silently misdescribe every branch but one. The quantile is carried by the label column, which is where Prometheus puts it. This is the first front end to emit `L2::Merge`, previously reserved. Corpus: testdata lowered 1502 → 1512. The 9 remaining `histogram_quantiles` lines are pre-existing deliberate rejections — 7 out-of-range φ (identical to `histogram_quantile(1.001, …)`) and 2 `__name__` regex selectors (#67). The coverage tripwire is ratcheted accordingly, per its own instructions. The other half of #109 — non-literal scalar `k` / φ parameters — is split out to #120: it needs a scalar subtree in the intent and at the L4 sketch readout. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
Author
|
@milindsrivastava1997 @Selvomega Interesting "Merge" node. |
This was referenced Jul 10, 2026
Collaborator
|
Not sure I understand. Can you give some context? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of #109 — the
histogram_quantileshalf. The non-literal scalark/φ half is split out to #120.Left open for review, not merged.
Change
histogram_quantiles(v, "label", φ₀, φ₁, …)is the experimental multi-quantile form:histogram_quantile(φᵢ, v)evaluated once per φ, each output series tagged withlabel = φᵢ. It parsed but was rejected asUnsupportedFunction(19 corpus lines).It lowers to a
Mergeof oneRelabel-wrapped quantile branch per φ:Each branch reuses the single-quantile decision — classic
le-buckets interpolate (HistogramQuantile), native histograms and raw samples take the sketch-ableQuantile(#43 / #79) — so the two functions cannot diverge. The choice is a property of the argument, not of φ, so it is made once.This is the first front end to emit
L2::Merge, previously marked reserved.Two details that were easy to get wrong
Label formatting. Prometheus uses
labels.FormatOpenMetricsFloat, not Go’s default:1 → "1.0",0 → "0.0", and%gexponent form below1e-4(0.00001 → "1e-05"). I readmodel/labels/float.gorather than guess; Rust’sDisplaywould have produced1,0, and0.00001.Union compatibility. Each branch aliases its value column to
valuerather than the intent-keyed name (quantile_0_5,quantile_0_9, …).Mergederives its schema from the first child, so branches disagreeing on a column name would make the merged schema silently misdescribe every branch but one. My first version had exactly that bug —merged schema = ["quantile_0_5", "q"]while branch 2 was["quantile_0_9", "q"]. The quantile belongs in the label, which is where Prometheus puts it.Corpus
testdatalowered 1502 → 1512; rejected 86 → 76.Of the 19
histogram_quantileslines, 10 now lower. The 9 that remain are pre-existing, deliberate rejections and not regressions:-0.1,1.01,NaN) — identical tohistogram_quantile(1.001, …); whether we should follow Prometheus and return ±Inf with an annotation is the open question already noted in PromQL: histogram_quantiles() and non-literal scalar k/φ parameters unsupported #109.__name__regex selectors — PromQL:__name__=~/!~regex selectors mislowered to a literal metric name #67.The coverage tripwire is ratcheted from 1485 to 1495, which is what its own comment asks for when coverage lands.
Tests (6 new)
histogram_quantiles_fans_out_over_native_histogramsQuantilehistogram_quantiles_over_classic_buckets_interpolates_bucket→HistogramQuantile, never a sketchhistogram_quantiles_branches_are_union_compatiblehistogram_quantiles_uses_the_given_label_name"phi", not a hardcoded"q"histogram_quantiles_formats_small_quantiles_like_prometheus1e-05histogram_quantiles_rejects_an_out_of_range_quantileVerification
cargo fmt --allnot run —maincarries pre-existing rustfmt diffs. Each touched file has the same violation count as onmain.🤖 Generated with Claude Code