-
Notifications
You must be signed in to change notification settings - Fork 6.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix PartsSplitter #62268
Fix PartsSplitter #62268
Conversation
This is an automated comment for commit 378d330 with description of existing statuses. It's updated for the latest CI running ❌ Click here to open a full report in a separate page
Successful checks
|
apparently problem is coming from recent optimisation of not loading some index columns. splitting on layers was incorrect, for one part instead of range (0, 4) we took only (0, 2). I noticed that for it we use only first column value as the index value, while the index consists of 3 columns. then I set |
This is an automatic comment. The PR descriptions does not match the template. Please, edit it accordingly. The error is: Changelog entry required for category 'Bug Fix' |
@@ -0,0 +1,48 @@ | |||
import pytest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could be a functional test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But the test restarts ClickHouse server.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yep, I'm not aware of a way to trigger index optimisation w/o server restart
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DETACH/ATTACH a table.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.
as some of us already noticed, though it seems there is a different issue, because this test fails almost in 100% cases with the following settings: set merge_tree_read_split_ranges_into_intersecting_and_non_intersecting_injection_probability=0,
allow_prefetched_read_pool_for_remote_filesystem=0,
max_block_size=69749, max_threads=32; yep, it is analyzer. |
5b4f67c
Backport #62268 to 24.3: Fix PartsSplitter
Changelog category (leave one):
Changelog entry:
When some index columns are not loaded into memory for some parts of a *MergeTree table, queries with
FINAL
might produce wrong results. Now we explicitly choose only the common prefix of index columns for all parts to avoid this issue.Related: #62173 #62192 #62252
cc @nikitamikhaylov @jkartseva