Allow any partition strategy to accept part export #1083
Conversation
| return stream_name; | ||
| } | ||
|
|
||
| Block IMergeTreeDataPart::MinMaxIndex::getBlock(const MergeTreeData & data) const |
There was a problem hiding this comment.
Fun fact: I copied this method from my first ClickHouse PR ever (or my second, I don't quite recall). This PR took over a year to be reviewed, and once merged, was reverted within a week :).
|
|
||
| const auto column = data_type->createColumn(); | ||
|
|
||
| const auto min_val = hyperrectangle.at(i).left; |
There was a problem hiding this comment.
it is better to use const auto & here to avoid copying. Also, it looks like this method looses information on inclusion of left and right (Range::left_included and Range::right_included)
There was a problem hiding this comment.
Also, it looks like this method looses information on inclusion of left and right (Range::left_included and Range::right_included)
This is an interesting problem. I could call shrinkToIncludedIfPOssible, but that only works for (U)Int64 type.
There was a problem hiding this comment.
I would really like to avoid reading the merge tree part just to get any value out of it..
|
|
||
| const auto column = data_type->createColumn(); | ||
|
|
||
| auto range = hyperrectangle.at(i); |
There was a problem hiding this comment.
@Enmk maybe I'll undo this change then so we can rely on references since we have reached the conclusion ranges will always be included for minmax? But maybe that's not true
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Allow any partition strategy to accept part export
Documentation entry for user-facing changes
For
wildcardstrategy, it is mandatory to include the{_export_filename}wildcard in the table definition. Example:CREATE TABLE s3_table ... ENGINE=S3(named_collection, filename='table_root/{_partition_id}/{_export_filename}.parquet') PARTITION BY toYYYYMM(...)CI/CD Options
Exclude tests:
Regression jobs to run: