Skip to content

Commit c3effd7

Browse files
mkmkmemkmkme
committed
Fix condition not being moved to PREWHERE in case there is a row policy
This commit is a manual backport of two PRs: 1. ClickHouse#87303 2. ClickHouse#88017 Additionally, the test from ClickHouse#88036 was added.
1 parent 73b4f3b commit c3effd7

37 files changed

+582
-379
lines changed

‎src/Interpreters/ExpressionAnalyzer.cpp‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1846,7 +1846,7 @@ ExpressionAnalysisResult::ExpressionAnalysisResult(
18461846
bool first_stage_,
18471847
bool second_stage_,
18481848
bool only_types,
1849-
const FilterDAGInfoPtr & filter_info_,
1849+
const FilterDAGInfoPtr & row_policy_info_,
18501850
const FilterDAGInfoPtr & additional_filter,
18511851
const Block & source_header)
18521852
: first_stage(first_stage_)
@@ -1944,10 +1944,10 @@ ExpressionAnalysisResult::ExpressionAnalysisResult(
19441944
columns_for_additional_filter.begin(), columns_for_additional_filter.end());
19451945
}
19461946

1947-
if (storage && filter_info_)
1947+
if (storage && row_policy_info_)
19481948
{
1949-
filter_info = filter_info_;
1950-
filter_info->do_remove_column = true;
1949+
row_policy_info = row_policy_info_;
1950+
row_policy_info->do_remove_column = true;
19511951
}
19521952

19531953
if (prewhere_dag_and_flags = query_analyzer.appendPrewhere(chain, !first_stage); prewhere_dag_and_flags)
@@ -2294,9 +2294,9 @@ std::string ExpressionAnalysisResult::dump() const
22942294
ss << "prewhere_info " << prewhere_info->dump() << "\n";
22952295
}
22962296

2297-
if (filter_info)
2297+
if (row_policy_info)
22982298
{
2299-
ss << "filter_info " << filter_info->dump() << "\n";
2299+
ss << "filter_info " << row_policy_info->dump() << "\n";
23002300
}
23012301

23022302
if (before_aggregation)

‎src/Interpreters/ExpressionAnalyzer.h‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -259,7 +259,7 @@ struct ExpressionAnalysisResult
259259
NameSet columns_to_remove_after_prewhere;
260260

261261
PrewhereInfoPtr prewhere_info;
262-
FilterDAGInfoPtr filter_info;
262+
FilterDAGInfoPtr row_policy_info;
263263
ConstantFilterDescription prewhere_constant_filter_description;
264264
ConstantFilterDescription where_constant_filter_description;
265265
/// Actions by every element of ORDER BY
@@ -274,12 +274,12 @@ struct ExpressionAnalysisResult
274274
bool first_stage,
275275
bool second_stage,
276276
bool only_types,
277-
const FilterDAGInfoPtr & filter_info,
277+
const FilterDAGInfoPtr & row_policy_info,
278278
const FilterDAGInfoPtr & additional_filter, /// for setting additional_filters
279279
const Block & source_header);
280280

281281
/// Filter for row-level security.
282-
bool hasFilter() const { return filter_info.get(); }
282+
bool hasRowPolicyFilter() const { return row_policy_info.get(); }
283283

284284
bool hasJoin() const { return join.get(); }
285285
bool hasPrewhere() const { return prewhere_info.get(); }

‎src/Interpreters/InterpreterSelectQuery.cpp‎

