Fixed LP BUG#652727 and LP BUG#643424.

The fixes for #643424 was part of the fix for #652727, that's why both
fixes are pushed together.

- The cause for #643424 was the improper use of get_partial_join_cost(),
  which assumed that the 'n_tables' parameter was the upper bound for
  query plan node indexes.
  Fixed by generalizing get_partial_join_cost() as a method that computes
  the cost of any partial join.

- The cause of #652727 was that JOIN::choose_subquery_plan() incorrectly
  deleted the contents of the old keyuse array in the cases when an injected
  plan would not provide more key accesses, and reoptimization was not actually
  performed.

mysql-test/r/subselect_mat_cost.result

@@ -3769,3 +3769,54 @@ drop procedure delete_materialization_data;
 drop procedure set_all_columns_not_null;
 drop procedure set_all_columns_nullable;
 drop table t1, t2, t1_1024, t2_1024;
+#
+# LP BUG#643424 valgrind warning in choose_subquery_plan()
+#
+CREATE TABLE t1 (
+pk int(11) NOT NULL AUTO_INCREMENT,
+c1 int(11) DEFAULT NULL,
+c2 int(11) DEFAULT NULL,
+PRIMARY KEY (pk),
+KEY c2 (c2));
+INSERT INTO t1 VALUES (1,NULL,2);
+INSERT INTO t1 VALUES (2,7,9);
+INSERT INTO t1 VALUES (9,NULL,8);
+CREATE TABLE t2 (
+pk int(11) NOT NULL AUTO_INCREMENT,
+c1 int(11) DEFAULT NULL,
+c2 int(11) DEFAULT NULL,
+PRIMARY KEY (pk),
+KEY c2 (c2));
+INSERT INTO t2 VALUES (1,1,7);
+set @save_optimizer_switch=@@optimizer_switch;
+set @@optimizer_switch='materialization=on,in_to_exists=on,semijoin=off';
+SELECT pk FROM t1 WHERE (c2, c1) IN (SELECT c2, c2 FROM t2);
+pk
+set session optimizer_switch=@save_optimizer_switch;
+drop table t1, t2;
+#
+# LP BUG#652727 Crash in create_ref_for_key()
+#
+CREATE TABLE t2 (
+pk int(11) NOT NULL AUTO_INCREMENT,
+c1 int(11) DEFAULT NULL,
+PRIMARY KEY (pk));
+INSERT INTO t2 VALUES (10,7);
+INSERT INTO t2 VALUES (11,1);
+INSERT INTO t2 VALUES (17,NULL);
+CREATE TABLE t1 (
+pk int(11) NOT NULL AUTO_INCREMENT,
+c1 int(11) DEFAULT NULL,
+PRIMARY KEY (pk));
+INSERT INTO t1 VALUES (15,1);
+INSERT INTO t1 VALUES (19,NULL);
+CREATE TABLE t3 (c2 int(11) DEFAULT NULL, KEY c2 (c2));
+INSERT INTO t3 VALUES (1);
+set @save_optimizer_switch=@@optimizer_switch;
+set @@optimizer_switch='materialization=on,in_to_exists=on,semijoin=off';
+SELECT c2
+FROM t3
+WHERE (2, 6) IN (SELECT t1.c1, t1.c1 FROM t1 STRAIGHT_JOIN t2 ON t2.pk = t1.pk);
+c2
+set session optimizer_switch=@save_optimizer_switch;
+drop table t1, t2, t3;

mysql-test/t/subselect_mat_cost.test

@@ -201,3 +201,71 @@ drop procedure delete_materialization_data;
 drop procedure set_all_columns_not_null;
 drop procedure set_all_columns_nullable;
 drop table t1, t2, t1_1024, t2_1024;
