From c3b8978023fae8adaa98a111f6253be50ee72a35 Mon Sep 17 00:00:00 2001
From: Amos Bird <amosbird@gmail.com>
Date: Wed, 19 Jul 2023 11:53:03 +0800
Subject: [PATCH] Don't use minmax_count projections when counting nullable
 columns

---
 .../optimizeUseAggregateProjection.cpp        | 32 ++++---------------
 ..._count_projection_count_nullable.reference |  1 +
 ...minmax_count_projection_count_nullable.sql |  9 ++++++
 3 files changed, 17 insertions(+), 25 deletions(-)
 create mode 100644 tests/queries/0_stateless/01710_minmax_count_projection_count_nullable.reference
 create mode 100644 tests/queries/0_stateless/01710_minmax_count_projection_count_nullable.sql

diff --git a/src/Processors/QueryPlan/Optimizations/optimizeUseAggregateProjection.cpp b/src/Processors/QueryPlan/Optimizations/optimizeUseAggregateProjection.cpp
index f183bdca7a9..4f25118958f 100644
--- a/src/Processors/QueryPlan/Optimizations/optimizeUseAggregateProjection.cpp
+++ b/src/Processors/QueryPlan/Optimizations/optimizeUseAggregateProjection.cpp
@@ -92,18 +92,6 @@ static AggregateProjectionInfo getAggregatingProjectionInfo(
     return info;
 }
 
-static bool hasNullableOrMissingColumn(const DAGIndex & index, const Names & names)
-{
-    for (const auto & query_name : names)
-    {
-        auto jt = index.find(query_name);
-        if (jt == index.end() || jt->second->result_type->isNullable())
-            return true;
-    }
-
-    return false;
-}
-
 struct AggregateFunctionMatch
 {
     const AggregateDescription * description = nullptr;
@@ -170,20 +158,14 @@ std::optional<AggregateFunctionMatches> matchAggregateFunctions(
             }
 
             /// This is a special case for the function count().
-            /// We can assume that 'count(expr) == count()' if expr is not nullable.
-            if (typeid_cast<const AggregateFunctionCount *>(candidate.function.get()))
+            /// We can assume that 'count(expr) == count()' if expr is not nullable,
+            /// which can be verified by simply casting to `AggregateFunctionCount *`.
+            if (typeid_cast<const AggregateFunctionCount *>(aggregate.function.get()))
             {
-                bool has_nullable_or_missing_arg = false;
-                has_nullable_or_missing_arg |= hasNullableOrMissingColumn(query_index, aggregate.argument_names);
-                has_nullable_or_missing_arg |= hasNullableOrMissingColumn(proj_index, candidate.argument_names);
-
-                if (!has_nullable_or_missing_arg)
-                {
-                    /// we can ignore arguments for count()
-                    found_match = true;
-                    res.push_back({&candidate, DataTypes()});
-                    break;
-                }
+                /// we can ignore arguments for count()
+                found_match = true;
+                res.push_back({&candidate, DataTypes()});
+                break;
             }
 
             /// Now, function names and types matched.
diff --git a/tests/queries/0_stateless/01710_minmax_count_projection_count_nullable.reference b/tests/queries/0_stateless/01710_minmax_count_projection_count_nullable.reference
new file mode 100644
index 00000000000..d00491fd7e5
--- /dev/null
+++ b/tests/queries/0_stateless/01710_minmax_count_projection_count_nullable.reference
@@ -0,0 +1 @@
+1
diff --git a/tests/queries/0_stateless/01710_minmax_count_projection_count_nullable.sql b/tests/queries/0_stateless/01710_minmax_count_projection_count_nullable.sql
new file mode 100644
index 00000000000..048d725e0a0
--- /dev/null
+++ b/tests/queries/0_stateless/01710_minmax_count_projection_count_nullable.sql
@@ -0,0 +1,9 @@
+DROP TABLE IF EXISTS test;
+
+CREATE TABLE test (`val` LowCardinality(Nullable(String))) ENGINE = MergeTree ORDER BY tuple() SETTINGS index_granularity = 8192;
+
+insert into test select number == 3 ? 'some value' : null from numbers(5);
+
+SELECT count(val) FROM test SETTINGS optimize_use_implicit_projections = 1;
+
+DROP TABLE test;