From d476a4a9bdacec244a8119f84aea7004d87ab1fa Mon Sep 17 00:00:00 2001 From: Azat Khuzhin Date: Sat, 17 Apr 2021 16:25:57 +0300 Subject: [PATCH] Add type conversion for optimize_skip_unused_shards_rewrite_in MSan reports [1]: 2021.04.17 15:20:06.665152 [ 56 ] {2336bf92-0269-4acd-8b3f-f09623223d18} executeQuery: (from [::1]:44744, using production parser) SELECT * FROM dist_01757 WHERE dummy IN ('255', 0) FORMAT Null ... 0 0x305af885 in (anonymous namespace)::shardContains() obj-x86_64-linux-gnu/../src/Interpreters/OptimizeShardingKeyRewriteInVisitor.cpp:50:28 ... Uninitialized value was created by an allocation of 'sharding_value' in the stack frame of function '_ZN12_GLOBAL__N_113shardContainsERKN2DB5FieldERKNSt3__112basic_stringIcNS4_11char_traitsIcEENS4_9allocatorIcEEEERKNS4_10shared_ptrINS0_17ExpressionActionsEEESC_RKNS0_7Cluster9ShardInfoERKNS4_6vectorImNS8_ImEEEE' 0 0x305ae260 in (anonymous namespace)::shardContains() obj-x86_64-linux-gnu/../src/Interpreters/OptimizeShardingKeyRewriteInVisitor.cpp:42 [1]: https://clickhouse-test-reports.s3.yandex.net/23212/d02eb3f6b7a8dc07bbed5605e1afcf7634436b3a/fuzzer_msan/report.html#fail1 P.S. fuzzers are great! --- .../OptimizeShardingKeyRewriteInVisitor.cpp | 8 ++++++++ ...optimize_skip_unused_shards_rewrite_in.reference | 3 +++ ...01756_optimize_skip_unused_shards_rewrite_in.sql | 13 ++++++++++++- 3 files changed, 23 insertions(+), 1 deletion(-) diff --git a/src/Interpreters/OptimizeShardingKeyRewriteInVisitor.cpp b/src/Interpreters/OptimizeShardingKeyRewriteInVisitor.cpp index a031b8dc962..399def00006 100644 --- a/src/Interpreters/OptimizeShardingKeyRewriteInVisitor.cpp +++ b/src/Interpreters/OptimizeShardingKeyRewriteInVisitor.cpp @@ -4,6 +4,7 @@ #include #include #include +#include #include namespace @@ -52,6 +53,13 @@ bool shardContains( return false; Field sharding_value = executeFunctionOnField(sharding_column_value, sharding_column_name, sharding_expr, sharding_key_column_name); + /// The value from IN can be non-numeric, + /// but in this case it should be convertible to numeric type, let's try. + sharding_value = convertFieldToType(sharding_value, DataTypeUInt64()); + /// In case of conversion is not possible (NULL), shard cannot contain the value anyway. + if (sharding_value.isNull()) + return false; + UInt64 value = sharding_value.get(); const auto shard_num = slots[value % slots.size()] + 1; return shard_info.shard_num == shard_num; diff --git a/tests/queries/0_stateless/01756_optimize_skip_unused_shards_rewrite_in.reference b/tests/queries/0_stateless/01756_optimize_skip_unused_shards_rewrite_in.reference index 494e9ca3237..a1bfcf043da 100644 --- a/tests/queries/0_stateless/01756_optimize_skip_unused_shards_rewrite_in.reference +++ b/tests/queries/0_stateless/01756_optimize_skip_unused_shards_rewrite_in.reference @@ -17,6 +17,9 @@ others 0 0 0 +different types -- prohibited +different types -- conversion +0 optimize_skip_unused_shards_limit 0 0 diff --git a/tests/queries/0_stateless/01756_optimize_skip_unused_shards_rewrite_in.sql b/tests/queries/0_stateless/01756_optimize_skip_unused_shards_rewrite_in.sql index 59e2ad75fcc..dc481ccca72 100644 --- a/tests/queries/0_stateless/01756_optimize_skip_unused_shards_rewrite_in.sql +++ b/tests/queries/0_stateless/01756_optimize_skip_unused_shards_rewrite_in.sql @@ -5,6 +5,7 @@ drop table if exists dist_01756; drop table if exists dist_01756_str; +drop table if exists dist_01756_column; drop table if exists data_01756_str; -- SELECT @@ -90,8 +91,10 @@ select * from dist_01756 where dummy in (0); -- { serverError 507 } -- optimize_skip_unused_shards does not support non-constants select * from dist_01756 where dummy in (select * from system.one); -- { serverError 507 } select * from dist_01756 where dummy in (toUInt8(0)); -- { serverError 507 } --- wrong type +-- wrong type (tuple) select * from dist_01756 where dummy in ('0'); -- { serverError 507 } +-- intHash64 does not accept string +select * from dist_01756 where dummy in ('0', '2'); -- { serverError 43 } -- NOT IN does not supported select * from dist_01756 where dummy not in (0, 2); -- { serverError 507 } @@ -110,6 +113,7 @@ select (2 IN (2,)), * from dist_01756 where dummy in (0, 2) format Null; select (dummy IN (toUInt8(2),)), * from dist_01756 where dummy in (0, 2) format Null; -- different type +select 'different types -- prohibited'; create table data_01756_str (key String) engine=Memory(); create table dist_01756_str as data_01756_str engine=Distributed(test_cluster_two_shards, currentDatabase(), data_01756_str, cityHash64(key)); select * from dist_01756_str where key in ('0', '2'); @@ -117,6 +121,12 @@ select * from dist_01756_str where key in ('0', Null); -- { serverError 507 } select * from dist_01756_str where key in (0, 2); -- { serverError 53 } select * from dist_01756_str where key in (0, Null); -- { serverError 53 } +-- different type #2 +select 'different types -- conversion'; +create table dist_01756_column as system.one engine=Distributed(test_cluster_two_shards, system, one, dummy); +select * from dist_01756_column where dummy in (0, '255'); +select * from dist_01756_column where dummy in (0, '255foo'); -- { serverError 53 } + -- optimize_skip_unused_shards_limit select 'optimize_skip_unused_shards_limit'; select * from dist_01756 where dummy in (0, 2) settings optimize_skip_unused_shards_limit=1; -- { serverError 507 } @@ -124,4 +134,5 @@ select * from dist_01756 where dummy in (0, 2) settings optimize_skip_unused_sha drop table dist_01756; drop table dist_01756_str; +drop table dist_01756_column; drop table data_01756_str;