Skip to content

Conversation

@vitlibar
Copy link
Member

@vitlibar vitlibar commented Jan 26, 2026

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: Simplify storing ranges and offsets in PrometheusQueryTree,
use DateTime64 instead of DecimalField<DateTime64>.

Part of #89356

@clickhouse-gh
Copy link

clickhouse-gh bot commented Jan 26, 2026

Workflow [PR], commit [e48a4a3]

Summary:

job_name test_name status info comment
Stress test (amd_debug) failure
Logical error: Block structure mismatch in A stream: different number of columns: (STID: 0993-38e6) FAIL cidb, issue ISSUE EXISTS
Stress test (amd_tsan) failure
Logical error: Block structure mismatch in A stream: different number of columns: (STID: 0993-2da1) FAIL cidb, issue ISSUE EXISTS
Stress test (amd_msan) failure
MemorySanitizer: use-of-uninitialized-value (STID: 3785-4c97) FAIL cidb, issue ISSUE CREATED
AST fuzzer (amd_debug) failure
Logical error: Block structure mismatch in A stream: different number of columns: (STID: 0993-38e6) FAIL cidb, issue ISSUE EXISTS
Performance Comparison (amd_release, master_head, 1/6) failure
Check Results failure IGNORED

@clickhouse-gh clickhouse-gh bot added the pr-not-for-changelog This PR should not be mentioned in the changelog label Jan 26, 2026
@vitlibar vitlibar changed the title Simplify PromQL tree decimals Simplify using decimals in PrometheusQueryTree Jan 26, 2026
@vitlibar vitlibar changed the title Simplify using decimals in PrometheusQueryTree PromQL: Simplify using decimals in PrometheusQueryTree Jan 26, 2026
@vitlibar vitlibar added the comp-promql Issues related to the PromQL support and TimeSeries table engine. label Jan 26, 2026
@vitlibar vitlibar requested a review from Copilot January 26, 2026 20:21
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 the handling of ranges and offsets in the Prometheus query parser by replacing DecimalField<DateTime64> and DecimalField<Decimal64> wrapper types with direct DateTime64 and Decimal64 primitive types throughout PrometheusQueryTree.

Changes:

  • Replaced ScalarLiteral and IntervalLiteral nodes with a single Scalar node type
  • Removed DecimalField wrappers in favor of storing DateTime64 and Decimal64 values directly
  • Updated parsing utilities to work with the new type system and take a timestamp_scale parameter

Reviewed changes

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

Show a summary per file
File Description
src/TableFunctions/TableFunctionPrometheusQuery.cpp Retrieves timestamp scale from storage metadata and passes it to the query parser
src/Storages/TimeSeries/PrometheusQueryToSQL.cpp Refactored to use primitive decimal types instead of DecimalField wrappers, simplified arithmetic operations
src/Storages/StoragePrometheusQuery.cpp Added whitespace formatting
src/Parsers/Prometheus/tests/gtest_PrometheusQueryTree.cpp Updated test expectations to reflect renamed node types and simplified output format
src/Parsers/Prometheus/PrometheusQueryTree.h Modified node structure to store primitive types and added timestamp_scale field to the tree
src/Parsers/Prometheus/PrometheusQueryTree.cpp Updated node implementations to use primitive types and pass tree context for dumping
src/Parsers/Prometheus/PrometheusQueryParsingUtil.h Restructured parsing API to separate scalar, timestamp, and duration parsing with explicit scale parameters
src/Parsers/Prometheus/PrometheusQueryParsingUtil.cpp Implemented new parsing functions with proper overflow checking and scale handling
src/Parsers/Prometheus/PrometheusQueryParsingUtil-antlr.cpp Updated ANTLR-based parser to use new parsing utilities and store values directly in nodes
src/Core/DecimalFunctions.h Added tryMultiplyAdd helper function for overflow-safe decimal arithmetic

@vitlibar vitlibar force-pushed the simplify-promql-tree-decimals branch from 1af45ba to e48a4a3 Compare January 26, 2026 21:49
@nikitamikhaylov nikitamikhaylov self-assigned this Jan 26, 2026
@vitlibar
Copy link
Member Author

Ready for review

@nikitamikhaylov nikitamikhaylov added this pull request to the merge queue Jan 29, 2026
Merged via the queue into ClickHouse:master with commit 8af83c4 Jan 30, 2026
128 of 134 checks passed
@robot-ch-test-poll1 robot-ch-test-poll1 added the pr-synced-to-cloud The PR is synced to the cloud repo label Jan 30, 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.

3 participants