Group the PromQL per-series aggregation by the raw id column

[nikitamikhaylov]

Changelog category (leave one):

• Performance Improvement

Changelog entry (a user-readable short description of the changes that goes into CHANGELOG.md):

Speeds up PromQL queries over TimeSeries tables: the per-series aggregation now groups raw samples by the raw id column and converts the id to a tag group with timeSeriesIdToGroup once per series after the aggregation, instead of once per sample before it.

Description

A PromQL selector transpiled to

SELECT timeSeriesIdToGroup(id) AS group, timestamp, value FROM timeSeriesSelector(...)

so the id→group lookup in the per-query tags collector (ContextTimeSeriesTagsCollector::getGroupByID) ran once per sample — ~4.4 billion times for a 30-day range query over 25,600 series. StoreMethod::RAW_DATA now carries the raw id column; its consumers aggregate with GROUP BY id and project timeSeriesIdToGroup(id) AS group right after:

SELECT timeSeriesIdToGroup(id) AS group, timeSeriesRateToGrid(...)(timestamp, value) AS `values`
FROM (SELECT id, timestamp, value FROM timeSeriesSelector(...)) GROUP BY id

so the lookup runs once per series, and every later step of the generated statement is byte-identical (verified by diffing the full generated SQL).

Measured on a TimeSeries table with 62.5 billion samples (32 cores), cold, results fingerprint-identical:

query shape	master	this PR	delta
sum by(ns)(rate(m[5m])), 30 d range, 25.6k series	40.6 s / 1106 s CPU	27.8 s / 745 s CPU	-32% wall / -33% CPU
quantile(0.95, gauge), 30 d range, 25.6k series	34.1 s / 934 s CPU	19.7 s / 568 s CPU	-42% wall / -39% CPU
instant rule join over 2 x 25.6k-series metrics	8.5 s	5.0 s	-42%
instant unless-join alert shape	8.7 s	5.7 s	-34%
instant label_replace join	2.0 s	1.4 s	-29%

Complementary to #113580 (memoized lookups): measured on top of it, this PR still gains an additional -12% to -39% on the same shapes, because it removes the per-sample conversion instead of accelerating it.

Semantics are preserved: the collector is populated by the tags subquery (CreatingSets) before the samples scan, and grouping by id equals grouping by group because distinct ids cannot share a tag set (the id is a hash of (metric_name, tags)). The one deliberate edge-case change — two distinct ids carrying an identical tag set, constructible only by manually inserting inconsistent rows into the inner tags table — now yields one row per id from selectors, and rate()-style paths fail with the existing "Multiple series have the same tags" check instead of silently merging; the test pins all three behaviors. As a side benefit, the heaviest GROUP BY key is now the leading sorting-key column of the samples table, which is groundwork for order-based aggregation later.

Tests: 04812_prometheus_query_group_by_raw_id.sql (instant + range: rate, sum by, group_left/unless joins, @ modifier, an error canary for a pre-existing unrelated applyOffset scale bug, the duplicated-tag-set edge case) and tests/performance/promql_query.xml (5,000 series, 7.2M samples).

🤖 Generated with Claude Code

[clickhouse-gh]

Workflow [PR], commit [9531ef3]

Summary: ✅

• Performance Comparison: Performance dashboard

---

AI Review

Summary

This PR speeds up PromQL over TimeSeries tables by carrying raw id values through selector output and converting them to groups only after the per-series aggregation. The optimization makes sense on valid data, but the current implementation breaks the existing one-series-per-labelset contract on duplicated inner-table ids: selector paths can now return duplicate label sets or feed duplicate group values into VECTOR_GRID, and unchanged downstream operators still assume that uniqueness.

Findings

❌ Blockers

• [src/Storages/TimeSeries/PrometheusQueryToSQL/applyFunctionOverRange.cpp:304; src/Storages/TimeSeries/PrometheusQueryToSQL/finalizeSQL.cpp:416] Grouping RAW_DATA by raw id instead of timeSeriesIdToGroup(id) breaks the existing uniqueness contract as soon as two ids carry the same tag set, which the new stateless test deliberately constructs via the inner tables. applyFunctionOverRange now returns a VECTOR_GRID with repeated group values even though SQLQueryPiece.h still documents group as unique, and unchanged code such as applySimpleBinaryOperator.cpp:118-152 skips timeSeriesThrowDuplicateSeriesIf in that case and can collapse one duplicate through ANY joins instead of throwing. The top-level RAW_DATA finalizer has the same problem for range selectors, returning duplicate tags rows rather than rejecting the invalid result. Preserve uniqueness by group or add explicit duplicate detection before these paths return selector results.

Final Verdict

❌ Changes requested: the selector paths need to keep the group / label-set uniqueness invariant intact.

LLVM Coverage Report

Metric	Baseline	Current	Δ
Lines	86.50%	86.50%	+0.00%
Functions	91.90%	91.90%	+0.00%
Branches	78.80%	78.80%	+0.00%

Changed lines: Changed C/C++ lines covered: 43/45 (95.56%) · Uncovered code

Full report · Diff report

[clickhouse-gh]

StoreMethod::VECTOR_GRID still promises one row per group, but GROUP BY id no longer guarantees that. The new duplicate-id test demonstrates the bad case: last_over_time(...) and plain selectors can now produce two rows with the same group whenever two inner-table ids carry the same tag set.

That is more than an edge-case presentation change. Unchanged vector operators still rely on group uniqueness: applySimpleBinaryOperator.cpp:118-152 skips timeSeriesThrowDuplicateSeriesIf when join_group is just group and then uses ANY joins. A query like http_requests_total{ns="a",pod="p1"} == http_requests_total{ns="a",pod="p1"} can therefore drop one duplicate silently instead of raising the existing Multiple series have the same tags exception.

We need to re-establish uniqueness before returning VECTOR_GRID here, or explicitly throw on duplicate timeSeriesIdToGroup(id) values.

[clickhouse-gh]

This RAW_DATA finalizer has the same duplicate-labelset problem on the top-level range-selector path. GROUP BY id means prometheusQuery('ts', 'http_requests_total{...}[90s]', ...) now returns one row per raw id, not one row per label set, so two ids with identical tags surface as duplicate tags rows; the new stateless test pins exactly that behavior.

For PromQL results we normally reject duplicate series instead of returning an invalid vector, and the rest of this subsystem already uses timeSeriesThrowDuplicateSeriesIf to enforce that contract. Please keep the final range-vector output unique by timeSeriesIdToGroup(id) or throw when several ids map to the same group.

[clickhouse-gh]

📊 Cloud Performance Report

✅ AI verdict: no_change — no significant changes across 37 queries analysed

This change is confined to PromQL-to-SQL generation for the experimental TimeSeries engine (grouping raw samples by id instead of precomputed group), plus a new perf test and stateless test. The only flagged query, ClickBench Q18, does not touch any of that code path, so its apparent 9.5% improvement is run-to-run variance and has been downgraded to not_sure. No genuine PR-attributable regressions or improvements were found in the flagged set.

clickbench

⚠️ 1 inconclusive

Flagged queries (1 of 43)

	Query	Verdict	Baseline median (ms)	PR median (ms)	Change	q-value	Hint
⚠️	18	not_sure	1326	1200	-9.5%	<0.0001	This PR only rewrites PromQL/TimeSeries RAW_DATA SQL generation, which ClickBench Q18 never executes, so the ×1.1 faster (-9.5%) reading is run-to-run variance, not a PR effect.

_{Change = percent below ×2; the ratio of medians (×N faster/slower) beyond, where percent understates the scale. q-value = BH-FDR adjusted p; smaller is stronger evidence. MIRAI flags a query when q < fdr_q (default 0.10) — the value the verdict is based on.}

tpch_adapted_1_official

🟢 No significant changes

Debug info

• StressHouse run: 46ca2d8c-dbeb-4819-86a6-0afda61fa347
• MIRAI run: 76404581-4d58-4ed7-be24-330a21f33e53
• PR check IDs:
  • clickbench_35417_1786023994
  • clickbench_35427_1786023994
  • clickbench_35435_1786023994
  • tpch_adapted_1_official_35452_1786023994
  • tpch_adapted_1_official_35473_1786023994
  • tpch_adapted_1_official_35487_1786023994