MDEV-29638 Crash when considering Split-Materialized plan

Variant 2: "Don't call optimize_stage2 too early"

The affected query had Split-Materialized derived table in another
derived table:

  select *   -- select#1
  from (
    select *  -- select#2
    from t1,
         (select * from t2 ... group by t2.group_col) DT -- select#3
    where t1.col=t2.group_col
  ) TBL;

The optimization went as follows:
JOIN::optimize() calls select_lex->handle_derived(DT_OPTIMIZE) which
calls JOIN::optimize() for all (direct and indirect) children SELECTs.

select#1->handle_derived() calls JOIN::optimize() for select#2 and #3;
select#2->handle_derived() calls JOIN::optimize() for select#3 the second
time. That call would call JOIN::optimize_stage2(), assuming the query
plan choice has been finalized for select#3.

But after that, JOIN::optimize() for select#2 would continue and consider
Split-Materialized for select#3. This would attempt to pick another join
order and cause a crash.

The fix is to have JOIN::optimize() not call optimize_stage2() ever.
Finalizing the query plan choice is now done by a separate call:
  select_lex->handle_derived(thd->lex, DT_OPTIMIZE_STAGE2)

which invokes JOIN::optimize_stage2() and saves the query plan.

Author: spetrunia

mysql-test/main/derived_split_innodb.result

@@ -1045,5 +1045,27 @@ WHERE t1.a LIKE 'B%';
 a
 DROP TABLE t1,t2;
 SET optimizer_switch= @save_optimizer_switch;
+#
+# MDEV-29638 Crash when considering Split-Materialized plan
+#
+set @save_optimizer_switch= @@optimizer_switch;
+set optimizer_switch='condition_pushdown_for_derived=off,split_materialized=on';
+CREATE TABLE t1 (id int PRIMARY KEY)engine=innodb;
+CREATE TABLE t2 (id int PRIMARY KEY, c int) engine=innodb;
+CREATE TABLE t3 (id int PRIMARY KEY, a int , b int, KEY (a))engine=innodb;
+SELECT * FROM
+(
+SELECT DISTINCT t1.id
+FROM t1 JOIN
+(
+SELECT t2.id FROM t2 JOIN t3
+ON t3.id = t2.c
+WHERE (t3.a > 2 AND t3.b = 2)
+GROUP BY t2.id
+) m2 ON m2.id = t1.id
+) dt;
+id
+drop table t1, t2, t3;
+SET optimizer_switch= @save_optimizer_switch;
 # End of 10.11 tests
 SET GLOBAL innodb_stats_persistent=@save_innodb_stats_persistent;

mysql-test/main/derived_split_innodb.test

@@ -664,5 +664,32 @@ eval $q;
 DROP TABLE t1,t2;
 
 SET optimizer_switch= @save_optimizer_switch;
+
+--echo #
+--echo # MDEV-29638 Crash when considering Split-Materialized plan
+--echo #
+
+set @save_optimizer_switch= @@optimizer_switch;
+set optimizer_switch='condition_pushdown_for_derived=off,split_materialized=on';
+
+CREATE TABLE t1 (id int PRIMARY KEY)engine=innodb;
+CREATE TABLE t2 (id int PRIMARY KEY, c int) engine=innodb;
+CREATE TABLE t3 (id int PRIMARY KEY, a int , b int, KEY (a))engine=innodb;
+
+SELECT * FROM
+(
+  SELECT DISTINCT t1.id
+    FROM t1 JOIN
+  (
+     SELECT t2.id FROM t2 JOIN t3
+       ON t3.id = t2.c
+       WHERE (t3.a > 2 AND t3.b = 2)
+       GROUP BY t2.id
+  ) m2 ON m2.id = t1.id
+) dt;
+
+drop table t1, t2, t3;
+SET optimizer_switch= @save_optimizer_switch;
+
 --echo # End of 10.11 tests
 SET GLOBAL innodb_stats_persistent=@save_innodb_stats_persistent;

sql/sql_derived.cc

@@ -41,6 +41,8 @@ typedef bool (*dt_processor)(THD *thd, LEX *lex, TABLE_LIST *derived);
 static bool mysql_derived_init(THD *thd, LEX *lex, TABLE_LIST *derived);
 static bool mysql_derived_prepare(THD *thd, LEX *lex, TABLE_LIST *derived);
 static bool mysql_derived_optimize(THD *thd, LEX *lex, TABLE_LIST *derived);
+static bool mysql_derived_optimize_stage2(THD *thd, LEX *lex,
+            TABLE_LIST *derived);
 static bool mysql_derived_merge(THD *thd, LEX *lex, TABLE_LIST *derived);
 static bool mysql_derived_create(THD *thd, LEX *lex, TABLE_LIST *derived);
 static bool mysql_derived_fill(THD *thd, LEX *lex, TABLE_LIST *derived);
@@ -58,6 +60,7 @@ dt_processor processors[]=
   &mysql_derived_create,
   &mysql_derived_fill,
   &mysql_derived_reinit,
+  &mysql_derived_optimize_stage2
 };
 
 /*
@@ -109,8 +112,8 @@ mysql_handle_derived(LEX *lex, uint phases)
       {
         if (!cursor->is_view_or_derived() && phases == DT_MERGE_FOR_INSERT)
           continue;
-        uint8 allowed_phases= (cursor->is_merged_derived() ? DT_PHASES_MERGE :
-                               DT_PHASES_MATERIALIZE | DT_MERGE_FOR_INSERT);
+        uint allowed_phases= (cursor->is_merged_derived() ? DT_PHASES_MERGE :
+                              DT_PHASES_MATERIALIZE | DT_MERGE_FOR_INSERT);
         /*
           Skip derived tables to which the phase isn't applicable.
           TODO: mark derived at the parse time, later set it's type
@@ -170,7 +173,7 @@ bool
 mysql_handle_single_derived(LEX *lex, TABLE_LIST *derived, uint phases)
 {
   bool res= FALSE;
-  uint8 allowed_phases= (derived->is_merged_derived() ? DT_PHASES_MERGE :
+  uint allowed_phases= (derived->is_merged_derived() ? DT_PHASES_MERGE :
                          DT_PHASES_MATERIALIZE);
   DBUG_ENTER("mysql_handle_single_derived");
   DBUG_PRINT("enter", ("phases: 0x%x  allowed: 0x%x  alias: '%s'",
@@ -1071,6 +1074,43 @@ bool mysql_derived_optimize(THD *thd, LEX *lex, TABLE_LIST *derived)
 }
 
 
+/*
+  @brief
+    Call JOIN::optimize_stage2_and_finish() for all child selects that use
+    two-phase optimization.
+*/
+
+static
+bool mysql_derived_optimize_stage2(THD *thd, LEX *lex, TABLE_LIST *derived)
+{
+  SELECT_LEX_UNIT *unit= derived->get_unit();
+  SELECT_LEX *first_select= unit->first_select();
+  SELECT_LEX *save_current_select= lex->current_select;
+  bool res= FALSE;
+
+  if (derived->merged || unit->is_unit_op())
+  {
+    /*
+      Two-phase join optimization is not applicable for merged derived tables
+      and UNIONs.
+    */
+    return FALSE;
+  }
+
+  lex->current_select= first_select;
+  /* Same condition as in mysql_derived_optimize(): */
+  if (unit->derived && !derived->is_merged_derived())
+  {
+    JOIN *join= first_select->join;
+    if (join && join->optimization_state == JOIN::OPTIMIZATION_PHASE_1_DONE)
+      res= join->optimize_stage2_and_finish();
+  }
+
+  lex->current_select= save_current_select;
+  return res;
+}
+
+
 /**
   Actually create result table for a materialized derived table/view.
 

sql/sql_select.cc

@@ -1947,7 +1947,6 @@ bool JOIN::build_explain()
 int JOIN::optimize()
 {
   int res= 0;
-  join_optimization_state init_state= optimization_state;
   if (select_lex->pushdown_select)
   {
     // Do same as JOIN::optimize_inner does:
@@ -1960,18 +1959,18 @@ int JOIN::optimize()
     }
     with_two_phase_optimization= false;
   }
-  else if (optimization_state == JOIN::OPTIMIZATION_PHASE_1_DONE)
-    res= optimize_stage2();
   else
   {
-    // to prevent double initialization on EXPLAIN
+    /*
+      This function may be invoked multiple times. Do nothing if the
+      optimization (either full or stage1) are already done.
+    */
     if (optimization_state != JOIN::NOT_OPTIMIZED)
       return FALSE;
     optimization_state= JOIN::OPTIMIZATION_IN_PROGRESS;
     res= optimize_inner();
   }
-  if (!with_two_phase_optimization ||
-      init_state == JOIN::OPTIMIZATION_PHASE_1_DONE)
+  if (!with_two_phase_optimization)
   {
     if (!res && have_query_plan != QEP_DELETED)
       res= build_explain();
@@ -1981,6 +1980,29 @@ int JOIN::optimize()
 }
 
 
+/*
+  @brief
+    Call optimize_stage2() and save the query plan.
+*/
+
+int JOIN::optimize_stage2_and_finish()
+{
+  int res= 0;
+  DBUG_ASSERT(with_two_phase_optimization);
+  DBUG_ASSERT(optimization_state == OPTIMIZATION_PHASE_1_DONE);
+
+  if (optimize_stage2())
+    res= 1;
+  else
+  {
+    if (have_query_plan != JOIN::QEP_DELETED)
+      res= build_explain();
+    optimization_state= JOIN::OPTIMIZATION_DONE;
+  }
+  return res;
+}
+
+
 /**
   @brief
     Create range filters objects needed in execution for all join tables
@@ -2691,6 +2713,19 @@ JOIN::optimize_inner()
 }
 
 
+/*
+  @brief
+    In the Stage 1 we've picked the join order.
+    Now, refine the query plan and sort out all the details.
+    The choice how to handle GROUP/ORDER BY is also made here.
+
+  @detail
+    The main reason this is a separate function is Split-Materialized
+    optimization. There, we first consider doing non-split Materialization for
+    a SELECT. After that, the parent SELECT will attempt doing Splitting in
+    multiple ways and make the final choice.
+*/
+
 int JOIN::optimize_stage2()
 {
   ulonglong select_opts_for_readinfo;
@@ -2711,7 +2746,7 @@ int JOIN::optimize_stage2()
   if (make_range_rowid_filters())
     DBUG_RETURN(1);
 
-  if (select_lex->handle_derived(thd->lex, DT_OPTIMIZE))
+  if (select_lex->handle_derived(thd->lex, DT_OPTIMIZE_STAGE2))
     DBUG_RETURN(1);
 
   if (optimizer_flag(thd, OPTIMIZER_SWITCH_DERIVED_WITH_KEYS))
@@ -3501,7 +3536,7 @@ int JOIN::optimize_stage2()
       some of the derived tables, and never did stage 2.
       Do it now, otherwise Explain data structure will not be complete.
     */
-    if (select_lex->handle_derived(thd->lex, DT_OPTIMIZE))
+    if (select_lex->handle_derived(thd->lex, DT_OPTIMIZE_STAGE2))
       DBUG_RETURN(1);
   }
   /*
@@ -4765,6 +4800,7 @@ bool JOIN::save_explain_data(Explain_query *output, bool can_overwrite,
 
 void JOIN::exec()
 {
+  DBUG_ASSERT(optimization_state == OPTIMIZATION_DONE);
   DBUG_EXECUTE_IF("show_explain_probe_join_exec_start", 
                   if (dbug_user_var_equals_int(thd, 
                                                "show_explain_probe_select_id", 

sql/sql_select.h

@@ -1676,6 +1676,7 @@ class JOIN :public Sql_alloc
   int optimize();
   int optimize_inner();
   int optimize_stage2();
+  int optimize_stage2_and_finish();
   bool build_explain();
   int reinit();
   int init_execution();

sql/table.h

@@ -2031,9 +2031,13 @@ class IS_table_read_plan;
 #define DT_CREATE           32U
 #define DT_FILL             64U
 #define DT_REINIT           128U
-#define DT_PHASES           8U
+
+#define DT_OPTIMIZE_STAGE2  256U
+
+/* Number of bits used by all phases: */
+#define DT_PHASES           9U
 /* Phases that are applicable to all derived tables. */
-#define DT_COMMON       (DT_INIT + DT_PREPARE + DT_REINIT + DT_OPTIMIZE)
+#define DT_COMMON       (DT_INIT + DT_PREPARE + DT_REINIT + DT_OPTIMIZE + DT_OPTIMIZE_STAGE2)
 /* Phases that are applicable only to materialized derived tables. */
 #define DT_MATERIALIZE  (DT_CREATE + DT_FILL)