Merge pull request ClickHouse#88690 from ClickHouse/backport/25.8/87958

Backport ClickHouse#87958 to 25.8: Don't remove injective functions from GROUP BY if arguments types are not allowed in GROUP BY

Author: Avogar

src/Analyzer/Passes/OptimizeGroupByInjectiveFunctionsPass.cpp

@@ -13,6 +13,7 @@ namespace Setting
 {
     extern const SettingsBool group_by_use_nulls;
     extern const SettingsBool optimize_injective_functions_in_group_by;
+    extern const SettingsBool allow_suspicious_types_in_group_by;
 }
 
 namespace
@@ -88,7 +89,8 @@ class OptimizeGroupByInjectiveFunctionsVisitor : public InDepthQueryTreeVisitorW
 
                 // Aggregate functions are not allowed in GROUP BY clause
                 auto function = function_node->getFunctionOrThrow();
-                bool can_be_eliminated = function->isInjective(function_node->getArgumentColumns());
+                auto arguments = function_node->getArgumentColumns();
+                bool can_be_eliminated = function->isInjective(arguments) && isValidGroupByKeyTypes(arguments);
 
                 if (can_be_eliminated)
                 {
@@ -106,6 +108,29 @@ class OptimizeGroupByInjectiveFunctionsVisitor : public InDepthQueryTreeVisitorW
 
         grouping_set = std::move(new_group_by_keys);
     }
+
+    bool isValidGroupByKeyTypes(const ColumnsWithTypeAndName & columns) const
+    {
+        if (getContext()->getSettingsRef()[Setting::allow_suspicious_types_in_group_by])
+            return true;
+
+        bool is_valid = true;
+        auto check = [&](const IDataType & type)
+        {
+            /// Dynamic and Variant types are not allowed in GROUP BY by default.
+            is_valid &= !isDynamic(type) && !isVariant(type);
+        };
+
+        for (const auto & column : columns)
+        {
+            check(*column.type);
+            column.type->forEachChild(check);
+            if (!is_valid)
+                break;
+        }
+
+        return is_valid;
+    }
 };
 
 }

src/Interpreters/TreeOptimizer.cpp

@@ -60,6 +60,7 @@ namespace Setting
     extern const SettingsBool optimize_redundant_functions_in_order_by;
     extern const SettingsBool optimize_rewrite_array_exists_to_has;
     extern const SettingsBool optimize_or_like_chain;
+    extern const SettingsBool optimize_injective_functions_in_group_by;
 }
 
 namespace ErrorCodes
@@ -132,6 +133,12 @@ void optimizeGroupBy(ASTSelectQuery * select_query, ContextPtr context)
     {
         if (const auto * function = group_exprs[i]->as<ASTFunction>())
         {
+            if (!settings[Setting::optimize_injective_functions_in_group_by])
+            {
+                ++i;
+                continue;
+            }
+
             /// assert function is injective
             if (possibly_injective_function_names.contains(function->name))
             {

tests/queries/0_stateless/03641_group_by_injective_functoon_bad_arguments.reference

@@ -0,0 +1,23 @@
+1	str
+1	str
+1	str
+QUERY id: 0
+  PROJECTION COLUMNS
+    count() UInt64
+    toString(json.a) String
+  PROJECTION
+    LIST id: 1, nodes: 2
+      FUNCTION id: 2, function_name: count, function_type: aggregate, result_type: UInt64
+      FUNCTION id: 3, function_name: toString, function_type: ordinary, result_type: String
+        ARGUMENTS
+          LIST id: 4, nodes: 1
+            COLUMN id: 5, column_name: json.a, result_type: Dynamic, source_id: 6
+  JOIN TREE
+    TABLE id: 6, alias: __table1, table_name: default.test
+  GROUP BY
+    LIST id: 7, nodes: 1
+      FUNCTION id: 8, function_name: toString, function_type: ordinary, result_type: String
+        ARGUMENTS
+          LIST id: 9, nodes: 1
+            COLUMN id: 10, column_name: json.a, result_type: Dynamic, source_id: 6
+  SETTINGS enable_analyzer=1 optimize_injective_functions_in_group_by=1

tests/queries/0_stateless/03641_group_by_injective_functoon_bad_arguments.sql

@@ -0,0 +1,8 @@
+create table test (json JSON) engine=MergeTree order by tuple();
+insert into test select '{"a" : "str"}';
+select count(), toString(json.a) from test group by toString(json.a) settings enable_analyzer=0, optimize_injective_functions_in_group_by=0;
+select count(), toString(json.a) from test group by toString(json.a) settings enable_analyzer=1, optimize_injective_functions_in_group_by=0;
+select count(), toString(json.a) from test group by toString(json.a) settings enable_analyzer=1, optimize_injective_functions_in_group_by=1;
+explain query tree select count(), toString(json.a) from test group by toString(json.a) settings enable_analyzer=1, optimize_injective_functions_in_group_by=1;
+drop table test;
+