Skip to content

Fixbug merge tree reader rows incorrect#90254

Merged
scanhex12 merged 4 commits intoClickHouse:masterfrom
fastio:fixbug-MergeTreeReaderRowsIncorrect
Nov 19, 2025
Merged

Fixbug merge tree reader rows incorrect#90254
scanhex12 merged 4 commits intoClickHouse:masterfrom
fastio:fixbug-MergeTreeReaderRowsIncorrect

Conversation

@fastio
Copy link
Copy Markdown
Contributor

@fastio fastio commented Nov 18, 2025 •
edited by clickhouse-gh bot
Loading

Changelog category (leave one):

  • Critical Bug Fix (crash, data loss, RBAC) or LOGICAL_ERROR

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

Fixes a row-count mismatch in MergeTreeReaderIndex when the part's only has fewer rows than index_granularity. Resolves #89691

Details

When a part consists of only one granule, and its total number of rows is less than the configured index_granularity, the current implementation of MergeTreeReaderIndex unconditionally returns index_granularity rows.
The subsequent reader in the chain (e.g., MergeTreeReader) then reads the actual number of rows from the part.
This discrepancy leads to a mismatch during the continueReadingChain consistency check and triggers a LOGICAL_ERROR.

How to reproduce it?

https://fiddle.clickhouse.com/73b7c1a9-ec7f-462e-9e79-b507e1b2b948

I previously opened a PR #89555 that mixed fixes for two different bugs. I'm splitting them now and will submit separate PRs for each one.

@scanhex12 scanhex12 added the can be tested Allows running workflows for external contributors label Nov 18, 2025
@scanhex12 scanhex12 self-assigned this Nov 18, 2025
@clickhouse-gh
Copy link
Copy Markdown
Contributor

clickhouse-gh bot commented Nov 18, 2025 •
edited
Loading

Workflow [PR], commit [8901c4b]

Summary: ❌

job_name test_name status info comment
Stateless tests (arm_asan, targeted) failure
01459_manual_write_to_replicas_quorum_detach_attach FAIL cidb
Bugfix validation (functional tests) failure
03534_skip_index_bug89691 FAIL cidb
Stress test (amd_tsan) failure
Server died FAIL cidb
Hung check failed, possible deadlock found (see hung_check.log) FAIL cidb
Killed by signal (in clickhouse-server.log) FAIL cidb
Fatal message in clickhouse-server.log (see fatal_messages.txt) FAIL cidb
Killed by signal (output files) FAIL cidb
Found signal in gdb.log FAIL cidb
BuzzHouse (amd_debug) failure
Logical error: 'Inconsistent AST formatting: the query: FAIL cidb
BuzzHouse (arm_asan) failure
Logical error: 'false && "WriteBuffer is neither finalized nor canceled in destructor."'. FAIL cidb
BuzzHouse (amd_ubsan) failure
Received signal 17 FAIL cidb

@clickhouse-gh clickhouse-gh bot added pr-critical-bugfix pr-must-backport Pull request should be backported intentionally. Use this label with great care! labels Nov 18, 2025
Ergus
Ergus approved these changes Nov 18, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@Ergus Ergus left a comment

Choose a reason for hiding this comment

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

This used to be part of #89555, so it is already reviewed ;)

@scanhex12 scanhex12 enabled auto-merge November 19, 2025 12:46
@scanhex12 scanhex12 added this pull request to the merge queue Nov 19, 2025
Merged via the queue into ClickHouse:master with commit 99a5e6f Nov 19, 2025
125 of 132 checks passed
@robot-ch-test-poll4 robot-ch-test-poll4 added the pr-synced-to-cloud The PR is synced to the cloud repo label Nov 19, 2025
@robot-ch-test-poll robot-ch-test-poll added the pr-must-backport-synced The `*-must-backport` labels are synced into the cloud Sync PR label Nov 19, 2025
This was referenced Nov 19, 2025
Closed
Closed
Closed
Closed
Merged
robot-clickhouse-ci-2 added a commit that referenced this pull request Nov 19, 2025
@robot-clickhouse-ci-2
Merge pull request #90380 from ClickHouse/cherrypick/25.11/90254
19a98ef 
Cherry pick #90254 to 25.11: Fixbug merge tree reader rows incorrect
robot-clickhouse added a commit that referenced this pull request Nov 19, 2025
@robot-clickhouse
Backport #90254 to 25.11: Fixbug merge tree reader rows incorrect
4f0e2e5
@robot-clickhouse-ci-2 robot-clickhouse-ci-2 mentioned this pull request Nov 19, 2025
Merged
clickhouse-gh bot added a commit that referenced this pull request Nov 19, 2025
@clickhouse-gh
Merge pull request #90381 from ClickHouse/backport/25.11/90254
cbe8a75 
Backport #90254 to 25.11: Fixbug merge tree reader rows incorrect
@nickitat nickitat mentioned this pull request Nov 21, 2025
Closed
@rschu1ze
Copy link
Copy Markdown
Member

rschu1ze commented Dec 2, 2025

@scanhex12 The backport PRs are unmerged since two weeks. Could I ask you to take care of them please? Thanks -

@rschu1ze
Copy link
Copy Markdown
Member

rschu1ze commented Dec 8, 2025 •
edited
Loading

@scanhex12 The backport PRs are unmerged since three weeks. Could I ask you to take care of them please? Thanks -

@rschu1ze
Copy link
Copy Markdown
Member

rschu1ze commented Dec 16, 2025 •
edited
Loading

@scanhex12 The backport PRs are unmerged since four weeks. Could I ask you to take care of them please? Thanks -

@robot-ch-test-poll robot-ch-test-poll mentioned this pull request Jan 31, 2026
Closed
@robot-clickhouse-ci-1 robot-clickhouse-ci-1 added the pr-backports-created Backport PRs are successfully created, it won't be processed by CI script anymore label Feb 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Ergus Ergus Ergus approved these changes

@scanhex12 scanhex12 scanhex12 approved these changes

Assignees

@scanhex12 scanhex12

Labels

can be tested Allows running workflows for external contributors pr-backports-created Backport PRs are successfully created, it won't be processed by CI script anymore pr-critical-bugfix 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

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Logical error when reading part with granule smaller than index_granularity

7 participants

@fastio @rschu1ze @Ergus @scanhex12 @robot-ch-test-poll @robot-ch-test-poll4 @robot-clickhouse-ci-1