Bug#36734772: Hypergraph optimizer fails to use index for t1 INNER

kahatlen · kahatlen · commit 0f0ec74b0932 · 2024-09-11T07:16:01.000+02:00
JOIN t2 RIGHT JOIN t3

The hypergraph optimizer did not use indexes in some queries where an
inner join was on the inner side of an outer join or on the outer side
of a semijoin.

This happened if both tables in the inner join could use index
lookups, and one of the index lookups was parameterized on the other
table in the inner join and the other index lookup was parameterized
on the table on the other side of the outer join or semijoin.

The problem was that DisallowParameterizedJoinPath() was too
restrictive, and rejected all good plans for the query. It tries to
avoid creating parameterized join paths if the parameterization could
have been resolved earlier, so that the optimizer doesn't have to
spend time unnecessarily on plans that are unlikely to be any better
than the alternatives. In these cases, the parameterization couldn't
have been resolved earlier, so it is too strict to reject these plans.

This code is now changed so that it also allows creating a
parameterized join if the left side of the join fully resolves the
parameterization of the right side.

Change-Id: I20728c24fe20128a76ae96772255a648842e65c3
diff --git a/mysql-test/r/explain_tree_hypergraph.result b/mysql-test/r/explain_tree_hypergraph.result
@@ -393,7 +393,8 @@ JSON_SEARCH(@var, 'one', '%FirstMatch%', NULL, '$**.operation') IS NOT NULL
 AS operation_contains_firstmatch;
 first_match	operation_contains_firstmatch
 ["firstmatch"]	1
-EXPLAIN FORMAT=tree SELECT * FROM t WHERE a IN (SELECT t1.a FROM t AS t1 JOIN t AS t2 ON t1.a=t2.b);
+EXPLAIN FORMAT=tree SELECT * FROM t WHERE a IN
+(SELECT /*+ SEMIJOIN(LOOSESCAN) */ t1.a FROM t AS t1 JOIN t AS t2 ON t1.a=t2.b);
 EXPLAIN
 -> Nested loop inner join (LooseScan)  (rows=1.41)
     -> Remove duplicates from input grouped on t2.b  (rows=1.41)
@@ -403,7 +404,8 @@ EXPLAIN
             -> Covering index lookup on t2 using b (b = t1.a)  (rows=1)
     -> Covering index lookup on t using a (a = t2.b)  (rows=1)
 
-EXPLAIN FORMAT=JSON INTO @var SELECT * FROM t WHERE a IN (SELECT t1.a FROM t AS t1 JOIN t AS t2 ON t1.a=t2.b);
+EXPLAIN FORMAT=JSON INTO @var SELECT * FROM t WHERE a IN
+(SELECT /*+ SEMIJOIN(LOOSESCAN) */ t1.a FROM t AS t1 JOIN t AS t2 ON t1.a=t2.b);
 SELECT
 JSON_EXTRACT(@var, '$**.semijoin_strategy') AS loosescan,
 JSON_SEARCH(@var, 'one', '%LooseScan%', NULL, '$**.operation') IS NOT NULL
diff --git a/mysql-test/r/opt_hints_subquery_hypergraph.result b/mysql-test/r/opt_hints_subquery_hypergraph.result
@@ -173,12 +173,11 @@ SELECT * FROM t3
 WHERE t3.a IN (SELECT /*+ QB_NAME(subq1) */ a FROM t1 tx
 WHERE tx.b IN (SELECT /*+ QB_NAME(subq2) */ a FROM t1 ty));
 EXPLAIN
--> Hash semijoin (FirstMatch) (t3.a = tx.a)  (rows=1.5)
+-> Nested loop semijoin (FirstMatch)  (rows=1.5)
     -> Table scan on t3  (rows=3)
-    -> Hash
-        -> Nested loop inner join  (rows=4)
-            -> Table scan on tx  (rows=4)
-            -> Single-row covering index lookup on ty using PRIMARY (a = tx.b)  (rows=1)
+    -> Nested loop inner join  (rows=1)
+        -> Single-row index lookup on tx using PRIMARY (a = t3.a)  (rows=1)
+        -> Single-row covering index lookup on ty using PRIMARY (a = tx.b)  (rows=1)
 
 No SEMIJOIN transformation for outer subquery
 EXPLAIN
@@ -1428,12 +1427,11 @@ SELECT /*+ SEMIJOIN(@subq1) SEMIJOIN(@subq2) */ * FROM t3
 WHERE t3.a IN (SELECT /*+ QB_NAME(subq1) */ a FROM t1 tx
 WHERE tx.b IN (SELECT /*+ QB_NAME(subq2) */ a FROM t1 ty));
 EXPLAIN
