iframe-proxy

vitlibar · 2026-05-11T08:08:17Z

Changelog category (leave one):

Not for changelog (changelog entry is not required)

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

PromQL: Add functions label_replace and label_join

Version info

Merged into: 26.5.1.817

clickhouse-gh · 2026-05-11T08:08:55Z

Workflow [PR], commit [43c2d3a]

Summary: ✅

AI Review

Summary

This PR adds PromQL label_replace / label_join support in the converter, updates replacement parsing to match Prometheus ExpandString rules, and adds broad integration/stateless coverage. One correctness issue is still live: the metric_name_dropped flag is forced from destination label name rather than from actual resulting labels, so it can become inconsistent with produced groups when __name__ is rewritten to an empty value.

Findings

❌ Blockers

[src/Storages/TimeSeries/PrometheusQueryToSQL/applyLabelManipulationFunction.cpp:279] res.metric_name_dropped is forced to false whenever dest_label == "__name__", but timeSeriesJoinTags / timeSeriesReplaceTag remove destination tags when computed value is empty. Example: label_join(up, "__name__", "_") removes __name__, yet the flag is set as if metric name is present. This violates the converter’s own metric-name state invariant and can mislead downstream group/name propagation logic.
Suggested fix: do not force metric_name_dropped = false based only on destination label name. Either (a) conservatively keep the previous value unless non-empty __name__ is guaranteed, or (b) make this path compute/propagate whether __name__ can be empty and update the flag accordingly. The same issue exists in the scalar branch at [src/Storages/TimeSeries/PrometheusQueryToSQL/applyLabelManipulationFunction.cpp:207].

Tests

⚠️ Add a focused regression covering label_join(..., "__name__", ...) / label_replace(..., "__name__", ...) where replacement can be empty, and assert both resulting labels and downstream behavior in a follow-up operation (e.g. binary/aggregation path that relies on metric-name state).

Final Verdict

Status: ⚠️ Request changes
Minimum required actions:

Fix metric_name_dropped updates in applyLabelManipulationFunction so they match actual post-transform labelsets when writing __name__.
Add a regression test for empty-result __name__ rewrites.

Copilot

Pull request overview

This PR adds PromQL label manipulation support by implementing label_replace() and label_join() in the PrometheusQueryToSQL converter, backed by existing ClickHouse time-series tag functions, and extends the replacement-pattern handling to better match Prometheus behavior.

Changes:

Implement PromQL label_replace / label_join conversion to SQL via timeSeriesReplaceTag / timeSeriesJoinTags.
Extend replacement-pattern parsing for timeSeriesReplaceTag (multi-digit refs, braced refs, literal $, etc.) and allow empty src_tag to match PromQL semantics.
Add/extend stateless and integration tests covering label manipulation and replacement edge cases.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
tests/queries/0_stateless/03779_time_series_tags_functions.sql	Adds stateless SQL coverage for replacement-pattern edge cases.
tests/queries/0_stateless/03779_time_series_tags_functions.reference	Updates expected output for the new stateless test block.
tests/integration/test_prometheus_protocols/test_evaluation.py	Adds integration tests for PromQL `label_replace` and `label_join` behavior and errors.
src/Storages/TimeSeries/PrometheusQueryToSQL/applyLabelManipulationFunction.h	Declares helpers for recognizing/applying label manipulation functions.
src/Storages/TimeSeries/PrometheusQueryToSQL/applyLabelManipulationFunction.cpp	Implements `label_replace` / `label_join` conversion and duplicate-labelset protection.
src/Storages/TimeSeries/PrometheusQueryToSQL/applyFunctionScalar.cpp	Adjusts scalar aggregation behavior for instant (`start_time == end_time`) evaluation.
src/Storages/TimeSeries/PrometheusQueryToSQL/applyFunction.cpp	Wires label manipulation functions into the function dispatch path.
src/Interpreters/ContextTimeSeriesTagsCollector.cpp	Updates replacement-pattern parsing logic to more closely follow Prometheus semantics.
src/Functions/TimeSeries/timeSeriesReplaceTag.cpp	Allows empty `src_tag` to support PromQL tricks like constant label insertion.

