Skip to content

PromQL: One path of evaluation in simple math functions#98235

Merged
alexey-milovidov merged 3 commits intoClickHouse:masterfrom
vitlibar:promql-one-path-in-simple-math-functions
Mar 1, 2026
Merged

PromQL: One path of evaluation in simple math functions#98235
alexey-milovidov merged 3 commits intoClickHouse:masterfrom
vitlibar:promql-one-path-in-simple-math-functions

Conversation

@vitlibar
Copy link
Member

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: One path of evaluation in simple math functions

@vitlibar vitlibar added the comp-promql Issues related to the PromQL support and TimeSeries table engine. label Feb 27, 2026
@clickhouse-gh
Copy link
Contributor

clickhouse-gh bot commented Feb 27, 2026

Workflow [PR], commit [c6f0b78]

Summary:

@clickhouse-gh clickhouse-gh bot added the pr-not-for-changelog This PR should not be mentioned in the changelog label Feb 27, 2026

case StoreMethod::CONST_SCALAR:
{
res.scalar_value = (impl_info->evaluate_with_const_argument)(argument.scalar_value);
Copy link
Member Author

@vitlibar vitlibar Feb 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now we use the same path of evaluation for StoreMethod::CONST_SCALAR and StoreMethod::SINGLE_SCALAR.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR simplifies PromQL “simple math function” evaluation by removing the separate C++ evaluation path for constant scalars and instead always generating a ClickHouse SQL expression (so constants and non-constants use the same evaluation path). The integration tests are updated accordingly, including disabling some checks for functions where ClickHouse numeric results differ from Prometheus.

Changes:

  • Refactor applyMathSimpleFunction() to use a single SQL-building path for CONST_SCALAR and SINGLE_SCALAR.
  • Simplify the function implementation map to only store the ClickHouse function name.
  • Adjust integration tests for math functions (comment out exp() and tanh() checks; change ln(vector(...)) input value).

Reviewed changes

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

File Description
src/Storages/TimeSeries/PrometheusQueryToSQL/applyMathSimpleFunction.cpp Removes per-function C++ constant evaluation and unifies evaluation via ClickHouse function AST generation.
tests/integration/test_prometheus_protocols/test_evaluation.py Updates math function expectations and disables some assertions for functions with known ClickHouse accuracy/consistency issues.

@vitlibar vitlibar force-pushed the promql-one-path-in-simple-math-functions branch from b3db572 to bfddff6 Compare February 27, 2026 19:51
@vitlibar vitlibar requested a review from Copilot February 27, 2026 19:53
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

@vitlibar vitlibar force-pushed the promql-one-path-in-simple-math-functions branch from bfddff6 to 2987cda Compare February 27, 2026 20:29
@vitlibar vitlibar requested a review from Copilot February 27, 2026 20:31
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

@vitlibar vitlibar force-pushed the promql-one-path-in-simple-math-functions branch 9 times, most recently from 32d9491 to d19b528 Compare March 1, 2026 14:56
@vitlibar vitlibar requested a review from Copilot March 1, 2026 14:59
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

@vitlibar vitlibar force-pushed the promql-one-path-in-simple-math-functions branch from d19b528 to b550ecd Compare March 1, 2026 16:18
@vitlibar vitlibar requested a review from Copilot March 1, 2026 16:18
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

@vitlibar vitlibar force-pushed the promql-one-path-in-simple-math-functions branch from b550ecd to c6f0b78 Compare March 1, 2026 16:33
@alexey-milovidov alexey-milovidov self-assigned this Mar 1, 2026
@alexey-milovidov alexey-milovidov added this pull request to the merge queue Mar 1, 2026
Merged via the queue into ClickHouse:master with commit 1b33a06 Mar 1, 2026
148 checks passed
@robot-ch-test-poll robot-ch-test-poll added the pr-synced-to-cloud The PR is synced to the cloud repo label Mar 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp-promql Issues related to the PromQL support and TimeSeries table engine. pr-not-for-changelog This PR should not be mentioned in the changelog pr-synced-to-cloud The PR is synced to the cloud repo

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants