From beffb92411ad726e2b2302934f0c37b9223359c4 Mon Sep 17 00:00:00 2001 From: avogar Date: Thu, 12 Sep 2024 14:51:59 +0000 Subject: [PATCH 1/5] Keep original order of conditions during move to prewhere --- .../MergeTree/MergeTreeWhereOptimizer.cpp | 21 +++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/src/Storages/MergeTree/MergeTreeWhereOptimizer.cpp b/src/Storages/MergeTree/MergeTreeWhereOptimizer.cpp index f0c26c302e1..76a02bbd2c4 100644 --- a/src/Storages/MergeTree/MergeTreeWhereOptimizer.cpp +++ b/src/Storages/MergeTree/MergeTreeWhereOptimizer.cpp @@ -361,11 +361,23 @@ std::optional MergeTreeWhereOptimizer:: UInt64 total_size_of_moved_conditions = 0; UInt64 total_number_of_moved_columns = 0; + /// Remember positions of conditions in where_conditions list + /// to keep original order of conditions in prewhere_conditions while moving. + std::unordered_map condition_positions; + size_t position= 0; + for (const auto & condition : where_conditions) + condition_positions[&condition] = position++; + /// Move condition and all other conditions depend on the same set of columns. auto move_condition = [&](Conditions::iterator cond_it) { LOG_TRACE(log, "Condition {} moved to PREWHERE", cond_it->node.getColumnName()); - prewhere_conditions.splice(prewhere_conditions.end(), where_conditions, cond_it); + /// Keep the original order of conditions in prewhere_conditions. + position = condition_positions[&(*cond_it)]; + auto prewhere_it = prewhere_conditions.begin(); + while (condition_positions[&(*prewhere_it)] < position && prewhere_it != prewhere_conditions.end()) + ++prewhere_it; + prewhere_conditions.splice(prewhere_it, where_conditions, cond_it); total_size_of_moved_conditions += cond_it->columns_size; total_number_of_moved_columns += cond_it->table_columns.size(); @@ -375,7 +387,12 @@ std::optional MergeTreeWhereOptimizer:: if (jt->viable && jt->columns_size == cond_it->columns_size && jt->table_columns == cond_it->table_columns) { LOG_TRACE(log, "Condition {} moved to PREWHERE", jt->node.getColumnName()); - prewhere_conditions.splice(prewhere_conditions.end(), where_conditions, jt++); + /// Keep the original order of conditions in prewhere_conditions. + position = condition_positions[&(*jt)]; + prewhere_it = prewhere_conditions.begin(); + while (condition_positions[&(*prewhere_it)] < position && prewhere_it != prewhere_conditions.end()) + ++prewhere_it; + prewhere_conditions.splice(prewhere_it, where_conditions, jt++); } else { From 401a3d09317d4cbe401d64cdf50c056a5c08e63a Mon Sep 17 00:00:00 2001 From: avogar Date: Thu, 12 Sep 2024 15:10:29 +0000 Subject: [PATCH 2/5] Add test --- .../0_stateless/03231_prewhere_conditions_order.reference | 1 + .../queries/0_stateless/03231_prewhere_conditions_order.sql | 6 ++++++ 2 files changed, 7 insertions(+) create mode 100644 tests/queries/0_stateless/03231_prewhere_conditions_order.reference create mode 100644 tests/queries/0_stateless/03231_prewhere_conditions_order.sql diff --git a/tests/queries/0_stateless/03231_prewhere_conditions_order.reference b/tests/queries/0_stateless/03231_prewhere_conditions_order.reference new file mode 100644 index 00000000000..bb14c5f88f2 --- /dev/null +++ b/tests/queries/0_stateless/03231_prewhere_conditions_order.reference @@ -0,0 +1 @@ +1 [0,1] [0,1] diff --git a/tests/queries/0_stateless/03231_prewhere_conditions_order.sql b/tests/queries/0_stateless/03231_prewhere_conditions_order.sql new file mode 100644 index 00000000000..acaba12684c --- /dev/null +++ b/tests/queries/0_stateless/03231_prewhere_conditions_order.sql @@ -0,0 +1,6 @@ +drop table if exists test; +create table test (x UInt32, arr1 Array(UInt32), arr2 Array(UInt32)) engine=MergeTree order by x; +insert into test values (1, [0, 1], [0, 1]), (2, [0], [0, 1]); +select * from test where x == 1 and arrayExists((x1, x2) -> (x1 == x2), arr1, arr2); +drop table test; + From 2e82e06330502b7c64a9bf5812f66a6147e79539 Mon Sep 17 00:00:00 2001 From: avogar Date: Thu, 12 Sep 2024 16:59:25 +0000 Subject: [PATCH 3/5] Update tests --- tests/queries/0_stateless/00000_test.reference | 2 ++ tests/queries/0_stateless/00000_test.sh | 12 ++++++++++++ tests/queries/0_stateless/00000_test.sql | 14 ++++++++++++++ .../02156_storage_merge_prewhere.reference | 14 +++++++------- .../02842_move_pk_to_end_of_prewhere.reference | 10 +++++----- ...tics_delayed_materialization_in_merge.reference | 4 ++-- .../0_stateless/02864_statistics_usage.reference | 8 ++++---- 7 files changed, 46 insertions(+), 18 deletions(-) create mode 100644 tests/queries/0_stateless/00000_test.reference create mode 100755 tests/queries/0_stateless/00000_test.sh create mode 100644 tests/queries/0_stateless/00000_test.sql diff --git a/tests/queries/0_stateless/00000_test.reference b/tests/queries/0_stateless/00000_test.reference new file mode 100644 index 00000000000..676c099a485 --- /dev/null +++ b/tests/queries/0_stateless/00000_test.reference @@ -0,0 +1,2 @@ +42 +select 42 diff --git a/tests/queries/0_stateless/00000_test.sh b/tests/queries/0_stateless/00000_test.sh new file mode 100755 index 00000000000..ae20733a597 --- /dev/null +++ b/tests/queries/0_stateless/00000_test.sh @@ -0,0 +1,12 @@ +#!/usr/bin/env bash +# Tags: no-fasttest + +CURDIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) +# shellcheck source=../shell_config.sh +. "$CURDIR"/../shell_config.sh + + +$CLICKHOUSE_CLIENT --query_id="my_id_$CLICKHOUSE_TEST_UNIQUE_NAME" -q "select 42"; +$CLICKHOUSE_CLIENT -q "system flush logs"; +$CLICKHOUSE_CLIENT -q "select query from system.query_log where query_id='my_id_$CLICKHOUSE_TEST_UNIQUE_NAME' and type='QueryFinish'" + diff --git a/tests/queries/0_stateless/00000_test.sql b/tests/queries/0_stateless/00000_test.sql new file mode 100644 index 00000000000..266af42f3db --- /dev/null +++ b/tests/queries/0_stateless/00000_test.sql @@ -0,0 +1,14 @@ +-- Tags: long, no-tsan, no-msan, no-ubsan, no-asan + +set allow_experimental_dynamic_type = 1; +set merge_tree_read_split_ranges_into_intersecting_and_non_intersecting_injection_probability = 1; + +drop table if exists test; + +create table test (id UInt64, d Dynamic) engine=MergeTree order by id settings min_rows_for_wide_part=1, min_bytes_for_wide_part=1, use_adaptive_write_buffer_for_dynamic_subcolumns=1, min_bytes_for_full_part_storage=100000000000; + +insert into test select number, if (number % 5 == 1, ('str_' || number)::LowCardinality(String)::Dynamic, number::Dynamic) from numbers(100000) settings min_insert_block_size_rows=50000; + +select count() from test where dynamicType(d) == 'UInt64'; + +drop table test; diff --git a/tests/queries/0_stateless/02156_storage_merge_prewhere.reference b/tests/queries/0_stateless/02156_storage_merge_prewhere.reference index 876cee60baa..8aa1f0b59d3 100644 --- a/tests/queries/0_stateless/02156_storage_merge_prewhere.reference +++ b/tests/queries/0_stateless/02156_storage_merge_prewhere.reference @@ -1,25 +1,25 @@ Prewhere info Prewhere filter - Prewhere filter column: and(notEmpty(v), equals(k, 3)) (removed) + Prewhere filter column: and(equals(k, 3), notEmpty(v)) (removed) Prewhere info Prewhere filter - Prewhere filter column: and(notEmpty(v), equals(k, 3)) (removed) + Prewhere filter column: and(equals(k, 3), notEmpty(v)) (removed) Prewhere info Prewhere filter - Prewhere filter column: and(notEmpty(v), equals(k, 3)) (removed) + Prewhere filter column: and(equals(k, 3), notEmpty(v)) (removed) Prewhere info Prewhere filter - Prewhere filter column: and(notEmpty(v), equals(k, 3)) (removed) + Prewhere filter column: and(equals(k, 3), notEmpty(v)) (removed) 2 Filter column: and(equals(k, 3), notEmpty(v)) (removed) Prewhere info Prewhere filter - Prewhere filter column: and(notEmpty(v), equals(k, 3)) (removed) + Prewhere filter column: and(equals(k, 3), notEmpty(v)) (removed) 2 Prewhere info Prewhere filter - Prewhere filter column: and(notEmpty(v), equals(k, 3)) (removed) + Prewhere filter column: and(equals(k, 3), notEmpty(v)) (removed) Prewhere info Prewhere filter - Prewhere filter column: and(notEmpty(v), equals(k, 3)) (removed) + Prewhere filter column: and(equals(k, 3), notEmpty(v)) (removed) 2 diff --git a/tests/queries/0_stateless/02842_move_pk_to_end_of_prewhere.reference b/tests/queries/0_stateless/02842_move_pk_to_end_of_prewhere.reference index b91a4dd2f68..254e59d479a 100644 --- a/tests/queries/0_stateless/02842_move_pk_to_end_of_prewhere.reference +++ b/tests/queries/0_stateless/02842_move_pk_to_end_of_prewhere.reference @@ -1,15 +1,15 @@ Prewhere filter - Prewhere filter column: and(notEmpty(v), equals(k, 3)) (removed) + Prewhere filter column: and(equals(k, 3), notEmpty(v)) (removed) 1 Prewhere filter - Prewhere filter column: and(like(d, \'%es%\'), less(c, 20), equals(b, \'3\'), equals(a, 3)) (removed) + Prewhere filter column: and(equals(a, 3), equals(b, \'3\'), less(c, 20), like(d, \'%es%\')) (removed) 1 Prewhere filter - Prewhere filter column: and(like(d, \'%es%\'), less(c, 20), greater(c, 0), equals(a, 3)) (removed) + Prewhere filter column: and(equals(a, 3), less(c, 20), greater(c, 0), like(d, \'%es%\')) (removed) 1 Prewhere filter - Prewhere filter column: and(like(d, \'%es%\'), equals(b, \'3\'), less(c, 20)) (removed) + Prewhere filter column: and(equals(b, \'3\'), less(c, 20), like(d, \'%es%\')) (removed) 1 Prewhere filter - Prewhere filter column: and(like(d, \'%es%\'), equals(b, \'3\'), equals(a, 3)) (removed) + Prewhere filter column: and(equals(a, 3), equals(b, \'3\'), like(d, \'%es%\')) (removed) 1 diff --git a/tests/queries/0_stateless/02864_statistics_delayed_materialization_in_merge.reference b/tests/queries/0_stateless/02864_statistics_delayed_materialization_in_merge.reference index eb5e685597c..c4ef127ebc0 100644 --- a/tests/queries/0_stateless/02864_statistics_delayed_materialization_in_merge.reference +++ b/tests/queries/0_stateless/02864_statistics_delayed_materialization_in_merge.reference @@ -5,8 +5,8 @@ After insert After merge Prewhere info Prewhere filter - Prewhere filter column: and(less(a, 10_UInt8), less(b, 10_UInt8)) (removed) + Prewhere filter column: and(less(b, 10_UInt8), less(a, 10_UInt8)) (removed) After truncate, insert, and materialize Prewhere info Prewhere filter - Prewhere filter column: and(less(a, 10_UInt8), less(b, 10_UInt8)) (removed) + Prewhere filter column: and(less(b, 10_UInt8), less(a, 10_UInt8)) (removed) diff --git a/tests/queries/0_stateless/02864_statistics_usage.reference b/tests/queries/0_stateless/02864_statistics_usage.reference index a9f669b88c1..fd4181a59c3 100644 --- a/tests/queries/0_stateless/02864_statistics_usage.reference +++ b/tests/queries/0_stateless/02864_statistics_usage.reference @@ -1,7 +1,7 @@ After insert Prewhere info Prewhere filter - Prewhere filter column: and(less(a, 10_UInt8), less(b, 10_UInt8)) (removed) + Prewhere filter column: and(less(b, 10_UInt8), less(a, 10_UInt8)) (removed) After drop statistic Prewhere info Prewhere filter @@ -9,12 +9,12 @@ After drop statistic After add and materialize statistic Prewhere info Prewhere filter - Prewhere filter column: and(less(a, 10_UInt8), less(b, 10_UInt8)) (removed) + Prewhere filter column: and(less(b, 10_UInt8), less(a, 10_UInt8)) (removed) After merge Prewhere info Prewhere filter - Prewhere filter column: and(less(a, 10_UInt8), less(b, 10_UInt8)) (removed) + Prewhere filter column: and(less(b, 10_UInt8), less(a, 10_UInt8)) (removed) After rename Prewhere info Prewhere filter - Prewhere filter column: and(less(a, 10_UInt8), less(c, 10_UInt8)) (removed) + Prewhere filter column: and(less(c, 10_UInt8), less(a, 10_UInt8)) (removed) From 9c1f4f4545dae17824a7d772cb9d1fab11c4f0db Mon Sep 17 00:00:00 2001 From: avogar Date: Thu, 12 Sep 2024 17:21:28 +0000 Subject: [PATCH 4/5] Remove bad files --- tests/queries/0_stateless/00000_test.reference | 2 -- tests/queries/0_stateless/00000_test.sh | 12 ------------ tests/queries/0_stateless/00000_test.sql | 14 -------------- 3 files changed, 28 deletions(-) delete mode 100644 tests/queries/0_stateless/00000_test.reference delete mode 100755 tests/queries/0_stateless/00000_test.sh delete mode 100644 tests/queries/0_stateless/00000_test.sql diff --git a/tests/queries/0_stateless/00000_test.reference b/tests/queries/0_stateless/00000_test.reference deleted file mode 100644 index 676c099a485..00000000000 --- a/tests/queries/0_stateless/00000_test.reference +++ /dev/null @@ -1,2 +0,0 @@ -42 -select 42 diff --git a/tests/queries/0_stateless/00000_test.sh b/tests/queries/0_stateless/00000_test.sh deleted file mode 100755 index ae20733a597..00000000000 --- a/tests/queries/0_stateless/00000_test.sh +++ /dev/null @@ -1,12 +0,0 @@ -#!/usr/bin/env bash -# Tags: no-fasttest - -CURDIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) -# shellcheck source=../shell_config.sh -. "$CURDIR"/../shell_config.sh - - -$CLICKHOUSE_CLIENT --query_id="my_id_$CLICKHOUSE_TEST_UNIQUE_NAME" -q "select 42"; -$CLICKHOUSE_CLIENT -q "system flush logs"; -$CLICKHOUSE_CLIENT -q "select query from system.query_log where query_id='my_id_$CLICKHOUSE_TEST_UNIQUE_NAME' and type='QueryFinish'" - diff --git a/tests/queries/0_stateless/00000_test.sql b/tests/queries/0_stateless/00000_test.sql deleted file mode 100644 index 266af42f3db..00000000000 --- a/tests/queries/0_stateless/00000_test.sql +++ /dev/null @@ -1,14 +0,0 @@ --- Tags: long, no-tsan, no-msan, no-ubsan, no-asan - -set allow_experimental_dynamic_type = 1; -set merge_tree_read_split_ranges_into_intersecting_and_non_intersecting_injection_probability = 1; - -drop table if exists test; - -create table test (id UInt64, d Dynamic) engine=MergeTree order by id settings min_rows_for_wide_part=1, min_bytes_for_wide_part=1, use_adaptive_write_buffer_for_dynamic_subcolumns=1, min_bytes_for_full_part_storage=100000000000; - -insert into test select number, if (number % 5 == 1, ('str_' || number)::LowCardinality(String)::Dynamic, number::Dynamic) from numbers(100000) settings min_insert_block_size_rows=50000; - -select count() from test where dynamicType(d) == 'UInt64'; - -drop table test; From 2812953a8ac38cddab356d2e24856ea8c8eb7ab1 Mon Sep 17 00:00:00 2001 From: avogar Date: Fri, 13 Sep 2024 13:37:42 +0000 Subject: [PATCH 5/5] Try to fix tests --- tests/queries/1_stateful/00091_prewhere_two_conditions.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/queries/1_stateful/00091_prewhere_two_conditions.sql b/tests/queries/1_stateful/00091_prewhere_two_conditions.sql index cd88743160c..649c63f2ec9 100644 --- a/tests/queries/1_stateful/00091_prewhere_two_conditions.sql +++ b/tests/queries/1_stateful/00091_prewhere_two_conditions.sql @@ -7,7 +7,7 @@ SET optimize_move_to_prewhere = 1; SET enable_multiple_prewhere_read_steps = 1; SELECT uniq(URL) FROM test.hits WHERE toTimeZone(EventTime, 'Asia/Dubai') >= '2014-03-20 00:00:00' AND toTimeZone(EventTime, 'Asia/Dubai') < '2014-03-21 00:00:00'; -SELECT uniq(URL) FROM test.hits WHERE toTimeZone(EventTime, 'Asia/Dubai') >= '2014-03-20 00:00:00' AND URL != '' AND toTimeZone(EventTime, 'Asia/Dubai') < '2014-03-21 00:00:00'; +SELECT uniq(URL) FROM test.hits WHERE toTimeZone(EventTime, 'Asia/Dubai') >= '2014-03-20 00:00:00' AND toTimeZone(EventTime, 'Asia/Dubai') < '2014-03-21 00:00:00' AND URL != ''; SELECT uniq(*) FROM test.hits WHERE toTimeZone(EventTime, 'Asia/Dubai') >= '2014-03-20 00:00:00' AND toTimeZone(EventTime, 'Asia/Dubai') < '2014-03-21 00:00:00' AND EventDate = '2014-03-21'; WITH toTimeZone(EventTime, 'Asia/Dubai') AS xyz SELECT uniq(*) FROM test.hits WHERE xyz >= '2014-03-20 00:00:00' AND xyz < '2014-03-21 00:00:00' AND EventDate = '2014-03-21';