Skip to content

Try not to re-caclculate indexes in requestReadingInOrder#99084

Merged
vdimir merged 1 commit intomasterfrom
vdimir/read_in_order_followup
Mar 10, 2026
Merged

Try not to re-caclculate indexes in requestReadingInOrder#99084
vdimir merged 1 commit intomasterfrom
vdimir/read_in_order_followup

Conversation

@vdimir
Copy link
Member

@vdimir vdimir commented Mar 9, 2026

Changelog category (leave one):

  • Bug Fix

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

  • Skip unnecessary extra index analysis when read-in-order optimization is applied.

Followup for #89815


Note

Medium Risk
Changes ReadFromMergeTree::requestReadingInOrder to stop calling selectRangesToRead(), which could affect how previously-computed analysis results are reused and thus impact read-order execution behavior if assumptions are wrong.

Overview
Avoids re-running index/range selection when switching a ReadFromMergeTree step to read-in-order.

Instead of calling selectRangesToRead() to recompute analysis, requestReadingInOrder() now directly updates the existing AnalysisResult::read_type (when present and not reading from a projection) based on the requested sort direction.

Written by Cursor Bugbot for commit 1dcc299. This will update automatically on new commits. Configure here.

@clickhouse-gh
Copy link
Contributor

clickhouse-gh bot commented Mar 9, 2026

Workflow [PR], commit [1dcc299]

Summary:

job_name test_name status info comment
Upgrade check (amd_release) failure
Error message in clickhouse-server.log (see upgrade_error_messages.txt) FAIL cidb IGNORED

@clickhouse-gh clickhouse-gh bot added the pr-not-for-changelog This PR should not be mentioned in the changelog label Mar 9, 2026
@clickhouse-gh
Copy link
Contributor

clickhouse-gh bot commented Mar 9, 2026

LLVM Coverage Report

Metric Baseline Current Δ
Lines 83.20% 83.80% +0.60%
Functions 23.60% 23.90% +0.30%
Branches 75.30% 76.40% +1.10%

PR changed lines: PR changed-lines coverage: 100.00% (11/11)
Diff coverage report
Uncovered code

@vdimir vdimir requested a review from CurtizJ March 10, 2026 08:01
@vdimir vdimir marked this pull request as ready for review March 10, 2026 08:01
@CurtizJ CurtizJ self-assigned this Mar 10, 2026
@CurtizJ
Copy link
Member

CurtizJ commented Mar 10, 2026

Let's backport this change. So, please write a changelog entry.

@vdimir vdimir added pr-bugfix Pull request with bugfix, not backported by default pr-must-backport Pull request should be backported intentionally. Use this label with great care! and removed pr-not-for-changelog This PR should not be mentioned in the changelog labels Mar 10, 2026
@vdimir vdimir added this pull request to the merge queue Mar 10, 2026
Merged via the queue into master with commit a862ab7 Mar 10, 2026
161 of 163 checks passed
@vdimir vdimir deleted the vdimir/read_in_order_followup branch March 10, 2026 14:09
@robot-clickhouse robot-clickhouse added the pr-must-backport-synced The `*-must-backport` labels are synced into the cloud Sync PR label Mar 10, 2026
@robot-ch-test-poll4 robot-ch-test-poll4 added the pr-synced-to-cloud The PR is synced to the cloud repo label Mar 10, 2026
robot-ch-test-poll2 added a commit that referenced this pull request Mar 10, 2026
Cherry pick #99084 to 25.12: Try not to re-caclculate indexes in requestReadingInOrder
robot-clickhouse added a commit that referenced this pull request Mar 10, 2026
robot-ch-test-poll2 added a commit that referenced this pull request Mar 10, 2026
Cherry pick #99084 to 26.1: Try not to re-caclculate indexes in requestReadingInOrder
robot-clickhouse added a commit that referenced this pull request Mar 10, 2026
robot-ch-test-poll2 added a commit that referenced this pull request Mar 10, 2026
Cherry pick #99084 to 26.2: Try not to re-caclculate indexes in requestReadingInOrder
robot-clickhouse added a commit that referenced this pull request Mar 10, 2026
@robot-ch-test-poll robot-ch-test-poll added the pr-backports-created Backport PRs are successfully created, it won't be processed by CI script anymore label Mar 10, 2026
clickhouse-gh bot added a commit that referenced this pull request Mar 10, 2026
Backport #99084 to 26.2: Try not to re-caclculate indexes in requestReadingInOrder
vdimir added a commit that referenced this pull request Mar 11, 2026
Backport #99084 to 25.12: Try not to re-caclculate indexes in requestReadingInOrder
vdimir added a commit that referenced this pull request Mar 11, 2026
Backport #99084 to 26.1: Try not to re-caclculate indexes in requestReadingInOrder
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-backports-created Backport PRs are successfully created, it won't be processed by CI script anymore pr-bugfix Pull request with bugfix, not backported by default pr-must-backport Pull request should be backported intentionally. Use this label with great care! pr-must-backport-synced The `*-must-backport` labels are synced into the cloud Sync PR 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.

5 participants