Improve support of integer keys in data type Map#21157
Improve support of integer keys in data type Map#21157nikitamikhaylov merged 7 commits intoClickHouse:masterfrom
Conversation
robot-clickhouse
added
the
pr-improvement
nikitamikhaylov
left a comment
nikitamikhaylov
left a comment
There was a problem hiding this comment.
Also there are some questions
|
|
||
| void DataTypeMap::assertKeyType() const | ||
| { | ||
| if (!key_type->isValueRepresentedByInteger() && !isStringOrFixedString(*key_type) && !WhichDataType(key_type).isNothing()) |
There was a problem hiding this comment.
Why you check type Nothing here? How nothing can represent a map key?
There was a problem hiding this comment.
To allow creating of empty maps for queries like this select count() from t where m != map()
| } | ||
| } | ||
|
|
||
| /// For maps |
There was a problem hiding this comment.
Why is it needed? To define equality operation on Map type or I am wrong?
There was a problem hiding this comment.
Yes, it's needed for comparisons.
| { | ||
| return ParserSubquery().parse(pos, node, expected) | ||
| || ParserTupleOfLiterals().parse(pos, node, expected) | ||
| || ParserMapOfLiterals().parse(pos, node, expected) |
There was a problem hiding this comment.
Why? Does this code enable when I query select map(1, 2) ?
There was a problem hiding this comment.
ParserMapOfLiterals was used to parse literals in form of {key : value}, but currently we decided to remove this syntax.
3 participants
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):
Improve support of integer keys in data type
Map.