Skip to content

Commit 2ed123b

Browse files
authored
Merge pull request ClickHouse#97990 from ClickHouse/revert-97498-improve-join-side-flipping-rules
Revert "Support swapping sides of ANTI, SEMI and FULL joins"
2 parents 4c91652 + 4f9b668 commit 2ed123b

9 files changed

Lines changed: 10 additions & 250 deletions

src/Processors/QueryPlan/Optimizations/joinOrder.cpp

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -550,9 +550,6 @@ std::optional<JoinKind> JoinOrderOptimizer::isValidJoinOrder(const BitSet & left
550550
return right_join_type;
551551
if (right_join_type == JoinKind::Inner)
552552
return left_join_type;
553-
/// Allow FULL join as it's restricted to table swapping and no reordering
554-
if (left_join_type == JoinKind::Full && right_join_type == JoinKind::Full)
555-
return JoinKind::Full;
556553

557554
/// Conflict, join is not possible:
558555
/// FROM t1 LEFT JOIN t2 LEFT JOIN t3

src/Processors/QueryPlan/Optimizations/optimizeJoin.cpp

Lines changed: 8 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -811,16 +811,6 @@ static const ActionsDAG::Node * trackInputColumn(const ActionsDAG::Node * node)
811811
return node;
812812
}
813813

814-
constexpr bool isSwapOnlyJoinKind(JoinKind kind)
815-
{
816-
return kind == JoinKind::Full;
817-
}
818-
819-
constexpr bool isSwapOnlyJoinStrictness(JoinStrictness strictness)
820-
{
821-
return strictness == JoinStrictness::Any || strictness == JoinStrictness::Semi || strictness == JoinStrictness::Anti;
822-
}
823-
824814
QueryPlan::Node chooseJoinOrder(QueryGraphBuilder query_graph_builder, QueryPlan::Nodes & nodes, JoinStrictness join_strictness)
825815
{
826816
QueryGraph query_graph;
@@ -961,17 +951,17 @@ QueryPlan::Node chooseJoinOrder(QueryGraphBuilder query_graph_builder, QueryPlan
961951
bool flip_join = has_prepared_storage_at_left || (!has_prepared_storage_at_right && swap_on_sizes);
962952

963953
/// fixme: USING clause handled specially in join algorithm, so swap breaks it
964-
/// At the time of writing, we're not able to swap inputs for ANY or SEMI partial merge join, because it only supports inner or left joins, but not right
965-
/// ANTI partial merge join is not supported for any join kind
954+
/// fixme: Swapping for SEMI and ANTI joins should be alright, need to try to enable it and test
955+
/// At the time of writing, we're not able to swap inputs for ANY partial merge join, because it only supports ANY inner or left joins, but not right.
966956
const bool partial_merge_join_can_be_selected = std::ranges::any_of(
967957
join_settings.join_algorithms,
968958
[](JoinAlgorithm alg)
969959
{ return alg == JoinAlgorithm::PARTIAL_MERGE || alg == JoinAlgorithm::PREFER_PARTIAL_MERGE || alg == JoinAlgorithm::AUTO; });
970960
const bool should_worry_about_partial_merge_join = partial_merge_join_can_be_selected
971961
&& (!MergeJoin::isSupported(join_operator.kind, join_operator.strictness)
972962
|| !MergeJoin::isSupported(reverseJoinKind(join_operator.kind), join_operator.strictness));
973-
const bool suitable_swap_only_join = isSwapOnlyJoinStrictness(join_operator.strictness) && !should_worry_about_partial_merge_join;
974-
if (join_operator.strictness != JoinStrictness::All && !suitable_swap_only_join)
963+
const bool suitable_any_join = join_operator.strictness == JoinStrictness::Any && !should_worry_about_partial_merge_join;
964+
if (join_operator.strictness != JoinStrictness::All && !suitable_any_join)
975965
flip_join = false;
976966

977967
if (flip_join)
@@ -1182,9 +1172,10 @@ void optimizeJoinLogicalImpl(JoinStepLogical * join_step, QueryPlan::Node & node
11821172
auto kind = join_operator.kind;
11831173
auto locality = join_operator.locality;
11841174
if (!optimization_settings.query_plan_optimize_join_order_limit
1185-
|| (strictness != JoinStrictness::All && !isSwapOnlyJoinStrictness(strictness))
1175+
|| (strictness != JoinStrictness::All && strictness != JoinStrictness::Any)
11861176
|| locality != JoinLocality::Unspecified
11871177
|| kind == JoinKind::Paste
1178+
|| kind == JoinKind::Full
11881179
|| !join_operator.residual_filter.empty()
11891180
)
11901181
{
@@ -1196,8 +1187,8 @@ void optimizeJoinLogicalImpl(JoinStepLogical * join_step, QueryPlan::Node & node
11961187
query_graph_builder.context->dummy_stats = join_step->getDummyStats();
11971188

11981189
int query_graph_size_limit = safe_cast<int>(optimization_settings.query_plan_optimize_join_order_limit);
1199-
if ((isSwapOnlyJoinStrictness(strictness) || isSwapOnlyJoinKind(kind)) && query_graph_size_limit > 2)
1200-
/// Do not reorder joins, only allow swap
1190+
if (strictness == JoinStrictness::Any)
1191+
/// Do not reorder ANY joins, only allow swap
12011192
query_graph_size_limit = 2;
12021193

12031194
buildQueryGraph(query_graph_builder, node, nodes, query_graph_size_limit);

tests/queries/0_stateless/03623_convert_any_join_to_semi_or_anti_2.reference

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,8 +67,8 @@ Positions: 0
6767
FUNCTION greater(__table3.age :: 0, 1_UInt8 :: 1) -> greater(__table3.age, 1_UInt8) UInt8 : 2
6868
Positions: 2
6969
JoinLogical
70-
Join: u1[100000] ⋉ u2[1000]
71-
ResultRows: 100000
70+
Join: u1 ⋉ u2
71+
ResultRows: unknown
7272
Type: LEFT
7373
Strictness: SEMI
7474
Locality: UNSPECIFIED

tests/queries/0_stateless/03926_semi_join_swap_tables.reference

Lines changed: 0 additions & 12 deletions
This file was deleted.

tests/queries/0_stateless/03926_semi_join_swap_tables.sql

Lines changed: 0 additions & 100 deletions
This file was deleted.

tests/queries/0_stateless/03927_anti_join_swap_tables.reference

Lines changed: 0 additions & 6 deletions
This file was deleted.

tests/queries/0_stateless/03927_anti_join_swap_tables.sql

Lines changed: 0 additions & 64 deletions
This file was deleted.

tests/queries/0_stateless/03928_full_join_swap_tables.reference

Lines changed: 0 additions & 4 deletions
This file was deleted.

tests/queries/0_stateless/03928_full_join_swap_tables.sql

Lines changed: 0 additions & 42 deletions
This file was deleted.

0 commit comments

Comments
 (0)