--> Hash semijoin (FirstMatch) (t3.a = tx.a)  (rows=1.5)
+-> Nested loop semijoin (FirstMatch)  (rows=1.5)
     -> Table scan on t3  (rows=3)
-    -> Hash
-        -> Nested loop inner join  (rows=4)
-            -> Table scan on tx  (rows=4)
-            -> Single-row covering index lookup on ty using PRIMARY (a = tx.b)  (rows=1)
+    -> Nested loop inner join  (rows=1)
+        -> Single-row index lookup on tx using PRIMARY (a = t3.a)  (rows=1)
+        -> Single-row covering index lookup on ty using PRIMARY (a = tx.b)  (rows=1)
 
 Test strategies when some are disabled by optimizer_switch
 SET optimizer_switch='semijoin=on';
diff --git a/mysql-test/t/explain_tree_hypergraph.test b/mysql-test/t/explain_tree_hypergraph.test
@@ -256,7 +256,8 @@ JSON_EXTRACT(@var, '$**.semijoin_strategy') AS first_match,
 JSON_SEARCH(@var, 'one', '%FirstMatch%', NULL, '$**.operation') IS NOT NULL
 AS operation_contains_firstmatch;
 # LooseScan strategy
-let $query = SELECT * FROM t WHERE a IN (SELECT t1.a FROM t AS t1 JOIN t AS t2 ON t1.a=t2.b);
+let $query = SELECT * FROM t WHERE a IN
+(SELECT /*+ SEMIJOIN(LOOSESCAN) */ t1.a FROM t AS t1 JOIN t AS t2 ON t1.a=t2.b);
 --replace_regex $elide_costs
 eval EXPLAIN FORMAT=tree $query;
 eval EXPLAIN FORMAT=JSON INTO @var $query;
diff --git a/sql/join_optimizer/join_optimizer.cc b/sql/join_optimizer/join_optimizer.cc
@@ -4228,12 +4228,49 @@ NodeMap FindReachableTablesFrom(NodeMap tables, const JoinHypergraph &graph) {
   return reachable;
 }
 
