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
Special query plan step for read from MergeTree #22352
Conversation
selectPartsToRead(parts, part_values, minmax_idx_condition, minmax_columns_types, partition_pruner, max_block_numbers_to_read); | ||
selectPartsToRead(parts, part_values, minmax_idx_condition, minmax_columns_types, partition_pruner, max_block_numbers_to_read, part_filter_counters); | ||
|
||
index_stats->emplace_back(ReadFromMergeTree::IndexStat{ |
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.
Maybe we can avoid printing it if we have at least one index condition?
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.
I think it is useful to add.
We can see how indexes drop parts and granules.
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.
LGTM, But I have a question. Why we return QueryPlan
with a single step? Seems like it would be more clear to return the step from storage?
Name: t_minmax | ||
Parts: 1 | ||
Granules: 2 | ||
Skip |
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.
I see Indexes
header, but this Skip
looks slightly confusing. Don't have any ideas on how to improve naming here. Maybe we can use something like Skip (minmax)
or Skip (set)
?
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.
Ok, I will add it
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.
Hm, also it may be confusing cause I grep out the description.
It will be minmax GRANULARITY 2
|
||
/// This step is created to read from MergeTree* table. | ||
/// For now, it takes a list of parts and creates source from it. | ||
class ReadFromMergeTree : public ISourceStep |
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.
final
?
InReverseOrder, | ||
}; | ||
|
||
explicit ReadFromMergeTree( |
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.
explicit looks redundant
Functional stateless tests (release, wide parts enabled)
that's unusual |
Internal documentation ticket: DOCSUP-8737. |
I hereby agree to the terms of the CLA available at: https://yandex.ru/legal/cla/?lang=en
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Add setting
indexes
(boolean, disabled by default) toEXPLAIN PIPELINE
query. When enabled, shows used indexes, number of filtered parts and granules for every index applied. Supported forMergeTree*
tables.