Lines changed: 46 additions & 88 deletions
Original file line numberDiff line numberDiff line change
@@ -777,7 +777,7 @@ InterpreterSelectQuery::InterpreterSelectQuery(
777777
/// Fix source_header for filter actions.
778778
if (row_policy_filter && !row_policy_filter->empty())
779779
{
780-
filter_info = generateFilterActions(
780+
row_policy_info = generateFilterActions(
781781
table_id, row_policy_filter->expression, context, storage, storage_snapshot, metadata_snapshot, required_columns,
782782
prepared_sets);
783783

@@ -944,8 +944,6 @@ bool InterpreterSelectQuery::adjustParallelReplicasAfterAnalysis()
944944
max_rows = max_rows ? std::min(max_rows, settings.max_rows_to_read.value) : settings.max_rows_to_read;
945945
query_info_copy.trivial_limit = max_rows;
946946

947-
/// Apply filters to prewhere and add them to the query_info so we can filter out parts efficiently during row estimation
948-
applyFiltersToPrewhereInAnalysis(analysis_copy);
949947
if (analysis_copy.prewhere_info)
950948
{
951949
query_info_copy.prewhere_info = analysis_copy.prewhere_info;
@@ -956,18 +954,16 @@ bool InterpreterSelectQuery::adjustParallelReplicasAfterAnalysis()
956954
ActionDAGNodes added_filter_nodes = MergeTreeData::getFiltersForPrimaryKeyAnalysis(*this);
957955
if (query_info_copy.prewhere_info)
958956
{
959-
{
960-
const auto & node
961-
= query_info_copy.prewhere_info->prewhere_actions.findInOutputs(query_info_copy.prewhere_info->prewhere_column_name);
962-
added_filter_nodes.nodes.push_back(&node);
963-
}
957+
const auto & node
958+
= query_info_copy.prewhere_info->prewhere_actions.findInOutputs(query_info_copy.prewhere_info->prewhere_column_name);
959+
added_filter_nodes.nodes.push_back(&node);
960+
}
964961

965-
if (query_info_copy.prewhere_info->row_level_filter)
966-
{
967-
const auto & node
968-
= query_info_copy.prewhere_info->row_level_filter->findInOutputs(query_info_copy.prewhere_info->row_level_column_name);
969-
added_filter_nodes.nodes.push_back(&node);
970-
}
962+
if (query_info_copy.row_level_filter)
963+
{
964+
const auto & node
965+
= query_info_copy.row_level_filter->actions.findInOutputs(query_info_copy.row_level_filter->column_name);
966+
added_filter_nodes.nodes.push_back(&node);
971967
}
972968

973969
if (auto filter_actions_dag = ActionsDAG::buildFilterActionsDAG(added_filter_nodes.nodes))
@@ -1071,7 +1067,7 @@ Block InterpreterSelectQuery::getSampleBlockImpl()
10711067
&& options.to_stage > QueryProcessingStage::WithMergeableState;
10721068

10731069
analysis_result = ExpressionAnalysisResult(
1074-
*query_analyzer, metadata_snapshot, first_stage, second_stage, options.only_analyze, filter_info, additional_filter_info, source_header);
1070+
*query_analyzer, metadata_snapshot, first_stage, second_stage, options.only_analyze, row_policy_info, additional_filter_info, source_header);
10751071

10761072
if (options.to_stage == QueryProcessingStage::Enum::FetchColumns)
10771073
{
@@ -1514,32 +1510,20 @@ void InterpreterSelectQuery::executeImpl(QueryPlan & query_plan, std::optional<P
15141510
auto read_nothing = std::make_unique<ReadNothingStep>(source_header);
15151511
query_plan.addStep(std::move(read_nothing));
15161512

1517-
if (expressions.filter_info)
1513+
if (expressions.row_policy_info)
15181514
{
15191515
auto row_level_security_step = std::make_unique<FilterStep>(
15201516
query_plan.getCurrentDataStream(),
1521-
expressions.filter_info->actions.clone(),
1522-
expressions.filter_info->column_name,
1523-
expressions.filter_info->do_remove_column);
1517+
expressions.row_policy_info->actions.clone(),
1518+
expressions.row_policy_info->column_name,
1519+
expressions.row_policy_info->do_remove_column);
15241520

15251521
row_level_security_step->setStepDescription("Row-level security filter");
15261522
query_plan.addStep(std::move(row_level_security_step));
15271523
}
15281524

15291525
if (expressions.prewhere_info)
15301526
{
1531-
if (expressions.prewhere_info->row_level_filter)
1532-
{
1533-
auto row_level_filter_step = std::make_unique<FilterStep>(
1534-
query_plan.getCurrentDataStream(),
1535-
expressions.prewhere_info->row_level_filter->clone(),
1536-
expressions.prewhere_info->row_level_column_name,
1537-
true);
1538-
1539-
row_level_filter_step->setStepDescription("Row-level security filter (PREWHERE)");
1540-
query_plan.addStep(std::move(row_level_filter_step));
1541-
}
1542-
15431527
auto prewhere_step = std::make_unique<FilterStep>(
15441528
query_plan.getCurrentDataStream(),
15451529
expressions.prewhere_info->prewhere_actions.clone(),
@@ -1640,13 +1624,13 @@ void InterpreterSelectQuery::executeImpl(QueryPlan & query_plan, std::optional<P
16401624
{
16411625
// If there is a storage that supports prewhere, this will always be nullptr
16421626
// Thus, we don't actually need to check if projection is active.
1643-
if (expressions.filter_info)
1627+
if (expressions.row_policy_info && !(!input_pipe && storage && storage->supportsPrewhere()))
16441628
{
16451629
auto row_level_security_step = std::make_unique<FilterStep>(
16461630
query_plan.getCurrentDataStream(),
1647-
expressions.filter_info->actions.clone(),
1648-
expressions.filter_info->column_name,
1649-
expressions.filter_info->do_remove_column);
1631+
expressions.row_policy_info->actions.clone(),
1632+
expressions.row_policy_info->column_name,
1633+
expressions.row_policy_info->do_remove_column);
16501634

16511635
row_level_security_step->setStepDescription("Row-level security filter");
16521636
query_plan.addStep(std::move(row_level_security_step));
@@ -2073,21 +2057,21 @@ void InterpreterSelectQuery::addEmptySourceToQueryPlan(QueryPlan & query_plan, c
20732057
{
20742058
Pipe pipe(std::make_shared<NullSource>(source_header));
20752059

2076-
if (query_info.prewhere_info)
2060+
if (query_info.row_level_filter)
20772061
{
2078-
auto & prewhere_info = *query_info.prewhere_info;
2079-
2080-
if (prewhere_info.row_level_filter)
2062+
auto row_level_actions = std::make_shared<ExpressionActions>(query_info.row_level_filter->actions.clone());
2063+
pipe.addSimpleTransform([&](const Block & header)
20812064
{
2082-
auto row_level_actions = std::make_shared<ExpressionActions>(prewhere_info.row_level_filter->clone());
2083-
pipe.addSimpleTransform([&](const Block & header)
2084-
{
2085-
return std::make_shared<FilterTransform>(header,
2086-
row_level_actions,
2087-
prewhere_info.row_level_column_name, true);
2088-
});
2089-
}
2065+
return std::make_shared<FilterTransform>(header,
2066+
row_level_actions,
2067+
query_info.row_level_filter->column_name,
2068+
query_info.row_level_filter->do_remove_column);
2069+
});
2070+
}
20902071

2072+
if (query_info.prewhere_info)
2073+
{
2074+
auto & prewhere_info = *query_info.prewhere_info;
20912075
auto filter_actions = std::make_shared<ExpressionActions>(prewhere_info.prewhere_actions.clone());
20922076
pipe.addSimpleTransform([&](const Block & header)
20932077
{
@@ -2123,38 +2107,9 @@ bool InterpreterSelectQuery::shouldMoveToPrewhere() const
21232107
return settings.optimize_move_to_prewhere && (!query.final() || settings.optimize_move_to_prewhere_if_final);
21242108
}
21252109

2126-
/// Note that this is const and accepts the analysis ref to be able to use it to do analysis for parallel replicas
2127-
/// without affecting the final analysis multiple times
2128-
void InterpreterSelectQuery::applyFiltersToPrewhereInAnalysis(ExpressionAnalysisResult & analysis) const
2129-
{
2130-
if (!analysis.filter_info)
2131-
return;
2132-
2133-
if (!analysis.prewhere_info)
2134-
{
2135-
const bool does_storage_support_prewhere = !input_pipe && storage && storage->supportsPrewhere();
2136-
if (does_storage_support_prewhere && shouldMoveToPrewhere())
2137-
{
2138-
/// Execute row level filter in prewhere as a part of "move to prewhere" optimization.
2139-
analysis.prewhere_info = std::make_shared<PrewhereInfo>(std::move(analysis.filter_info->actions), analysis.filter_info->column_name);
2140-
analysis.prewhere_info->remove_prewhere_column = std::move(analysis.filter_info->do_remove_column);
2141-
analysis.prewhere_info->need_filter = true;
2142-
analysis.filter_info = nullptr;
2143-
}
2144-
}
2145-
else
2146-
{
2147-
/// Add row level security actions to prewhere.
2148-
analysis.prewhere_info->row_level_filter = std::move(analysis.filter_info->actions);
2149-
analysis.prewhere_info->row_level_column_name = std::move(analysis.filter_info->column_name);
2150-
analysis.filter_info = nullptr;
2151-
}
2152-
}
2153-
2154-
21552110
void InterpreterSelectQuery::addPrewhereAliasActions()
21562111
{
2157-
applyFiltersToPrewhereInAnalysis(analysis_result);
2112+
auto & row_level_filter = analysis_result.row_policy_info;
21582113
auto & prewhere_info = analysis_result.prewhere_info;
21592114
auto & columns_to_remove_after_prewhere = analysis_result.columns_to_remove_after_prewhere;
21602115

@@ -2181,12 +2136,12 @@ void InterpreterSelectQuery::addPrewhereAliasActions()
21812136
/// Get some columns directly from PREWHERE expression actions
21822137
auto prewhere_required_columns = prewhere_info->prewhere_actions.getRequiredColumns().getNames();
21832138
columns.insert(prewhere_required_columns.begin(), prewhere_required_columns.end());
2139+
}
21842140

2185-
if (prewhere_info->row_level_filter)
2186-
{
2187-
auto row_level_required_columns = prewhere_info->row_level_filter->getRequiredColumns().getNames();
2188-
columns.insert(row_level_required_columns.begin(), row_level_required_columns.end());
2189-
}
2141+
if (row_level_filter)
2142+
{
2143+
auto row_level_required_columns = row_level_filter->actions.getRequiredColumns().getNames();
2144+
columns.insert(row_level_required_columns.begin(), row_level_required_columns.end());
21902145
}
21912146

21922147
return columns;
@@ -2345,13 +2300,15 @@ std::optional<UInt64> InterpreterSelectQuery::getTrivialCount(UInt64 allow_exper
23452300
{
23462301
// It's possible to optimize count() given only partition predicates
23472302
ActionsDAG::NodeRawConstPtrs filter_nodes;
2303+
if (analysis_result.hasRowPolicyFilter())
2304+
{
2305+
auto & row_level_filter = analysis_result.row_policy_info;
2306+
filter_nodes.push_back(&row_level_filter->actions.findInOutputs(row_level_filter->column_name));
2307+
}
23482308
if (analysis_result.hasPrewhere())
23492309
{
23502310
auto & prewhere_info = analysis_result.prewhere_info;
23512311
filter_nodes.push_back(&prewhere_info->prewhere_actions.findInOutputs(prewhere_info->prewhere_column_name));
2352-
2353-
if (prewhere_info->row_level_filter)
2354-
filter_nodes.push_back(&prewhere_info->row_level_filter->findInOutputs(prewhere_info->row_level_column_name));
23552312
}
23562313
if (analysis_result.hasWhere())
23572314
{
@@ -2541,10 +2498,11 @@ void InterpreterSelectQuery::executeFetchColumns(QueryProcessingStage::Enum proc
25412498
streams_with_ratio);
25422499
}
25432500

2544-
auto & prewhere_info = analysis_result.prewhere_info;
2501+
if (analysis_result.row_policy_info && (!input_pipe && storage && storage->supportsPrewhere()))
2502+
query_info.row_level_filter = analysis_result.row_policy_info;
25452503

2546-
if (prewhere_info)
2547-
query_info.prewhere_info = prewhere_info;
2504+
if (analysis_result.prewhere_info)
2505+
query_info.prewhere_info = analysis_result.prewhere_info;
25482506

25492507
bool optimize_read_in_order = analysis_result.optimize_read_in_order;
25502508
bool optimize_aggregation_in_order = analysis_result.optimize_aggregation_in_order && !query_analyzer->useGroupingSetKey();

‎src/Interpreters/InterpreterSelectQuery.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -223,7 +223,7 @@ class InterpreterSelectQuery : public IInterpreterUnionOrSelectQuery
223223
ExpressionAnalysisResult analysis_result;
224224
/// For row-level security.
225225
RowPolicyFilterPtr row_policy_filter;
226-
FilterDAGInfoPtr filter_info;
226+
FilterDAGInfoPtr row_policy_info;
227227

228228
/// For additional_filter setting.
229229
FilterDAGInfoPtr additional_filter_info;

‎src/Interpreters/getHeaderForProcessingStage.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ Block getHeaderForProcessingStage(
9999
case QueryProcessingStage::FetchColumns:
100100
{
101101
Block header = storage_snapshot->getSampleBlockForColumns(column_names);
102-
header = SourceStepWithFilter::applyPrewhereActions(header, query_info.prewhere_info);
102+
header = SourceStepWithFilter::applyPrewhereActions(header, query_info.row_level_filter, query_info.prewhere_info);
103103
return header;
104104
}
105105
case QueryProcessingStage::WithMergeableState:

0 commit comments

Comments
 (0)