Skip to content

Commit

Permalink
Do not rewrite sum(Nullable()) to count()
Browse files Browse the repository at this point in the history
v2: fix for LowCardinality
Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>
  • Loading branch information
azat committed Feb 15, 2024
1 parent 7c01705 commit d2d58bf
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 0 deletions.
6 changes: 6 additions & 0 deletions src/Analyzer/Passes/NormalizeCountVariantsPass.cpp
Expand Up @@ -7,6 +7,7 @@
#include <Analyzer/ConstantNode.h>
#include <Analyzer/FunctionNode.h>
#include <Interpreters/Context.h>
#include <DataTypes/DataTypesNumber.h>

namespace DB
{
Expand All @@ -32,6 +33,11 @@ class NormalizeCountVariantsVisitor : public InDepthQueryTreeVisitorWithContext<
if (function_node->getArguments().getNodes().size() != 1)
return;

/// forbid the optimization if return value of sum() and count() differs:
/// count() returns only UInt64 type, while sum() could return Nullable().
if (!function_node->getResultType()->equals(DataTypeUInt64()))
return;

auto & first_argument = function_node->getArguments().getNodes()[0];
auto * first_argument_constant_node = first_argument->as<ConstantNode>();
if (!first_argument_constant_node)
Expand Down
@@ -0,0 +1,4 @@
Nullable(UInt64)
UInt64
Nullable(UInt64)
UInt64
7 changes: 7 additions & 0 deletions tests/queries/0_stateless/02991_count_rewrite_analyzer.sql
@@ -0,0 +1,7 @@
-- Regression test for https://github.com/ClickHouse/ClickHouse/issues/59919
SET allow_experimental_analyzer=1;

SELECT toTypeName(sum(toNullable('a') IN toNullable('a'))) AS x;
SELECT toTypeName(count(toNullable('a') IN toNullable('a'))) AS x;
SELECT toTypeName(sum(toFixedString('a', toLowCardinality(toNullable(1))) IN toFixedString('a', 1))) AS x;
SELECT toTypeName(count(toFixedString('a', toLowCardinality(toNullable(1))) IN toFixedString('a', 1))) AS x;

0 comments on commit d2d58bf

Please sign in to comment.