+
+--echo #
+--echo # LP BUG#643424 valgrind warning in choose_subquery_plan()
+--echo #
+
+CREATE TABLE t1 (
+  pk int(11) NOT NULL AUTO_INCREMENT,
+  c1 int(11) DEFAULT NULL,
+  c2 int(11) DEFAULT NULL,
+  PRIMARY KEY (pk),
+  KEY c2 (c2));
+
+INSERT INTO t1 VALUES (1,NULL,2);
+INSERT INTO t1 VALUES (2,7,9);
+INSERT INTO t1 VALUES (9,NULL,8);
+
+CREATE TABLE t2 (
+  pk int(11) NOT NULL AUTO_INCREMENT,
+  c1 int(11) DEFAULT NULL,
+  c2 int(11) DEFAULT NULL,
+  PRIMARY KEY (pk),
+  KEY c2 (c2));
+
+INSERT INTO t2 VALUES (1,1,7);
+
+set @save_optimizer_switch=@@optimizer_switch;
+set @@optimizer_switch='materialization=on,in_to_exists=on,semijoin=off';
+
+SELECT pk FROM t1 WHERE (c2, c1) IN (SELECT c2, c2 FROM t2);
+
+set session optimizer_switch=@save_optimizer_switch;
+
+drop table t1, t2;
+
+
+--echo #
+--echo # LP BUG#652727 Crash in create_ref_for_key()
+--echo #
+
+CREATE TABLE t2 (
+  pk int(11) NOT NULL AUTO_INCREMENT,
+  c1 int(11) DEFAULT NULL,
+  PRIMARY KEY (pk));
+
+INSERT INTO t2 VALUES (10,7);
+INSERT INTO t2 VALUES (11,1);
+INSERT INTO t2 VALUES (17,NULL);
+
+CREATE TABLE t1 (
+  pk int(11) NOT NULL AUTO_INCREMENT,
+  c1 int(11) DEFAULT NULL,
+  PRIMARY KEY (pk));
+
+INSERT INTO t1 VALUES (15,1);
+INSERT INTO t1 VALUES (19,NULL);
+
+CREATE TABLE t3 (c2 int(11) DEFAULT NULL, KEY c2 (c2));
+INSERT INTO t3 VALUES (1);
+
+set @save_optimizer_switch=@@optimizer_switch;
+set @@optimizer_switch='materialization=on,in_to_exists=on,semijoin=off';
+
+SELECT c2
+FROM t3
+WHERE (2, 6) IN (SELECT t1.c1, t1.c1 FROM t1 STRAIGHT_JOIN t2 ON t2.pk = t1.pk);
+
+set session optimizer_switch=@save_optimizer_switch;
+drop table t1, t2, t3;

sql/opt_subselect.cc

@@ -1204,8 +1204,8 @@ bool optimize_semijoin_nests(JOIN *join, table_map all_table_map)
         sjm->tables= n_tables;
         sjm->is_used= FALSE;
         double subjoin_out_rows, subjoin_read_time;
-        get_partial_join_cost(join, n_tables,
-                              &subjoin_read_time, &subjoin_out_rows);
+        join->get_partial_join_cost(n_tables + join->const_tables,
+                                    &subjoin_read_time, &subjoin_out_rows);
 
         sjm->materialization_cost.convert_from_cost(subjoin_read_time);
         sjm->rows= subjoin_out_rows;
@@ -3615,16 +3615,14 @@ bool JOIN::optimize_unflattened_subqueries()
 */
 
 bool JOIN::choose_subquery_plan(table_map join_tables)
