Skip to content

Better check for tuple size in SSD cache complex key external dictionaries#14313

Merged
alexey-milovidov merged 4 commits intomasterfrom
fix-fuzz-test6
Sep 2, 2020
Merged

Better check for tuple size in SSD cache complex key external dictionaries#14313
alexey-milovidov merged 4 commits intomasterfrom
fix-fuzz-test6

Conversation

@alexey-milovidov
Copy link
Copy Markdown
Member

@alexey-milovidov alexey-milovidov commented Aug 31, 2020

Changelog category (leave one):

  • Bug Fix

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Better check for tuple size in SSD cache complex key external dictionaries. This fixes #13981.

Detailed description / Documentation draft:
See https://clickhouse-test-reports.s3.yandex.net/14232/b38c5a844c4178db8f201cf10fb8ef08f769fdff/fuzzer/server.log

No test because the possibility of specifying path in SSD cache dictionary will be removed soon.

@robot-clickhouse robot-clickhouse added the pr-bugfix Pull request with bugfix, not backported by default label Aug 31, 2020
@alexey-milovidov alexey-milovidov added the testing Special issue with list of bugs found by CI label Aug 31, 2020
@alexey-milovidov alexey-milovidov changed the title Better check for tuple size in complex key external dictionaries Better check for tuple size in SSD cache complex key external dictionaries Aug 31, 2020
@alexey-milovidov alexey-milovidov marked this pull request as ready for review August 31, 2020 23:13
@alexey-milovidov
Copy link
Copy Markdown
Member Author

CC @nikvas0

assert(key_columns.size() == key_types.size());
assert(key_columns.size() == dict_struct.key->size());

dict_struct.validateKeyTypes(key_types);
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only two calls to validateKeyTypes matter.
Everything else is just "defensive programming".

@nikitamikhaylov nikitamikhaylov self-assigned this Sep 1, 2020
const auto & structure = dict->getStructure();
assert(structure.key);
size_t key_size = structure.key->size();
if (key_columns.size() != key_size)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems it doesn't work for polygon dictionaries because they have Array(Array(Array(Array(Float64)))) as key of size 1. And third argument in dictGet is tuple(x, y) of size 2.

@alexey-milovidov
Copy link
Copy Markdown
Member Author

Ok. I will remove the check from functions.

@alexey-milovidov alexey-milovidov merged commit 7bd31fb into master Sep 2, 2020
@alexey-milovidov alexey-milovidov deleted the fix-fuzz-test6 branch September 2, 2020 21:50
robot-clickhouse pushed a commit that referenced this pull request Sep 2, 2020
robot-clickhouse pushed a commit that referenced this pull request Sep 2, 2020
robot-clickhouse pushed a commit that referenced this pull request Sep 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-bugfix Pull request with bugfix, not backported by default testing Special issue with list of bugs found by CI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

An error in SSD Cache dictionary (found by fuzzer)

3 participants