Skip to content

Fix conversions for fields in function 'arrayElement' for type Map#21699

Merged
CurtizJ merged 3 commits intoClickHouse:masterfrom
CurtizJ:fix-map-field-conversion
Mar 23, 2021
Merged

Fix conversions for fields in function 'arrayElement' for type Map#21699
CurtizJ merged 3 commits intoClickHouse:masterfrom
CurtizJ:fix-map-field-conversion

Conversation

@CurtizJ
Copy link
Copy Markdown
Member

@CurtizJ CurtizJ commented Mar 13, 2021

I hereby agree to the terms of the CLA available at: https://yandex.ru/legal/cla/?lang=en

Changelog category (leave one):

  • Bug Fix

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Fix function arrayElement with type Map for constant integer arguments.

@robot-clickhouse robot-clickhouse added the pr-bugfix Pull request with bugfix, not backported by default label Mar 13, 2021
@CurtizJ CurtizJ force-pushed the fix-map-field-conversion branch from 0e65393 to 68b3cbb Compare March 13, 2021 14:49
@vitlibar vitlibar self-assigned this Mar 15, 2021

SELECT m[-1], m[0], m[toInt128(1234567898765432123456789)], m[toInt128(-1234567898765432123456789)] FROM table_map_with_key_integer;
SELECT m[toUInt64(0)], m[toInt64(0)], m[toUInt8(0)], m[toUInt16(0)] FROM table_map_with_key_integer;

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 line 70 should be

SELECT 'Map(Int128, String)';

instead of

SELECT 'Map(Int128, Int32)';

Could you fix that?

return false;

bool is_integer_field = Field::dispatch([](const auto & value)
std::optional<DataType> index_value;
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.

index_as_integer seems to be more clear name


bool is_integer_field = Field::dispatch([](const auto & value)
std::optional<DataType> index_value;
Field::dispatch([&index_value](const auto & value)
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.

[&] is enough here

@CurtizJ
Copy link
Copy Markdown
Member Author

CurtizJ commented Mar 22, 2021

Had to change range function to array literal, because, the following query, which was generated by fuzzer doesn't fail by memory limit, but leads to OOM in build with TSan. (report)

SELECT range(65535), m[toDate(number)] FROM table_map_with_key_integer ARRAY JOIN range(65536) AS number

See #22008.

@CurtizJ
Copy link
Copy Markdown
Member Author

CurtizJ commented Mar 23, 2021

Failed tests are not related.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants