From d1c45662e8560177eb1e90d5ad159b58ff396f44 Mon Sep 17 00:00:00 2001 From: Igor Nikonov Date: Fri, 26 Jul 2024 20:24:41 +0000 Subject: [PATCH 1/4] Fix: missing conversion in IN operator with parallel replicas --- src/Analyzer/FunctionNode.cpp | 6 ------ ...arallel_replicas_lost_decimal_conversion.reference | 0 ...3209_parallel_replicas_lost_decimal_conversion.sql | 11 +++++++++++ 3 files changed, 11 insertions(+), 6 deletions(-) create mode 100644 tests/queries/0_stateless/03209_parallel_replicas_lost_decimal_conversion.reference create mode 100644 tests/queries/0_stateless/03209_parallel_replicas_lost_decimal_conversion.sql diff --git a/src/Analyzer/FunctionNode.cpp b/src/Analyzer/FunctionNode.cpp index e98b04fe9a9..debed0983fd 100644 --- a/src/Analyzer/FunctionNode.cpp +++ b/src/Analyzer/FunctionNode.cpp @@ -239,12 +239,6 @@ ASTPtr FunctionNode::toASTImpl(const ConvertToASTOptions & options) const if (function_name == "_CAST" && !argument_nodes.empty() && argument_nodes[0]->getNodeType() == QueryTreeNodeType::CONSTANT) new_options.add_cast_for_constants = false; - /// Avoid cast for `IN tuple(...)` expression. - /// Tuples could be quite big, and adding a type may significantly increase query size. - /// It should be safe because set type for `column IN tuple` is deduced from `column` type. - if (isNameOfInFunction(function_name) && argument_nodes.size() > 1 && argument_nodes[1]->getNodeType() == QueryTreeNodeType::CONSTANT) - new_options.add_cast_for_constants = false; - const auto & parameters = getParameters(); if (!parameters.getNodes().empty()) { diff --git a/tests/queries/0_stateless/03209_parallel_replicas_lost_decimal_conversion.reference b/tests/queries/0_stateless/03209_parallel_replicas_lost_decimal_conversion.reference new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tests/queries/0_stateless/03209_parallel_replicas_lost_decimal_conversion.sql b/tests/queries/0_stateless/03209_parallel_replicas_lost_decimal_conversion.sql new file mode 100644 index 00000000000..bcc9dec306b --- /dev/null +++ b/tests/queries/0_stateless/03209_parallel_replicas_lost_decimal_conversion.sql @@ -0,0 +1,11 @@ +DROP TABLE IF EXISTS t_03209 SYNC; + +CREATE TABLE t_03209 ( `a` Decimal(18, 0), `b` Decimal(18, 1), `c` Decimal(36, 0) ) ENGINE = ReplicatedMergeTree('/clickhouse/{database}/test_03209', 'r1') ORDER BY tuple(); +INSERT INTO t_03209 VALUES ('33', '44.4', '35'); + +SET max_parallel_replicas = 2, cluster_for_parallel_replicas='parallel_replicas'; + +SELECT * FROM t_03209 WHERE a IN toDecimal32('33.3000', 4) SETTINGS allow_experimental_parallel_reading_from_replicas=0; +SELECT * FROM t_03209 WHERE a IN toDecimal32('33.3000', 4) SETTINGS allow_experimental_parallel_reading_from_replicas=1; + +DROP TABLE t_03209 SYNC; From 4c7d93dcc87ac4056c73356c155e441e1dffba2a Mon Sep 17 00:00:00 2001 From: Igor Nikonov Date: Sat, 27 Jul 2024 17:03:19 +0000 Subject: [PATCH 2/4] Update test output --- ...imize_skip_unused_shards_rewrite_in.reference | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) 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 28dbb9215a8..0a7a9d64208 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 @@ -24,8 +24,8 @@ select splitByString('IN', query)[-1] from system.query_log where query like concat('%', currentDatabase(), '%AS%id_no%') and type = 'QueryFinish' order by query; - (0, 2) - (0, 2) + _CAST((0, 2), \'Tuple(UInt8, UInt8)\') + _CAST((0, 2), \'Tuple(UInt8, UInt8)\') -- -- w/ optimize_skip_unused_shards_rewrite_in=1 -- @@ -45,8 +45,8 @@ select splitByString('IN', query)[-1] from system.query_log where query like concat('%', currentDatabase(), '%AS%id_02%') and type = 'QueryFinish' order by query; - tuple(0) - tuple(2) + _CAST(tuple(0), \'Tuple(UInt8)\') + _CAST(tuple(2), \'Tuple(UInt8)\') select 'optimize_skip_unused_shards_rewrite_in(2,)'; optimize_skip_unused_shards_rewrite_in(2,) with (select currentDatabase()) as id_2 select *, ignore(id_2) from dist_01756 where dummy in (2,); @@ -59,7 +59,7 @@ select splitByString('IN', query)[-1] from system.query_log where query like concat('%', currentDatabase(), '%AS%id_2%') and type = 'QueryFinish' order by query; - tuple(2) + _CAST(tuple(2), \'Tuple(UInt8)\') select 'optimize_skip_unused_shards_rewrite_in(0,)'; optimize_skip_unused_shards_rewrite_in(0,) with (select currentDatabase()) as id_00 select *, ignore(id_00) from dist_01756 where dummy in (0,); @@ -73,7 +73,7 @@ select splitByString('IN', query)[-1] from system.query_log where query like concat('%', currentDatabase(), '%AS%id_00%') and type = 'QueryFinish' order by query; - tuple(0) + _CAST(tuple(0), \'Tuple(UInt8)\') -- signed column select 'signed column'; signed column @@ -88,8 +88,8 @@ select splitByString('IN', query)[-1] from system.query_log where query like concat('%', currentDatabase(), '%AS%key_signed%') and type = 'QueryFinish' order by query; - tuple(-1) - tuple(-2) + _CAST(tuple(-1), \'Tuple(Int8)\') + _CAST(tuple(-2), \'Tuple(Int8)\') -- not tuple select * from dist_01756 where dummy in (0); 0 From ea06447ca3b5330e09c0426574583b11c2ecaaa0 Mon Sep 17 00:00:00 2001 From: Igor Nikonov Date: Tue, 30 Jul 2024 22:03:29 +0000 Subject: [PATCH 3/4] Fix: skip cast only if constant doesn't have source expression --- src/Analyzer/FunctionNode.cpp | 7 +++++++ ...ize_skip_unused_shards_rewrite_in.reference | 18 +++++++++--------- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/src/Analyzer/FunctionNode.cpp b/src/Analyzer/FunctionNode.cpp index debed0983fd..8e4e0725a2d 100644 --- a/src/Analyzer/FunctionNode.cpp +++ b/src/Analyzer/FunctionNode.cpp @@ -239,6 +239,13 @@ ASTPtr FunctionNode::toASTImpl(const ConvertToASTOptions & options) const if (function_name == "_CAST" && !argument_nodes.empty() && argument_nodes[0]->getNodeType() == QueryTreeNodeType::CONSTANT) new_options.add_cast_for_constants = false; + /// Avoid cast for `IN tuple(...)` expression. + /// Tuples could be quite big, and adding a type may significantly increase query size. + /// It should be safe because set type for `column IN tuple` is deduced from `column` type. + if (isNameOfInFunction(function_name) && argument_nodes.size() > 1 && argument_nodes[1]->getNodeType() == QueryTreeNodeType::CONSTANT + && !static_cast(argument_nodes[1].get())->hasSourceExpression()) + new_options.add_cast_for_constants = false; + const auto & parameters = getParameters(); if (!parameters.getNodes().empty()) { 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 0a7a9d64208..8d064020c1f 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 @@ -24,10 +24,10 @@ select splitByString('IN', query)[-1] from system.query_log where query like concat('%', currentDatabase(), '%AS%id_no%') and type = 'QueryFinish' order by query; - _CAST((0, 2), \'Tuple(UInt8, UInt8)\') - _CAST((0, 2), \'Tuple(UInt8, UInt8)\') + (0, 2) + (0, 2) -- --- w/ optimize_skip_unused_shards_rewrite_in=1 +-- w/ otimize_skip_unused_shards_rewrite_in=1 -- set optimize_skip_unused_shards_rewrite_in=1; @@ -45,8 +45,8 @@ select splitByString('IN', query)[-1] from system.query_log where query like concat('%', currentDatabase(), '%AS%id_02%') and type = 'QueryFinish' order by query; - _CAST(tuple(0), \'Tuple(UInt8)\') - _CAST(tuple(2), \'Tuple(UInt8)\') + tuple(0) + tuple(2) select 'optimize_skip_unused_shards_rewrite_in(2,)'; optimize_skip_unused_shards_rewrite_in(2,) with (select currentDatabase()) as id_2 select *, ignore(id_2) from dist_01756 where dummy in (2,); @@ -59,7 +59,7 @@ select splitByString('IN', query)[-1] from system.query_log where query like concat('%', currentDatabase(), '%AS%id_2%') and type = 'QueryFinish' order by query; - _CAST(tuple(2), \'Tuple(UInt8)\') + tuple(2) select 'optimize_skip_unused_shards_rewrite_in(0,)'; optimize_skip_unused_shards_rewrite_in(0,) with (select currentDatabase()) as id_00 select *, ignore(id_00) from dist_01756 where dummy in (0,); @@ -73,7 +73,7 @@ select splitByString('IN', query)[-1] from system.query_log where query like concat('%', currentDatabase(), '%AS%id_00%') and type = 'QueryFinish' order by query; - _CAST(tuple(0), \'Tuple(UInt8)\') + tuple(0) -- signed column select 'signed column'; signed column @@ -88,8 +88,8 @@ select splitByString('IN', query)[-1] from system.query_log where query like concat('%', currentDatabase(), '%AS%key_signed%') and type = 'QueryFinish' order by query; - _CAST(tuple(-1), \'Tuple(Int8)\') - _CAST(tuple(-2), \'Tuple(Int8)\') + tuple(-1) + tuple(-2) -- not tuple select * from dist_01756 where dummy in (0); 0 From d23e9fc9eea9f21e5168e618f63add35704dda4b Mon Sep 17 00:00:00 2001 From: Igor Nikonov Date: Thu, 1 Aug 2024 13:10:13 +0000 Subject: [PATCH 4/4] Fix text --- ...56_optimize_skip_unused_shards_rewrite_in.reference | 10 +++++----- .../01756_optimize_skip_unused_shards_rewrite_in.sql | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) 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 8d064020c1f..237bca6305a 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 @@ -27,7 +27,7 @@ order by query; (0, 2) (0, 2) -- --- w/ otimize_skip_unused_shards_rewrite_in=1 +-- w/ optimize_skip_unused_shards_rewrite_in=1 -- set optimize_skip_unused_shards_rewrite_in=1; @@ -49,7 +49,7 @@ order by query; tuple(2) select 'optimize_skip_unused_shards_rewrite_in(2,)'; optimize_skip_unused_shards_rewrite_in(2,) -with (select currentDatabase()) as id_2 select *, ignore(id_2) from dist_01756 where dummy in (2,); +with (select currentDatabase()) as id_2 select *, ignore(id_2) from dist_01756 where dummy in (2); system flush logs; select splitByString('IN', query)[-1] from system.query_log where event_date >= yesterday() and @@ -59,10 +59,10 @@ select splitByString('IN', query)[-1] from system.query_log where query like concat('%', currentDatabase(), '%AS%id_2%') and type = 'QueryFinish' order by query; - tuple(2) + (2) select 'optimize_skip_unused_shards_rewrite_in(0,)'; optimize_skip_unused_shards_rewrite_in(0,) -with (select currentDatabase()) as id_00 select *, ignore(id_00) from dist_01756 where dummy in (0,); +with (select currentDatabase()) as id_00 select *, ignore(id_00) from dist_01756 where dummy in (0); 0 0 system flush logs; select splitByString('IN', query)[-1] from system.query_log where @@ -73,7 +73,7 @@ select splitByString('IN', query)[-1] from system.query_log where query like concat('%', currentDatabase(), '%AS%id_00%') and type = 'QueryFinish' order by query; - tuple(0) + (0) -- signed column select 'signed column'; signed column 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 9a1a00cc0a1..0b7a44665dc 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 @@ -63,7 +63,7 @@ select splitByString('IN', query)[-1] from system.query_log where order by query; select 'optimize_skip_unused_shards_rewrite_in(2,)'; -with (select currentDatabase()) as id_2 select *, ignore(id_2) from dist_01756 where dummy in (2,); +with (select currentDatabase()) as id_2 select *, ignore(id_2) from dist_01756 where dummy in (2); system flush logs; select splitByString('IN', query)[-1] from system.query_log where event_date >= yesterday() and @@ -75,7 +75,7 @@ select splitByString('IN', query)[-1] from system.query_log where order by query; select 'optimize_skip_unused_shards_rewrite_in(0,)'; -with (select currentDatabase()) as id_00 select *, ignore(id_00) from dist_01756 where dummy in (0,); +with (select currentDatabase()) as id_00 select *, ignore(id_00) from dist_01756 where dummy in (0); system flush logs; select splitByString('IN', query)[-1] from system.query_log where event_date >= yesterday() and