Skip to content

Commit a8f5b10

Browse files
alexey-milovidovalexey-milovidov
authored
Merge pull request ClickHouse#10621 from azat/enable_optimize_predicate_expression-order
Fix predicates optimization for distributed queries with HAVING
2 parents 9a362eb + 6a9ec13 commit a8f5b10

5 files changed

Lines changed: 31 additions & 22 deletions

File tree

‎src/Interpreters/Aggregator.cpp‎

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -128,11 +128,11 @@ Block Aggregator::getHeader(bool final) const
128128

129129
if (final)
130130
{
131-
for (size_t i = 0; i < params.aggregates_size; ++i)
131+
for (const auto & aggregate : params.aggregates)
132132
{
133-
auto & elem = res.getByPosition(params.keys_size + i);
133+
auto & elem = res.getByName(aggregate.column_name);
134134

135-
elem.type = params.aggregates[i].function->getReturnType();
135+
elem.type = aggregate.function->getReturnType();
136136
elem.column = elem.type->createColumn();
137137
}
138138
}
@@ -1060,7 +1060,8 @@ Block Aggregator::prepareBlockAndFill(
10601060
{
10611061
if (!final)
10621062
{
1063-
aggregate_columns[i] = header.safeGetByPosition(i + params.keys_size).type->createColumn();
1063+
const auto & aggregate_column_name = params.aggregates[i].column_name;
1064+
aggregate_columns[i] = header.getByName(aggregate_column_name).type->createColumn();
10641065

10651066
/// The ColumnAggregateFunction column captures the shared ownership of the arena with the aggregate function states.
10661067
ColumnAggregateFunction & column_aggregate_func = assert_cast<ColumnAggregateFunction &>(*aggregate_columns[i]);
@@ -1096,10 +1097,11 @@ Block Aggregator::prepareBlockAndFill(
10961097

10971098
for (size_t i = 0; i < params.aggregates_size; ++i)
10981099
{
1100+
const auto & aggregate_column_name = params.aggregates[i].column_name;
10991101
if (final)
1100-
res.getByPosition(i + params.keys_size).column = std::move(final_aggregate_columns[i]);
1102+
res.getByName(aggregate_column_name).column = std::move(final_aggregate_columns[i]);
11011103
else
1102-
res.getByPosition(i + params.keys_size).column = std::move(aggregate_columns[i]);
1104+
res.getByName(aggregate_column_name).column = std::move(aggregate_columns[i]);
11031105
}
11041106

11051107
/// Change the size of the columns-constants in the block.
@@ -1824,7 +1826,10 @@ void NO_INLINE Aggregator::mergeStreamsImplCase(
18241826
key_columns[i] = block.safeGetByPosition(i).column.get();
18251827

18261828
for (size_t i = 0; i < params.aggregates_size; ++i)
1827-
aggregate_columns[i] = &typeid_cast<const ColumnAggregateFunction &>(*block.safeGetByPosition(params.keys_size + i).column).getData();
1829+
{
1830+
const auto & aggregate_column_name = params.aggregates[i].column_name;
1831+
aggregate_columns[i] = &typeid_cast<const ColumnAggregateFunction &>(*block.getByName(aggregate_column_name).column).getData();
1832+
}
18281833

18291834
typename Method::State state(key_columns, key_sizes, aggregation_state_cache);
18301835

@@ -1900,7 +1905,10 @@ void NO_INLINE Aggregator::mergeWithoutKeyStreamsImpl(
19001905

19011906
/// Remember the columns we will work with
19021907
for (size_t i = 0; i < params.aggregates_size; ++i)
1903-
aggregate_columns[i] = &typeid_cast<const ColumnAggregateFunction &>(*block.safeGetByPosition(params.keys_size + i).column).getData();
1908+
{
1909+
const auto & aggregate_column_name = params.aggregates[i].column_name;
1910+
aggregate_columns[i] = &typeid_cast<const ColumnAggregateFunction &>(*block.getByName(aggregate_column_name).column).getData();
1911+
}
19041912

19051913
AggregatedDataWithoutKey & res = result.without_key;
19061914
if (!res)

‎src/Interpreters/ExpressionActions.cpp‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -342,11 +342,7 @@ void ExpressionAction::execute(Block & block, bool dry_run, ExtraBlockPtr & not_
342342
{
343343
ColumnNumbers arguments(argument_names.size());
344344
for (size_t i = 0; i < argument_names.size(); ++i)
345-
{
346-
if (!block.has(argument_names[i]))
347-
throw Exception("Not found column: '" + argument_names[i] + "'", ErrorCodes::NOT_FOUND_COLUMN_IN_BLOCK);
348345
arguments[i] = block.getPositionByName(argument_names[i]);
349-
}
350346

351347
size_t num_columns_without_result = block.columns();
352348
block.insert({ nullptr, result_type, result_name});

‎src/Interpreters/PredicateExpressionsOptimizer.cpp‎

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -49,13 +49,6 @@ static ASTs splitConjunctionPredicate(const std::initializer_list<const ASTPtr>
4949
{
5050
std::vector<ASTPtr> res;
5151

52-
auto remove_expression_at_index = [&res] (const size_t index)
53-
{
54-
if (index < res.size() - 1)
55-
std::swap(res[index], res.back());
56-
res.pop_back();
57-
};
58-
5952
for (const auto & predicate : predicates)
6053
{
6154
if (!predicate)
@@ -65,14 +58,15 @@ static ASTs splitConjunctionPredicate(const std::initializer_list<const ASTPtr>
6558

6659
for (size_t idx = 0; idx < res.size();)
6760
{
68-
const auto & expression = res.at(idx);
61+
ASTPtr expression = res.at(idx);
6962

7063
if (const auto * function = expression->as<ASTFunction>(); function && function->name == "and")
7164
{
65+
res.erase(res.begin() + idx);
66+
7267
for (auto & child : function->arguments->children)
7368
res.emplace_back(child);
7469

75-
remove_expression_at_index(idx);
7670
continue;
7771
}
7872
++idx;

‎tests/queries/0_stateless/01056_predicate_optimizer_bugs.reference‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ a 2 1 0
44
a 3 1 0
55
b 13 2 0
66
b 15 2 0
7-
SELECT \n co, \n co2, \n co3, \n num\nFROM \n(\n SELECT \n co, \n co2, \n co3, \n count() AS num\n FROM \n (\n SELECT \n 1 AS co, \n 2 AS co2, \n 3 AS co3\n )\n GROUP BY \n co, \n co2, \n co3\n WITH CUBE\n HAVING (co != 0) AND (co2 != 2)\n)\nWHERE (co != 0) AND (co2 != 2)
7+
SELECT \n co, \n co2, \n co3, \n num\nFROM \n(\n SELECT \n co, \n co2, \n co3, \n count() AS num\n FROM \n (\n SELECT \n 1 AS co, \n 2 AS co2, \n 3 AS co3\n )\n GROUP BY \n co, \n co2, \n co3\n WITH CUBE\n HAVING (co2 != 2) AND (co != 0)\n)\nWHERE (co != 0) AND (co2 != 2)
88
1 0 3 1
99
1 0 0 1
1010
SELECT alias AS name\nFROM \n(\n SELECT name AS alias\n FROM system.settings\n WHERE alias = \'enable_optimize_predicate_expression\'\n)\nANY INNER JOIN \n(\n SELECT name\n FROM system.settings\n) USING (name)\nWHERE name = \'enable_optimize_predicate_expression\'
@@ -26,3 +26,4 @@ SELECT dummy\nFROM \n(\n SELECT dummy\n FROM system.one\n WHERE arrayMa
2626
0
2727
SELECT \n id, \n value, \n value_1\nFROM \n(\n SELECT \n 1 AS id, \n 2 AS value\n)\nALL INNER JOIN \n(\n SELECT \n 1 AS id, \n 3 AS value_1\n) USING (id)\nWHERE arrayMap(x -> ((x + value) + value_1), [1]) = [6]
2828
1 2 3
29+
SELECT dummy\nFROM system.one\nWHERE (dummy > 0) AND (dummy < 0)

‎tests/queries/0_stateless/01056_predicate_optimizer_bugs.sql‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,3 +74,13 @@ SELECT * FROM (SELECT * FROM system.one) WHERE arrayMap(x -> x + 1, [dummy]) = [
7474

7575
ANALYZE SELECT * FROM (SELECT 1 AS id, 2 AS value) INNER JOIN (SELECT 1 AS id, 3 AS value_1) USING id WHERE arrayMap(x -> x + value + value_1, [1]) = [6];
7676
SELECT * FROM (SELECT 1 AS id, 2 AS value) INNER JOIN (SELECT 1 AS id, 3 AS value_1) USING id WHERE arrayMap(x -> x + value + value_1, [1]) = [6];
77+
78+
-- check order is preserved
79+
ANALYZE SELECT * FROM system.one HAVING dummy > 0 AND dummy < 0;
80+
81+
-- from #10613
82+
SELECT name, count() AS cnt
83+
FROM remote('127.{1,2}', system.settings)
84+
GROUP BY name
85+
HAVING (max(value) > '9') AND (min(changed) = 0)
86+
FORMAT Null;

0 commit comments

Comments
 (0)