-{
-  /* The original QEP of the subquery. */
+{  /* The original QEP of the subquery. */
   DYNAMIC_ARRAY save_keyuse; /* Copy of the JOIN::keyuse array. */
   POSITION save_best_positions[MAX_TABLES+1]; /* Copy of JOIN::best_positions */
   /* Copies of the JOIN_TAB::keyuse pointers for each JOIN_TAB. */
   KEYUSE *save_join_tab_keyuse[MAX_TABLES];
   /* Copies of JOIN_TAB::checked_keys for each JOIN_TAB. */
   key_map save_join_tab_checked_keys[MAX_TABLES];
-
-  bool in_exists_reoptimized= false;
+  enum_reopt_result reopt_result= REOPT_NONE;
   Item_in_subselect *in_subs;
 
   if (select_lex->master_unit()->item &&
@@ -3667,8 +3665,8 @@ bool JOIN::choose_subquery_plan(table_map join_tables)
     double in_exists_strategy_cost;
 
     if (outer_join)
-      get_partial_join_cost(outer_join, outer_join->tables,
-                            &outer_read_time, &outer_record_count);
+      outer_join->get_partial_join_cost(outer_join->tables,
+                                        &outer_read_time, &outer_record_count);
     else
     {
       /*
@@ -3679,8 +3677,8 @@ bool JOIN::choose_subquery_plan(table_map join_tables)
       outer_record_count= 1;  /* TODO */
     }
 
-    get_partial_join_cost(inner_join, inner_join->tables,
-                          &inner_read_time_1, &inner_record_count_1);
+    inner_join->get_partial_join_cost(inner_join->tables,
+                                      &inner_read_time_1, &inner_record_count_1);
 
     if (in_to_exists_where && const_tables != tables)
     {
@@ -3689,13 +3687,17 @@ bool JOIN::choose_subquery_plan(table_map join_tables)
         conditions.
       */
       if (save_query_plan(&save_keyuse, save_best_positions,
-                          save_join_tab_keyuse, save_join_tab_checked_keys) ||
-          reoptimize(in_to_exists_where, join_tables, save_best_positions))
+                          save_join_tab_keyuse, save_join_tab_checked_keys))
+        return TRUE;
+      reopt_result= reoptimize(in_to_exists_where, join_tables);
+      if (reopt_result == REOPT_OLD_PLAN)
+        restore_query_plan(&save_keyuse, save_best_positions,
+                           save_join_tab_keyuse, save_join_tab_checked_keys);
+      else if (reopt_result == REOPT_ERROR)
         return TRUE;
-      in_exists_reoptimized= true;
 
-      get_partial_join_cost(inner_join, inner_join->tables,
-                            &inner_read_time_2, &inner_record_count_2);
+      inner_join->get_partial_join_cost(inner_join->tables,
+                                        &inner_read_time_2, &inner_record_count_2);
     }
     else
     {
@@ -3765,8 +3767,8 @@ bool JOIN::choose_subquery_plan(table_map join_tables)
 
   if (in_subs->in_strategy & SUBS_MATERIALIZATION)
   {
-    /* Restore orginal query plan used for materialization. */
-    if (in_exists_reoptimized)
+    /* Restore the original query plan used for materialization. */
+    if (reopt_result == REOPT_NEW_PLAN)
       restore_query_plan(&save_keyuse, save_best_positions,
                          save_join_tab_keyuse, save_join_tab_checked_keys);
 
@@ -3792,14 +3794,11 @@ bool JOIN::choose_subquery_plan(table_map join_tables)
   }
   else if (in_subs->in_strategy & SUBS_IN_TO_EXISTS)
   {
-    /* Keep the new query plan with injected conditions, delete the old one. */
-    if (save_keyuse.elements)
-    {
-      DBUG_ASSERT(in_exists_reoptimized);
+    /* Keep the new query plan with injected conditions, delete the old plan. */
+    if (reopt_result == REOPT_NEW_PLAN)
       delete_dynamic(&save_keyuse);
-    }
 
-    if (!in_exists_reoptimized && in_to_exists_where && const_tables != tables)
+    if (reopt_result == REOPT_NONE && in_to_exists_where && const_tables != tables)
     {
       /*
         The subquery was not reoptimized either because the user allowed only the
@@ -3811,7 +3810,8 @@ bool JOIN::choose_subquery_plan(table_map join_tables)
         join_tab[i].keyuse= NULL;
         join_tab[i].checked_keys.clear_all();
       }
-      if (reoptimize(in_to_exists_where, join_tables, NULL))
+      if ((reopt_result= reoptimize(in_to_exists_where, join_tables)) ==
+          REOPT_ERROR)
         return TRUE;
     }
 

sql/sql_select.cc

@@ -229,8 +229,6 @@ static bool update_sum_func(Item_sum **func);
 static void select_describe(JOIN *join, bool need_tmp_table,bool need_order,
 			    bool distinct, const char *message=NullS);
 static void add_group_and_distinct_keys(JOIN *join, JOIN_TAB *join_tab);
-void get_partial_join_cost(JOIN *join, uint idx, double *read_time_arg,
-                           double *record_count_arg);
 static uint make_join_orderinfo(JOIN *join);
 static int
 join_read_record_no_init(JOIN_TAB *tab);
@@ -5422,40 +5420,43 @@ greedy_search(JOIN      *join,
 }
 
 
-/*
-  Calculate a cost of given partial join order
+/**
+  Calculate a cost of given partial join order in join->positions.
  
-  SYNOPSIS
-    get_partial_join_cost()
-      join               IN    Join to use. join->positions holds the
-                               partial join order
-      idx                IN    # tables in the partial join order
-      read_time_arg      OUT   Store read time here 
-      record_count_arg   OUT   Store record count here
+  @param n_tables[in]      # tables in the partial join order after the last
+                           constant table
+  @param read_time_arg[out]    store read time here 
+  @param record_count_arg[out] store record count here
 
-  DESCRIPTION
-
-    This is needed for semi-join materialization code. The idea is that 
-    we detect sj-materialization after we've put all sj-inner tables into
-    the join prefix
+  @note
+    When used by semi-join materialization code the idea is that we
+    detect sj-materialization after we've put all sj-inner tables into
+    the join prefix.
 
       prefix-tables semi-join-inner-tables  tN
                                              ^--we're here
 
     and we'll need to get the cost of prefix-tables prefix again.
+
+    When used with non-flattened subqueries, the method computes the
+    total cost of query plan.
+
+  @returns
+    read_time_arg and record_count_arg contain the computed cost.
 */
 
-void get_partial_join_cost(JOIN *join, uint n_tables, double *read_time_arg,
-                           double *record_count_arg)
+void JOIN::get_partial_join_cost(uint n_tables,
+                                 double *read_time_arg, double *record_count_arg)
 {
   double record_count= 1;
   double read_time= 0.0;
-  for (uint i= join->const_tables; i < n_tables + join->const_tables ; i++)
+
+  for (uint i= const_tables; i < n_tables; i++)
   {
-    if (join->best_positions[i].records_read)
+    if (best_positions[i].records_read)
     {
-      record_count *= join->best_positions[i].records_read;
-      read_time += join->best_positions[i].read_time;
+      record_count *= best_positions[i].records_read;
+      read_time += best_positions[i].read_time;
     }
   }
   *read_time_arg= read_time;// + record_count / TIME_FOR_COMPARE;
@@ -19331,12 +19332,12 @@ void JOIN::restore_query_plan(DYNAMIC_ARRAY *save_keyuse,
   4. Re-sort and re-filter the JOIN::keyuse array with the newly added
      KEYUSE elements. 
  
-  @retval 0  OK
-  @retval 1  memory allocation error
+  @retval REOPT_NEW_PLAN  there is a new plan.
+  @retval REOPT_OLD_PLAN  no new improved plan was produced, use the old one.
+  @retval REOPT_ERROR     an irrecovarable error occured during reoptimization.
 */
 
-int JOIN::reoptimize(Item *added_where, table_map join_tables,
-                     POSITION *save_best_positions)
+JOIN::enum_reopt_result JOIN::reoptimize(Item *added_where, table_map join_tables)
 {
   DYNAMIC_ARRAY added_keyuse;
   SARGABLE_PARAM *sargables= 0; /* Used only as a dummy parameter. */
@@ -19346,25 +19347,18 @@ int JOIN::reoptimize(Item *added_where, table_map join_tables,
                           ~outer_join, select_lex, &sargables))
   {
     delete_dynamic(&added_keyuse);
-    return 1;
+    return REOPT_ERROR;
   }
 
   if (!added_keyuse.elements)
-  {
-    /* No need to optimize if no new access methods were discovered. */
-    if (save_best_positions)
-      memcpy((uchar*) best_positions, (uchar*) save_best_positions,
-             sizeof(POSITION) * (tables + 1));
-    delete_dynamic(&added_keyuse);
-    return 0;
-  }
+    return REOPT_OLD_PLAN;
 
   /* Add the new access methods to the keyuse array. */
   if (!keyuse.buffer &&
       my_init_dynamic_array(&keyuse, sizeof(KEYUSE), 20, 64))
   {
     delete_dynamic(&added_keyuse);
-    return 1;
+    return REOPT_ERROR;
   }
   allocate_dynamic(&keyuse, keyuse.elements + added_keyuse.elements);
   memcpy(keyuse.buffer + keyuse.elements * keyuse.size_of_element,
@@ -19374,14 +19368,14 @@ int JOIN::reoptimize(Item *added_where, table_map join_tables,
   delete_dynamic(&added_keyuse);
 
   if (sort_and_filter_keyuse(&keyuse))
-    return 1;
+    return REOPT_ERROR;
   optimize_keyuse(this, &keyuse);
 
   /* Re-run the join optimizer to compute a new query plan. */
   if (choose_plan(this, join_tables))
-    return 1;
+    return REOPT_ERROR;
 
-  return 0;
+  return REOPT_NEW_PLAN;
 }
 
 /**

sql/sql_select.h

@@ -1374,9 +1374,16 @@ private:
   JOIN& operator=(const JOIN &rhs);             /**< not implemented */
 
 protected:
+  /* Results of reoptimizing a JOIN via JOIN::reoptimize(). */
+  enum enum_reopt_result {
+    REOPT_NEW_PLAN, /* there is a new reoptimized plan */
+    REOPT_OLD_PLAN, /* no new improved plan can be found, use the old one */
+    REOPT_ERROR,    /* an irrecovarable error occured during reoptimization */
+    REOPT_NONE      /* not yet reoptimized */
+  };
+
   /* Support for plan reoptimization with rewritten conditions. */
-  int reoptimize(Item *added_where, table_map join_tables,
-                 POSITION *save_best_positions);
+  enum_reopt_result reoptimize(Item *added_where, table_map join_tables);
   int save_query_plan(DYNAMIC_ARRAY *save_keyuse, POSITION *save_positions,
                       KEYUSE **save_join_tab_keyuse,
                       key_map *save_join_tab_checked_keys);
@@ -1762,6 +1769,9 @@ public:
   }
   bool setup_subquery_caches();
   bool choose_subquery_plan(table_map join_tables);
+  void get_partial_join_cost(uint n_tables,
+                             double *read_time_arg, double *record_count_arg);
+
 private:
   /**
     TRUE if the query contains an aggregate function but has no GROUP
@@ -1993,8 +2003,6 @@ inline Item * and_items(Item* cond, Item *item)
   return (cond? (new Item_cond_and(cond, item)) : item);
 }
 bool choose_plan(JOIN *join,table_map join_tables);
-void get_partial_join_cost(JOIN *join, uint n_tables, double *read_time_arg,
-                           double *record_count_arg);
 void optimize_wo_join_buffering(JOIN *join, uint first_tab, uint last_tab, 
                                 table_map last_remaining_tables, 
                                 bool first_alt, uint no_jbuf_before,