Mark getClientHTTPHeader as non-deterministic by alexey-milovidov · Pull Request #108029 · ClickHouse/ClickHouse · GitHub
Skip to content

Mark getClientHTTPHeader as non-deterministic#108029

Merged
Algunenano merged 1 commit into
masterfrom
fix-getclienthttpheader-non-deterministic
Jun 24, 2026
Merged

Mark getClientHTTPHeader as non-deterministic#108029
Algunenano merged 1 commit into
masterfrom
fix-getclienthttpheader-non-deterministic

Conversation

@alexey-milovidov

@alexey-milovidov alexey-milovidov commented Jun 20, 2026

Copy link
Copy Markdown
Member

getClientHTTPHeader returns a per-request value (it reads the HTTP headers of the current request), so it is non-deterministic. It did not override isDeterministic, so it inherited the default true. As a consequence the query result cache treated queries using it as cacheable, which could return a stale header value for a later request. Other runtime-dependent functions (queryID, currentDatabase, connectionId, initialQueryID) already override isDeterministic to return false.

This revives the change from the closed PR #102766 (closed for process reasons), adds a regression test, and rebases it onto current master.

Closes: #101120
Related: #102766

Changelog category (leave one):

  • Bug Fix (user-visible misbehavior in an official stable release)

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

getClientHTTPHeader is now correctly treated as non-deterministic, so its result is no longer incorrectly reused by the query result cache.

Documentation entry for user-facing changes

  • Documentation is written (mandatory for new features)

Version info

  • Merged into: 26.6.1.1198

getClientHTTPHeader reads per-request HTTP headers but inherited the default
isDeterministic() == true, so the query result cache could reuse a stale value
for a later request. Other runtime-dependent functions (queryID, currentDatabase,
connectionId, initialQueryID) already override isDeterministic to return false.

Closes: #101120
Related: #102766

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@clickhouse-gh

clickhouse-gh Bot commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

@clickhouse-gh clickhouse-gh Bot added the pr-bugfix Pull request with bugfix, not backported by default label Jun 20, 2026
@clickhouse-gh

clickhouse-gh Bot commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

LLVM Coverage Report

Metric Baseline Current Δ
Lines 85.30% 85.30% +0.00%
Functions 92.50% 92.60% +0.10%
Branches 77.50% 77.50% +0.00%

Changed lines: Changed C/C++ lines covered by tests: 3/4 (75.00%) | Lost baseline coverage: none · Uncovered code

Full report · Diff report

@Algunenano Algunenano self-assigned this Jun 24, 2026
@Algunenano Algunenano enabled auto-merge June 24, 2026 08:11
@Algunenano Algunenano added this pull request to the merge queue Jun 24, 2026
Merged via the queue into master with commit 2d882b3 Jun 24, 2026
326 of 328 checks passed
@Algunenano Algunenano deleted the fix-getclienthttpheader-non-deterministic branch June 24, 2026 08:35
@robot-ch-test-poll robot-ch-test-poll added the pr-synced-to-cloud The PR is synced to the cloud repo label Jun 24, 2026
alexey-milovidov added a commit that referenced this pull request Jun 24, 2026
Add changelog entries for PRs that landed on the 26.6 release line after
the first pass: #82414, #108042 (New Feature), #107428 (Improvement),
and #108128, #108288, #100205, #108029 (Bug Fix).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@clickgapai

Copy link
Copy Markdown
Contributor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-bugfix Pull request with bugfix, not backported by default 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.

getClientHTTPHeader missing isDeterministic()=false override, same bug class as connectionId()

4 participants