-// Returns whether the given set of parameter tables is partially, but not
-// fully, resolved by joining towards the other side.
-bool PartiallyResolvedParameterization(NodeMap parameter_tables,
-                                       NodeMap other_side) {
-  return (parameter_tables & ~other_side) != 0 &&
-         (parameter_tables & ~other_side) != parameter_tables;
+/**
+  Is it possible to resolve more parameter tables before performing a nested
+  loop join between "outer" and "inner", or will the join have to be performed
+  first?
+
+  In more precise terms:
+
+  Consider the set of parameters (a set of tables) that are left unresolved
+  after joining inner and outer. This function returns true if this set is
+  non-empty and at least one of these unresolved parameter tables, denoted by t,
+  can be joined directly into either outer or inner such that the result of
+  joining either {outer, t} with {inner} or {outer} with {inner, t} would end up
+  with more resolved parameters (fewer unresolved parameters) than simply
+  joining {outer} and {inner}.
+ */
+bool CanResolveMoreParameterTables(NodeMap outer, NodeMap inner,
+                                   NodeMap outer_parameters,
+                                   NodeMap inner_parameters,
+                                   NodeMap outer_reachable,
+                                   NodeMap inner_reachable) {
+  const NodeMap unresolved_parameters =
+      (outer_parameters | inner_parameters) & ~(outer | inner);
+
+  if (unresolved_parameters == 0) {
+    // No unresolved parameters after joining outer and inner (so we cannot
+    // resolve more parameters by first joining in parameter tables).
+    return false;
+  }
+
+  // Unresolved parameterizations on either side of the join can be resolved by
+  // joining a parameter table into the outer path first, if it's reachable.
+  if (Overlaps(unresolved_parameters, outer_reachable)) {
+    return true;
+  }
+
+  // Unresolved parameterizations that are only on the inner path, can also be
+  // resolved by joining a parameter table to the inner path first, if it's
+  // reachable.
+  if (Overlaps(unresolved_parameters & ~outer_parameters, inner_reachable)) {
+    return true;
+  }
+
+  return false;
 }
 
 /**
@@ -4245,40 +4282,72 @@ bool PartiallyResolvedParameterization(NodeMap parameter_tables,
   plans to be left-deep (since such plans never gain anything from being
   bushy), reducing the search space significantly without compromising
   plan quality.
+
+  @param left_path An access path which joins together a superset of all the
+  tables on the left-hand side of the hyperedge for which we are creating a
+  join.
+
+  @param right_path An access path which joins together a superset of all the
+  tables on the right-hand side of the hyperedge for which we are creating a
+  join.
+
+  @param left The set of tables joined together in "left_path".
+
+  @param right The set of tables joined together in "right_path".
+
+  @param left_reachable The set of tables that can be joined directly with
+  "left_path", with no intermediate join being performed first. If a table is in
+  this set, it is possible to construct a nested loop join between an access
+  path accessing only that table and the access path pointed to by "left_path".
+
+  @param right_reachable The set of tables that can be joined directly with
+  "right_path", with no intermediate join being performed first. If a table is
+  in this set, it is possible to construct a nested loop join between an access
+  path accessing only that table and the access path pointed to by "right_path".
+
+  @param is_reorderable True if the optimizer may try to construct a nested loop
+  join between "left_path" and "right_path" in either direction. False if the
+  optimizer will consider nested loop joins in only one direction, with
+  "left_path" as the outer table and "right_path" as the inner table. When it is
+  true, we disallow a parameterized join path only if it is possible to resolve
+  more parameter tables first in both join orders. This is slightly more lenient
+  than it has to be, as it will allow parameterized join paths with both join
+  orders, even though one of the orders can join with a parameter table first.
+  Since all of these joins will be parameterized on the same set of tables, this
+  extra leniency is not believed to contribute much to the explosion of plans
+  with different parameterizations.
  */
 bool DisallowParameterizedJoinPath(AccessPath *left_path,
                                    AccessPath *right_path, NodeMap left,
                                    NodeMap right, NodeMap left_reachable,
-                                   NodeMap right_reachable) {
+                                   NodeMap right_reachable,
+                                   bool is_reorderable) {
   const NodeMap left_parameters = left_path->parameter_tables & ~RAND_TABLE_BIT;
   const NodeMap right_parameters =
       right_path->parameter_tables & ~RAND_TABLE_BIT;
 
-  if (IsSubset(left_parameters | right_parameters, left | right)) {
-    // Not creating a parameterized path, so it's always fine.
+  if (!CanResolveMoreParameterTables(left, right, left_parameters,
+                                     right_parameters, left_reachable,
+                                     right_reachable)) {
+    // Neither left nor right can resolve parameterization that is left
+    // unresolved by this join by first joining in one of the parameter tables.
+    // E.g., we're still on the inside of an outer join, and the parameter
+    // tables are outside the outer join, and we still need to join together
+    // more tables on the inner side of the outer join before we're allowed to
+    // do the outer join. We have to allow creation of a parameterized join path
+    // if we want to use index lookups here at all.
     return false;
   }
 
-  if (!Overlaps(right_parameters, right_reachable) &&
-      !Overlaps(left_parameters, left_reachable)) {
-    // Either left or right cannot resolve any of their parameterizations yet
-    // (e.g., we're still on the inside of an outer join that we cannot
-    // finish yet), so we cannot avoid keeping them if we want to use index
-    // lookups here at all.
-    return false;
-  }
-
-  // If the outer table partially, but not fully, resolves the inner table's
-  // parameterization, we still allow it (otherwise, we could not have
-  // multi-part index lookups where the keyparts come from different tables).
-  // This is the so-called “star-schema exception”.
-  //
-  // We need to check both ways, in case we try to swap them for a hash join.
-  // Only one of these will ever be true in any given join anyway (joins where
-  // we try to resolve the outer path's parameterizations with the inner one
-  // are disallowed), so we do not allow more than is required.
-  if (PartiallyResolvedParameterization(left_parameters, right) ||
-      PartiallyResolvedParameterization(right_parameters, left)) {
+  // If the join can be performed both ways (such as a commutable join
+  // operation, or a semijoin that can be rewritten to an inner join), we're a
+  // bit more lenient and allow creation of a parameterized join path even
+  // though a parameter table can be resolved first, if it is not possible to
+  // resolve any parameter tables first in the reordered join. Otherwise, we
+  // might not be able to use indexes in the reordered join.
+  if (is_reorderable && !CanResolveMoreParameterTables(
+                            right, left, right_parameters, left_parameters,
+                            right_reachable, left_reachable)) {
     return false;
   }
 
@@ -4565,9 +4634,21 @@ bool CostingReceiver::FoundSubgraphPair(NodeMap left, NodeMap right,
       zero_path->delayed_predicates = right_path->delayed_predicates;
       right_path = zero_path;
     }
+
+    // Can this join be performed in both left-right and right-left order? It
+    // can if the join operation is commutative (or rewritable to one) and
+    // right_path's parameterization doesn't force it to be on the right side.
+    // If this condition is true, the right-left join will be attempted proposed
+    // in addition to the left-right join, but the additional checks in
+    // AllowNestedLoopJoin() and AllowHashJoin() decide if they are actually
+    // proposed.
+    const bool is_reorderable = (is_commutative || can_rewrite_semi_to_inner) &&
+                                !Overlaps(right_path->parameter_tables, left);
+
     for (AccessPath *left_path : left_it->second.paths) {
       if (DisallowParameterizedJoinPath(left_path, right_path, left, right,
-                                        left_reachable, right_reachable)) {
+                                        left_reachable, right_reachable,
+                                        is_reorderable)) {
         continue;
       }
 
@@ -4606,7 +4687,7 @@ bool CostingReceiver::FoundSubgraphPair(NodeMap left, NodeMap right,
                           new_obsolete_orderings,
                           /*rewrite_semi_to_inner=*/false, &wrote_trace);
         }
-        if (is_commutative || can_rewrite_semi_to_inner) {
+        if (is_reorderable) {
           ProposeHashJoin(right, left, right_path, left_path, edge, new_fd_set,
                           new_obsolete_orderings,
                           /*rewrite_semi_to_inner=*/can_rewrite_semi_to_inner,
@@ -4617,7 +4698,7 @@ bool CostingReceiver::FoundSubgraphPair(NodeMap left, NodeMap right,
       ProposeNestedLoopJoin(left, right, left_path, right_path, edge,
                             /*rewrite_semi_to_inner=*/false, new_fd_set,
                             new_obsolete_orderings, &wrote_trace);
-      if (is_commutative || can_rewrite_semi_to_inner) {
+      if (is_reorderable) {
         ProposeNestedLoopJoin(
             right, left, right_path, left_path, edge,
             /*rewrite_semi_to_inner=*/can_rewrite_semi_to_inner, new_fd_set,
diff --git a/unittest/gunit/hypergraph_optimizer-t.cc b/unittest/gunit/hypergraph_optimizer-t.cc
@@ -2726,6 +2726,125 @@ TEST_F(HypergraphOptimizerTest, InnerNestloopShouldBeLeftDeep) {
   // We don't verify the plan in itself.
 }
 
+// Verify that we can produce plans on this form for an inner join inside a left
+// outer join:
+//
+//     -> Nested loop left join
+//         -> Table scan on t1
+//         -> Nested loop inner join
+//             -> Single-row index lookup on t2 using key0 (x = t1.x)
+//             -> Single-row index lookup on t3 using key0 (x = t2.y)
+//
+// We should be able to use index lookups for both tables in the inner join.
+TEST_F(HypergraphOptimizerTest, UseIndexesInInnerJoinInsideOuterJoin) {
+  Query_block *query_block = ParseAndResolve(
+      "SELECT 1 FROM t1 LEFT JOIN t2 INNER JOIN t3 ON t2.y=t3.x ON t1.x=t2.x",
+      /*nullable=*/true);
+
+  // Make the outer table small, so that it looks attractive with a nested loop
+  // with t1 on the left side and index lookups on t2 and t3 on the right side.
+  Fake_TABLE *t1 = m_fake_tables["t1"];
+  t1->file->stats.records = 10;
+  t1->file->stats.data_file_length = 1000;
+
+  // Make t2 and t3 big, so that using index lookups looks more attractive
+  // than scanning the tables, and create unique indexes on t2(x) and t3(x).
+  for (string table_name : {"t2", "t3"}) {
+    Fake_TABLE *t23 = m_fake_tables[table_name];
+    t23->file->stats.records = 1e6;
+    t23->file->stats.data_file_length = 1e9;
+    t23->create_index(t23->field[0], HA_NOSAME);
+  }
+
+  TraceGuard trace(m_thd);
+  AccessPath *root = FindBestQueryPlan(m_thd, query_block);
+  SCOPED_TRACE(trace.contents());  // Prints out the trace on failure.
+  ASSERT_NE(nullptr, root);
+  // Prints out the query plan on failure.
+  SCOPED_TRACE(PrintQueryPlan(0, root, query_block->join,
+                              /*is_root_of_join=*/true));
+
+  // Expect the plan to be NLJ(t1, NLJ(INDEX_LOOKUP(t2), INDEX_LOOKUP(t3))). It
+  // used to do full table scans on t2 and t3 instead of index lookups.
+  ASSERT_EQ(AccessPath::NESTED_LOOP_JOIN, root->type);
+  const auto &outer_join = root->nested_loop_join();
+
+  ASSERT_EQ(AccessPath::NESTED_LOOP_JOIN, outer_join.inner->type);
+  const auto &inner_join = outer_join.inner->nested_loop_join();
+
+  ASSERT_EQ(AccessPath::EQ_REF, inner_join.outer->type);
+  EXPECT_STREQ("t2", inner_join.outer->eq_ref().table->alias);
+
+  ASSERT_EQ(AccessPath::EQ_REF, inner_join.inner->type);
+  EXPECT_STREQ("t3", inner_join.inner->eq_ref().table->alias);
+}
+
+// Verify that we can produce plans on this form for a semijoin with an inner
+// join on the outer side.
+//
+//     -> Nested loop inner join (LooseScan)
+//         -> Remove duplicates from input grouped on t3.x, t3.y
+//             -> Sort: t3.x, t3.y
+//                 -> Table scan on t3
+//         -> Filter: (t1.y = t3.y)
+//             -> Nested loop inner join
+//                 -> Single-row index lookup on t2 using key0 (x = t3.x)
+//                 -> Single-row index lookup on t1 using key0 (x = t2.y)
+//
+// We should be able to put the inner join on the right hand side of a nested
+// loop join, so that we can use index lookups on both the tables that are outer
+// to the semijoin.
+TEST_F(HypergraphOptimizerTest, UseIndexesInInnerJoinOutsideSemijoin) {
+  Query_block *query_block = ParseAndResolve(
+      "SELECT 1 FROM t1, t2 WHERE t1.x = t2.y AND "
+      "(t2.x, t1.y) IN (SELECT t3.x, t3.y FROM t3)",
+      /*nullable=*/true);
+
+  // Make t1 and t2 big, so that using index lookups looks more attractive
+  // than scanning the tables, and create unique indexes on t1(x) and t2(x).
+  for (string table_name : {"t1", "t2"}) {
+    Fake_TABLE *t12 = m_fake_tables[table_name];
+    t12->file->stats.records = 1e6;
+    t12->file->stats.data_file_length = 1e9;
+    t12->create_index(t12->field[0], HA_NOSAME);
+  }
+
+  // Make t3 small, so that it looks attractive with a nested loop with t3 on
+  // the left side and index lookups on t1 and t2 on the right side.
+  Fake_TABLE *t3 = m_fake_tables["t3"];
+  t3->file->stats.records = 10;
+  t3->file->stats.data_file_length = 1000;
+
+  TraceGuard trace(m_thd);
+  AccessPath *root = FindBestQueryPlan(m_thd, query_block);
+  SCOPED_TRACE(trace.contents());  // Prints out the trace on failure.
+  ASSERT_NE(nullptr, root);
+  // Prints out the query plan on failure.
+  SCOPED_TRACE(PrintQueryPlan(0, root, query_block->join,
+                              /*is_root_of_join=*/true));
+
+  // Expect the plan to be
+  // NLJ(REMOVE_DUPS(t3), FILTER(NLJ(INDEX_LOOKUP(t2), INDEX_LOOKUP(t1)))).
+  // It used to do a full table scan on t1 instead of an index lookup.
+  ASSERT_EQ(AccessPath::NESTED_LOOP_JOIN, root->type);
+  const auto &outer_join = root->nested_loop_join();
+  EXPECT_EQ(AccessPath::REMOVE_DUPLICATES, outer_join.outer->type);
+
+  // The exact placement of the t1.y=t3.y filter is not important. It could also
+  // have been pushed down directly on top of the index lookup on t1(x). See
+  // bug#33477822.
+  ASSERT_EQ(AccessPath::FILTER, outer_join.inner->type);
+  ASSERT_EQ(AccessPath::NESTED_LOOP_JOIN,
+            outer_join.inner->filter().child->type);
+  const auto &inner_join = outer_join.inner->filter().child->nested_loop_join();
+
+  ASSERT_EQ(AccessPath::EQ_REF, inner_join.outer->type);
+  EXPECT_STREQ("t2", inner_join.outer->eq_ref().table->alias);
+
+  ASSERT_EQ(AccessPath::EQ_REF, inner_join.inner->type);
+  EXPECT_STREQ("t1", inner_join.inner->eq_ref().table->alias);
+}
+
 TEST_F(HypergraphOptimizerTest, CombineFilters) {
   Query_block *query_block = ParseAndResolve(
       "SELECT 1 FROM t1 WHERE t1.x = 1 HAVING RAND() > 0.5", /*nullable=*/true);
