From 22ca96cf8d44aa907c0c3c463e4b0a5628312aa0 Mon Sep 17 00:00:00 2001 From: Antonio Andelic Date: Thu, 14 Mar 2024 16:05:01 +0000 Subject: [PATCH 1/4] Disable optimize_rewrite_sum_if_to_count_if if return is nullable --- src/Analyzer/Passes/SumIfToCountIfPass.cpp | 2 +- .../0_stateless/03010_sum_to_to_count_if_nullable.reference | 0 .../queries/0_stateless/03010_sum_to_to_count_if_nullable.sql | 3 +++ 3 files changed, 4 insertions(+), 1 deletion(-) create mode 100644 tests/queries/0_stateless/03010_sum_to_to_count_if_nullable.reference create mode 100644 tests/queries/0_stateless/03010_sum_to_to_count_if_nullable.sql diff --git a/src/Analyzer/Passes/SumIfToCountIfPass.cpp b/src/Analyzer/Passes/SumIfToCountIfPass.cpp index 1a6ee9215a9..d374d92c1fb 100644 --- a/src/Analyzer/Passes/SumIfToCountIfPass.cpp +++ b/src/Analyzer/Passes/SumIfToCountIfPass.cpp @@ -32,7 +32,7 @@ public: return; auto * function_node = node->as(); - if (!function_node || !function_node->isAggregateFunction()) + if (!function_node || !function_node->isAggregateFunction() || function_node->getResultType()->isNullable()) return; auto function_name = function_node->getFunctionName(); diff --git a/tests/queries/0_stateless/03010_sum_to_to_count_if_nullable.reference b/tests/queries/0_stateless/03010_sum_to_to_count_if_nullable.reference new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tests/queries/0_stateless/03010_sum_to_to_count_if_nullable.sql b/tests/queries/0_stateless/03010_sum_to_to_count_if_nullable.sql new file mode 100644 index 00000000000..394cd4f1ea5 --- /dev/null +++ b/tests/queries/0_stateless/03010_sum_to_to_count_if_nullable.sql @@ -0,0 +1,3 @@ +SET optimize_rewrite_sum_if_to_count_if = 1; +SELECT (sumIf(toNullable(1), (number % 2) = 0), NULL) FROM numbers(10) SETTINGS allow_experimental_analyzer=0; +SELECT (sumIf(toNullable(1), (number % 2) = 0), NULL) FROM numbers(10) SETTINGS allow_experimental_analyzer=1; \ No newline at end of file From 7e9d863c22e65ce34539670519de096a9f2261e6 Mon Sep 17 00:00:00 2001 From: Antonio Andelic Date: Thu, 14 Mar 2024 18:08:55 +0000 Subject: [PATCH 2/4] Update reference of the test --- .../0_stateless/03010_sum_to_to_count_if_nullable.reference | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/queries/0_stateless/03010_sum_to_to_count_if_nullable.reference b/tests/queries/0_stateless/03010_sum_to_to_count_if_nullable.reference index e69de29bb2d..8627f639a03 100644 --- a/tests/queries/0_stateless/03010_sum_to_to_count_if_nullable.reference +++ b/tests/queries/0_stateless/03010_sum_to_to_count_if_nullable.reference @@ -0,0 +1,2 @@ +(5,NULL) +(5,NULL) From b0008495294761e3394de0060dda61ee66808476 Mon Sep 17 00:00:00 2001 From: Antonio Andelic Date: Thu, 14 Mar 2024 18:15:12 +0000 Subject: [PATCH 3/4] Better --- src/Analyzer/Passes/SumIfToCountIfPass.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Analyzer/Passes/SumIfToCountIfPass.cpp b/src/Analyzer/Passes/SumIfToCountIfPass.cpp index d374d92c1fb..2c41b6dc467 100644 --- a/src/Analyzer/Passes/SumIfToCountIfPass.cpp +++ b/src/Analyzer/Passes/SumIfToCountIfPass.cpp @@ -32,7 +32,7 @@ public: return; auto * function_node = node->as(); - if (!function_node || !function_node->isAggregateFunction() || function_node->getResultType()->isNullable()) + if (!function_node || !function_node->isAggregateFunction()) return; auto function_name = function_node->getFunctionName(); @@ -54,10 +54,10 @@ public: if (!constant_node) return; - const auto & constant_value_literal = constant_node->getValue(); - if (!isInt64OrUInt64FieldType(constant_value_literal.getType())) + if (auto constant_type = constant_node->getResultType(); !isUInt64(constant_type) && !isInt64(constant_type)) return; + const auto & constant_value_literal = constant_node->getValue(); if (getSettings().aggregate_functions_null_for_empty) return; From 060f79862d1c13f9fb723b592e384db009a45823 Mon Sep 17 00:00:00 2001 From: Antonio Andelic Date: Fri, 15 Mar 2024 10:49:36 +0000 Subject: [PATCH 4/4] Fix --- src/Analyzer/Passes/SumIfToCountIfPass.cpp | 2 +- ...3010_sum_to_to_count_if_nullable.reference | 66 +++++++++++++++++++ .../03010_sum_to_to_count_if_nullable.sql | 12 +++- 3 files changed, 77 insertions(+), 3 deletions(-) diff --git a/src/Analyzer/Passes/SumIfToCountIfPass.cpp b/src/Analyzer/Passes/SumIfToCountIfPass.cpp index 2c41b6dc467..1c2097e7be9 100644 --- a/src/Analyzer/Passes/SumIfToCountIfPass.cpp +++ b/src/Analyzer/Passes/SumIfToCountIfPass.cpp @@ -54,7 +54,7 @@ public: if (!constant_node) return; - if (auto constant_type = constant_node->getResultType(); !isUInt64(constant_type) && !isInt64(constant_type)) + if (auto constant_type = constant_node->getResultType(); !isNativeInteger(constant_type)) return; const auto & constant_value_literal = constant_node->getValue(); diff --git a/tests/queries/0_stateless/03010_sum_to_to_count_if_nullable.reference b/tests/queries/0_stateless/03010_sum_to_to_count_if_nullable.reference index 8627f639a03..89e5f639c66 100644 --- a/tests/queries/0_stateless/03010_sum_to_to_count_if_nullable.reference +++ b/tests/queries/0_stateless/03010_sum_to_to_count_if_nullable.reference @@ -1,2 +1,68 @@ (5,NULL) (5,NULL) +(5,NULL) +QUERY id: 0 + PROJECTION COLUMNS + (sumIf(toNullable(1), equals(modulo(number, 2), 0)), NULL) Tuple(Nullable(UInt64), Nullable(Nothing)) + PROJECTION + LIST id: 1, nodes: 1 + FUNCTION id: 2, function_name: tuple, function_type: ordinary, result_type: Tuple(Nullable(UInt64), Nullable(Nothing)) + ARGUMENTS + LIST id: 3, nodes: 2 + FUNCTION id: 4, function_name: sumIf, function_type: aggregate, result_type: Nullable(UInt64) + ARGUMENTS + LIST id: 5, nodes: 2 + CONSTANT id: 6, constant_value: UInt64_1, constant_value_type: Nullable(UInt8) + EXPRESSION + FUNCTION id: 7, function_name: toNullable, function_type: ordinary, result_type: Nullable(UInt8) + ARGUMENTS + LIST id: 8, nodes: 1 + CONSTANT id: 9, constant_value: UInt64_1, constant_value_type: UInt8 + FUNCTION id: 10, function_name: equals, function_type: ordinary, result_type: UInt8 + ARGUMENTS + LIST id: 11, nodes: 2 + FUNCTION id: 12, function_name: modulo, function_type: ordinary, result_type: UInt8 + ARGUMENTS + LIST id: 13, nodes: 2 + COLUMN id: 14, column_name: number, result_type: UInt64, source_id: 15 + CONSTANT id: 16, constant_value: UInt64_2, constant_value_type: UInt8 + CONSTANT id: 17, constant_value: UInt64_0, constant_value_type: UInt8 + CONSTANT id: 18, constant_value: NULL, constant_value_type: Nullable(Nothing) + JOIN TREE + TABLE_FUNCTION id: 15, alias: __table1, table_function_name: numbers + ARGUMENTS + LIST id: 19, nodes: 1 + CONSTANT id: 20, constant_value: UInt64_10, constant_value_type: UInt8 +(5,NULL) +QUERY id: 0 + PROJECTION COLUMNS + (sum(if(equals(modulo(number, 2), 0), toNullable(1), 0)), NULL) Tuple(Nullable(UInt64), Nullable(Nothing)) + PROJECTION + LIST id: 1, nodes: 1 + FUNCTION id: 2, function_name: tuple, function_type: ordinary, result_type: Tuple(Nullable(UInt64), Nullable(Nothing)) + ARGUMENTS + LIST id: 3, nodes: 2 + FUNCTION id: 4, function_name: sumOrNullIf, function_type: aggregate, result_type: Nullable(UInt64) + ARGUMENTS + LIST id: 5, nodes: 2 + CONSTANT id: 6, constant_value: UInt64_1, constant_value_type: Nullable(UInt8) + EXPRESSION + FUNCTION id: 7, function_name: toNullable, function_type: ordinary, result_type: Nullable(UInt8) + ARGUMENTS + LIST id: 8, nodes: 1 + CONSTANT id: 9, constant_value: UInt64_1, constant_value_type: UInt8 + FUNCTION id: 10, function_name: equals, function_type: ordinary, result_type: UInt8 + ARGUMENTS + LIST id: 11, nodes: 2 + FUNCTION id: 12, function_name: modulo, function_type: ordinary, result_type: UInt8 + ARGUMENTS + LIST id: 13, nodes: 2 + COLUMN id: 14, column_name: number, result_type: UInt64, source_id: 15 + CONSTANT id: 16, constant_value: UInt64_2, constant_value_type: UInt8 + CONSTANT id: 17, constant_value: UInt64_0, constant_value_type: UInt8 + CONSTANT id: 18, constant_value: NULL, constant_value_type: Nullable(Nothing) + JOIN TREE + TABLE_FUNCTION id: 15, alias: __table1, table_function_name: numbers + ARGUMENTS + LIST id: 19, nodes: 1 + CONSTANT id: 20, constant_value: UInt64_10, constant_value_type: UInt8 diff --git a/tests/queries/0_stateless/03010_sum_to_to_count_if_nullable.sql b/tests/queries/0_stateless/03010_sum_to_to_count_if_nullable.sql index 394cd4f1ea5..b283a69a020 100644 --- a/tests/queries/0_stateless/03010_sum_to_to_count_if_nullable.sql +++ b/tests/queries/0_stateless/03010_sum_to_to_count_if_nullable.sql @@ -1,3 +1,11 @@ SET optimize_rewrite_sum_if_to_count_if = 1; -SELECT (sumIf(toNullable(1), (number % 2) = 0), NULL) FROM numbers(10) SETTINGS allow_experimental_analyzer=0; -SELECT (sumIf(toNullable(1), (number % 2) = 0), NULL) FROM numbers(10) SETTINGS allow_experimental_analyzer=1; \ No newline at end of file + +SET allow_experimental_analyzer = 0; +SELECT (sumIf(toNullable(1), (number % 2) = 0), NULL) FROM numbers(10); +SELECT (sum(if((number % 2) = 0, toNullable(1), 0)), NULL) FROM numbers(10); + +SET allow_experimental_analyzer = 1; +SELECT (sumIf(toNullable(1), (number % 2) = 0), NULL) FROM numbers(10); +EXPLAIN QUERY TREE SELECT (sumIf(toNullable(1), (number % 2) = 0), NULL) FROM numbers(10); +SELECT (sum(if((number % 2) = 0, toNullable(1), 0)), NULL) FROM numbers(10); +EXPLAIN QUERY TREE SELECT (sum(if((number % 2) = 0, toNullable(1), 0)), NULL) FROM numbers(10); \ No newline at end of file