Fix on-disk format breakage for secondary indices over Nullable column#27197
Merged
alexey-milovidov merged 2 commits intoClickHouse:masterfrom Aug 5, 2021
Merged
Fix on-disk format breakage for secondary indices over Nullable column#27197alexey-milovidov merged 2 commits intoClickHouse:masterfrom
alexey-milovidov merged 2 commits intoClickHouse:masterfrom
Conversation
[1] breaks on disk format (and the relevant change in the: [1]: ClickHouse#12455 (comment) Too bad that I checked this patchset only for compatibility after reverting this patch [2] (use case: I've applied it manually, then revert it, and data skipping indexes over Nullable column had been broken) [2]: ClickHouse#12455 (comment) But this patchset actually breaks compatibility with older versions of clickhouse for Nullable data skipping indexes after simple upgrade: Here is a simple reproducer: -- -- run this with 21.6 or similar (i.e. w/o this patch) -- CREATE TABLE data ( `key` Int, `value` Nullable(Int), INDEX value_index value TYPE minmax GRANULARITY 1 ) ENGINE = MergeTree ORDER BY key; INSERT INTO data SELECT number, number FROM numbers(10000); SELECT * FROM data WHERE value = 20000 SETTINGS force_data_skipping_indices = 'value_index' SETTINGS force_data_skipping_indices = 'value_index', max_rows_to_read=1; Now upgrade and run the query again: SELECT * FROM data WHERE value = 20000 SETTINGS force_data_skipping_indices = 'value_index' SETTINGS force_data_skipping_indices = 'value_index', max_rows_to_read=1; And it will fail because of on disk format changes: $ ll --time-style=+ data/*/data/all_1_1_0/skp*.idx -rw-r----- 1 azat azat 36 data/with_nullable_patch/data/all_1_1_0/skp_idx_value_index.idx -rw-r----- 1 azat azat 37 data/without_nullable_patch/data/all_1_1_0/skp_idx_value_index.idx $ md5sum data/*/data/all_1_1_0/skp*.idx a19c95c4a14506c65665a1e30ab404bf data/with_nullable_patch/data/all_1_1_0/skp_idx_value_index.idx e50e2fcfa873b232196623d56ab26105 data/without_nullable_patch/data/all_1_1_0/skp_idx_value_index.idx Note, that there is no stable release with this patch included yet, so no need to backport. Also note that you may create data skipping indexes over Nullable column even before [3]. [3]: ClickHouse#12455 v2: break cases when granulas has Null in values due to backward compatibility
5668e1f to
dee27fc
Compare
Member
Author
|
As a long-term solution I think new format for Nullable indexed should be added, with a new name, that will be able to skip granulas with Null in it (see modifications in tests). |
amosbird
reviewed
Aug 5, 2021
Contributor
21.8 was already forked. |
Member
Author
21.8 does not includes #12455 |
09c3762 to
76af33f
Compare
Contributor
You're right (thought it is). |
39f8257 to
0ba6efc
Compare
Member
|
@Mergifyio update |
Contributor
|
Command
|
6d76081 to
dee27fc
Compare
amosbird
reviewed
Aug 5, 2021
alexey-milovidov
approved these changes
Aug 5, 2021
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Fix on-disk format breakage for secondary indices over Nullable column (no stable release had been affected)
Detailed description / Documentation draft:
1 breaks on disk format.
I checked this patchset for compatibility after
reverting this patch 2 (use case: I've applied it manually, then
revert it, and data skipping indexes over Nullable column had been
broken)
But this patchset actually breaks compatibility with older versions of
clickhouse for Nullable data skipping indexes after simple upgrade:
Details
Here is a simple reproducer:
Now upgrade and run the query again:
And it will fail because of on disk format changes:
Note, that there is no stable release with this patch included yet, so
no need to backport.
Also note that you may create data skipping indexes over Nullable
column even before 3.
Changelog:
Details
Cc: @alexey-milovidov (please add no-backport label)
Cc: @amosbird (please take a look)