Copilot

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.

vitlibar · 2026-05-11T16:19:32Z

@groeneai Investigate CI failures

groeneai · 2026-05-11T17:19:43Z

@vitlibar — investigated CI failures on fc1c72d (Unit tests ran at 11:51 UTC). All chronic / infra, none PR-caused — detail below.

1. `Unit tests (asan_ubsan, function_prop_fuzzer)` — `FunctionsStress` (Logical error: `prev.thread_id == curr.thread_id`) — CHRONIC, FIX MERGED

This is the exact bug fixed by your own PR #103982 (merged 2026-05-11 14:55:17 UTC). PR #104564's CI run was 11:51 UTC, ~3h before the fix. 30-day CIDB: 56 hits / 26 distinct PRs / 7 master hits — all today, all on commits predating the fix.

2. `Unit tests (amd_llvm_coverage)` — `FileCacheTest.SLRUFreeSpaceKeepingProtectedOnly` — CHRONIC

30-day CIDB: 21 hits today / 20 unrelated PRs + chronic since 2026-05-08 (introduced by PR #104313 on 2026-05-08 randomizing keep_free_space_elements_ratio; the test races with FileCache async eviction). Currently being fixed in PR #104418 (@groeneai, in CI) with keep_free_space_elements_ratio=0 to bypass the race; failure expected message is Expected equality: cache->getFileSegmentsNum() = 3, expected 0, which matches.

3. `Finish Workflow` — `Post Hooks/check_report_messages.py` — CIDB INFRASTRUCTURE

Read timed out. (read timeout=20) writing to play.clickhouse.com across many sub-jobs (Stateless tests, Build, Fast test, etc). Same global CIDB throughput issue affecting PR #104563's performance comparison shards today.

Recommendation

Rebase against current master (post #103982) and re-run CI; FunctionsStress should clear immediately. FileCacheTest may still flake until #104418 lands; LLVM Coverage Unit tests is a known chronic until then.

alexey-milovidov · 2026-05-12T16:51:12Z

The FileCacheTest.SLRUFreeSpaceKeepingProtectedOnly failure on this PR is a known flaky test that was fixed in #104418. Could you please update the branch with the latest master to pick up the fix?

…ible

Copilot

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

src/Storages/TimeSeries/PrometheusQueryToSQL/applyLabelManipulationFunction.cpp:279

This unconditionally clears metric_name_dropped when writing to __name__, but the tag functions drop dest_tag if the computed destination value is empty. Cases like joining only missing source labels into __name__ therefore leave the actual group without __name__ while downstream code treats it as present; avoid forcing the flag to false unless the produced value cannot be empty.

            if (dest_label == kMetricName)
                res.metric_name_dropped = false;

clickhouse-gh · 2026-05-15T20:00:33Z

LLVM Coverage Report

Changed lines: 71.30% (164/230) | lost baseline coverage: 13 line(s) · Uncovered code

Full report · Diff report

vitlibar added the comp-promql PromQL / time-series subsystem: TimeSeries storage engine, PromQL parser, PromQL-to-SQL converter... label May 11, 2026

vitlibar requested a review from Copilot May 11, 2026 08:08

clickhouse-gh Bot added the pr-not-for-changelog This PR should not be mentioned in the changelog label May 11, 2026

Copilot started reviewing on behalf of vitlibar May 11, 2026 08:09 View session

clickhouse-gh Bot reviewed May 11, 2026

View reviewed changes

Comment thread src/Storages/TimeSeries/PrometheusQueryToSQL/applyLabelManipulationFunction.cpp

Copilot AI reviewed May 11, 2026

View reviewed changes

vitlibar marked this pull request as draft May 11, 2026 08:26

vitlibar force-pushed the promql-label-replace branch from a7dc5ca to 6cf9805 Compare May 11, 2026 10:35

vitlibar requested a review from Copilot May 11, 2026 10:35

Copilot started reviewing on behalf of vitlibar May 11, 2026 10:36 View session

Copilot AI reviewed May 11, 2026

View reviewed changes

vitlibar force-pushed the promql-label-replace branch from 6cf9805 to b43963b Compare May 11, 2026 11:03

nikitamikhaylov self-assigned this May 11, 2026

vitlibar force-pushed the promql-label-replace branch from b43963b to fc1c72d Compare May 11, 2026 11:20

vitlibar marked this pull request as ready for review May 11, 2026 16:19

groeneai mentioned this pull request May 11, 2026

Add functions arrayTopK and arrayBottomK #104563

Merged

Improve function scalar() by using StoreMethod::SINGLE_SCALAR if poss…

6c71fe6

…ible

vitlibar force-pushed the promql-label-replace branch from fc1c72d to 6e023ff Compare May 15, 2026 11:16

clickhouse-gh Bot reviewed May 15, 2026

View reviewed changes

Comment thread src/Storages/TimeSeries/PrometheusQueryToSQL/applyLabelManipulationFunction.cpp

Add functions label_replace(), label_join()

43c2d3a

vitlibar force-pushed the promql-label-replace branch from 72177cb to 43c2d3a Compare May 15, 2026 17:05

vitlibar requested a review from Copilot May 15, 2026 17:06

Copilot started reviewing on behalf of vitlibar May 15, 2026 17:07 View session

Copilot AI reviewed May 15, 2026

View reviewed changes

Comment thread src/Storages/TimeSeries/PrometheusQueryToSQL/applyLabelManipulationFunction.cpp

alexey-milovidov mentioned this pull request May 17, 2026

Stop the bleeding in function_prop_fuzzer #105146

Merged

1 task

nikitamikhaylov approved these changes May 19, 2026

View reviewed changes

nikitamikhaylov added this pull request to the merge queue May 19, 2026

Merged via the queue into ClickHouse:master with commit ba4ced7 May 19, 2026
330 of 334 checks passed

robot-clickhouse-ci-1 added the pr-synced-to-cloud The PR is synced to the cloud repo label May 19, 2026

Sunbelt Computer Software

PL/B Language Development and Support

Uh oh!

Conversation

vitlibar commented May 11, 2026 • edited by robot-clickhouse Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changelog category (leave one):

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

Version info

Uh oh!

clickhouse-gh Bot commented May 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

AI Review

Summary

Findings

Tests

Final Verdict

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vitlibar commented May 11, 2026

Uh oh!

groeneai commented May 11, 2026

1. Unit tests (asan_ubsan, function_prop_fuzzer) — FunctionsStress (Logical error: prev.thread_id == curr.thread_id) — CHRONIC, FIX MERGED

2. Unit tests (amd_llvm_coverage) — FileCacheTest.SLRUFreeSpaceKeepingProtectedOnly — CHRONIC

3. Finish Workflow — Post Hooks/check_report_messages.py — CIDB INFRASTRUCTURE

Recommendation

Uh oh!

alexey-milovidov commented May 12, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

clickhouse-gh Bot commented May 15, 2026

LLVM Coverage Report

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

vitlibar commented May 11, 2026 •

edited by robot-clickhouse

Loading

clickhouse-gh Bot commented May 11, 2026 •

edited

Loading

1. `Unit tests (asan_ubsan, function_prop_fuzzer)` — `FunctionsStress` (Logical error: `prev.thread_id == curr.thread_id`) — CHRONIC, FIX MERGED

2. `Unit tests (amd_llvm_coverage)` — `FileCacheTest.SLRUFreeSpaceKeepingProtectedOnly` — CHRONIC

3. `Finish Workflow` — `Post Hooks/check_report_messages.py` — CIDB INFRASTRUCTURE