Skip to content

Exposing 'RangeReader read N rows, but M expected' error#74472

Closed
devcrafter wants to merge 2 commits intomasterfrom
reader_read_error
Closed

Exposing 'RangeReader read N rows, but M expected' error#74472
devcrafter wants to merge 2 commits intomasterfrom
reader_read_error

Conversation

@devcrafter
Copy link
Copy Markdown
Member

Related to #62741 #56640

The issue is happened to parts with constant index granularity and triggered by having PREWHERE in the query.
So, having a MergeTree table with index_granularity_bytes = 0, index_granularity = 42 settings and 1 row, and reading it, will trigger the following error:
RangeReader read 1 rows, but 42 expected

Technically, MergeTreeIndexGranularityInfo::fixed_index_granularity

/// Fixed size in rows of one granule if index_granularity_bytes is zero
size_t fixed_index_granularity = 0;

is used to initialize MergeTreeIndexGranularityConstant::last_mark_granularity (it's 42 in our example)
Which in turn used to get the number of rows in the part range (one only in our example, i.e. last as well) in MergeTreeIndexGranularityConstant::getRowsCountInRange()


Which will return not a number of rows in the granule but its granularity

CC @CurtizJ, fill free to use this PR for the fix if applicable

Changelog category (leave one):

  • Not for changelog (changelog entry is not required)

Information about CI checks: https://clickhouse.com/docs/en/development/continuous-integration/

CI Settings (Only check the boxes if you know what you are doing)

All builds in Builds_1 and Builds_2 stages are always mandatory
and will run independently of the checks below:

  • Allow: All Required Checks
  • Allow: Stateless tests
  • Allow: Stateful tests
  • Allow: Integration Tests
  • Allow: Performance tests
  • Allow: All Builds
  • Allow: batch 1, 2 for multi-batch jobs
  • Allow: batch 3, 4, 5, 6 for multi-batch jobs

  • Exclude: Style check
  • Exclude: Fast test
  • Exclude: All with ASAN
  • Exclude: All with TSAN, MSAN, UBSAN, Coverage
  • Exclude: All with aarch64
  • Exclude: All with release
  • Exclude: All with debug

  • Run only fuzzers related jobs (libFuzzer fuzzers, AST fuzzers, BuzzHouse, etc.)
  • Exclude: AST fuzzers

  • Do not test
  • Woolen Wolfdog
  • Upload binaries for special builds
  • Disable merge-commit
  • Disable CI cache

@robot-clickhouse-ci-1 robot-clickhouse-ci-1 added the pr-not-for-changelog This PR should not be mentioned in the changelog label Jan 10, 2025
@robot-clickhouse-ci-1
Copy link
Copy Markdown
Contributor

robot-clickhouse-ci-1 commented Jan 10, 2025

This is an automated comment for commit c0055e1 with description of existing statuses. It's updated for the latest CI running

❌ Click here to open a full report in a separate page

Check nameDescriptionStatus
Fast testNormally this is the first check that is ran for a PR. It builds ClickHouse and runs most of stateless functional tests, omitting some. If it fails, further checks are not started until it is fixed. Look at the report to see which tests fail, then reproduce the failure locally as described here❌ failure
Successful checks
Check nameDescriptionStatus
Docs checkBuilds and tests the documentation✅ success
Style checkRuns a set of checks to keep the code style clean. If some of tests failed, see the related log from the report✅ success

@CurtizJ
Copy link
Copy Markdown
Member

CurtizJ commented Jan 30, 2025

Thanks for the investigation and the test. I added your test in the PR with a fix: #75270.

@CurtizJ CurtizJ closed this Jan 30, 2025
@devcrafter devcrafter deleted the reader_read_error branch February 7, 2025 13:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-not-for-changelog This PR should not be mentioned in the changelog

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants