Skip to content

Commit 5200b9c

Browse files
Backport ClickHouse#87958 to 25.8: Don't remove injective functions from GROUP BY if arguments types are not allowed in GROUP BY
1 parent 17fafc3 commit 5200b9c

File tree

4 files changed

+64
-1
lines changed

4 files changed

+64
-1
lines changed

src/Analyzer/Passes/OptimizeGroupByInjectiveFunctionsPass.cpp

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ namespace Setting
1313
{
1414
extern const SettingsBool group_by_use_nulls;
1515
extern const SettingsBool optimize_injective_functions_in_group_by;
16+
extern const SettingsBool allow_suspicious_types_in_group_by;
1617
}
1718

1819
namespace
@@ -88,7 +89,8 @@ class OptimizeGroupByInjectiveFunctionsVisitor : public InDepthQueryTreeVisitorW
8889

8990
// Aggregate functions are not allowed in GROUP BY clause
9091
auto function = function_node->getFunctionOrThrow();
91-
bool can_be_eliminated = function->isInjective(function_node->getArgumentColumns());
92+
auto arguments = function_node->getArgumentColumns();
93+
bool can_be_eliminated = function->isInjective(arguments) && isValidGroupByKeyTypes(arguments);
9294

9395
if (can_be_eliminated)
9496
{
@@ -106,6 +108,29 @@ class OptimizeGroupByInjectiveFunctionsVisitor : public InDepthQueryTreeVisitorW
106108

107109
grouping_set = std::move(new_group_by_keys);
108110
}
111+
112+
bool isValidGroupByKeyTypes(const ColumnsWithTypeAndName & columns) const
113+
{
114+
if (getContext()->getSettingsRef()[Setting::allow_suspicious_types_in_group_by])
115+
return true;
116+
117+
bool is_valid = true;
118+
auto check = [&](const IDataType & type)
119+
{
120+
/// Dynamic and Variant types are not allowed in GROUP BY by default.
121+
is_valid &= !isDynamic(type) && !isVariant(type);
122+
};
123+
124+
for (const auto & column : columns)
125+
{
126+
check(*column.type);
127+
column.type->forEachChild(check);
128+
if (!is_valid)
129+
break;
130+
}
131+
132+
return is_valid;
133+
}
109134
};
110135

111136
}

src/Interpreters/TreeOptimizer.cpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ namespace Setting
6060
extern const SettingsBool optimize_redundant_functions_in_order_by;
6161
extern const SettingsBool optimize_rewrite_array_exists_to_has;
6262
extern const SettingsBool optimize_or_like_chain;
63+
extern const SettingsBool optimize_injective_functions_in_group_by;
6364
}
6465

6566
namespace ErrorCodes
@@ -132,6 +133,12 @@ void optimizeGroupBy(ASTSelectQuery * select_query, ContextPtr context)
132133
{
133134
if (const auto * function = group_exprs[i]->as<ASTFunction>())
134135
{
136+
if (!settings[Setting::optimize_injective_functions_in_group_by])
137+
{
138+
++i;
139+
continue;
140+
}
141+
135142
/// assert function is injective
136143
if (possibly_injective_function_names.contains(function->name))
137144
{
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
1 str
2+
1 str
3+
1 str
4+
QUERY id: 0
5+
PROJECTION COLUMNS
6+
count() UInt64
7+
toString(json.a) String
8+
PROJECTION
9+
LIST id: 1, nodes: 2
10+
FUNCTION id: 2, function_name: count, function_type: aggregate, result_type: UInt64
11+
FUNCTION id: 3, function_name: toString, function_type: ordinary, result_type: String
12+
ARGUMENTS
13+
LIST id: 4, nodes: 1
14+
COLUMN id: 5, column_name: json.a, result_type: Dynamic, source_id: 6
15+
JOIN TREE
16+
TABLE id: 6, alias: __table1, table_name: default.test
17+
GROUP BY
18+
LIST id: 7, nodes: 1
19+
FUNCTION id: 8, function_name: toString, function_type: ordinary, result_type: String
20+
ARGUMENTS
21+
LIST id: 9, nodes: 1
22+
COLUMN id: 10, column_name: json.a, result_type: Dynamic, source_id: 6
23+
SETTINGS enable_analyzer=1 optimize_injective_functions_in_group_by=1
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
create table test (json JSON) engine=MergeTree order by tuple();
2+
insert into test select '{"a" : "str"}';
3+
select count(), toString(json.a) from test group by toString(json.a) settings enable_analyzer=0, optimize_injective_functions_in_group_by=0;
4+
select count(), toString(json.a) from test group by toString(json.a) settings enable_analyzer=1, optimize_injective_functions_in_group_by=0;
5+
select count(), toString(json.a) from test group by toString(json.a) settings enable_analyzer=1, optimize_injective_functions_in_group_by=1;
6+
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;
7+
drop table test;
8+

0 commit comments

Comments
 (0)