List:Commits« Previous MessageNext Message »
From:Guilhem Bichot Date:January 31 2011 9:51pm
Subject:bzr commit into mysql-trunk branch (guilhem.bichot:3335) Bug#59852
View as plain text  
#At file:///home/mysql_src/bzrrepos_new/59852/ based on revid:roy.lyseng@stripped

 3335 Guilhem Bichot	2011-01-31
      Fix for BUG#59852 "EXPLAIN EXTENDED + UNION + 'ORDER BY x IN (subquery)' does not show IN->EXISTS transformation":
      make EXPLAIN show it, and fix the resulting crash by getting rid of a redundant, bad THD pointer.
     @ mysql-test/r/union.result
        result. Without the change to sql_select.cc, the last EXPLAIN would not show
        the IN->EXISTS transformation (would just show the original query).
     @ mysql-test/t/union.test
        test for bug
     @ sql/item_subselect.cc
        Item_subselect::thd and subselect_engine::thd are deleted.
        The first is replaced with the existing Item_subselect::unit::thd.
        The second one is replaced with the existing subselect_engine::item::unit::thd
        (same object as for the first one, actually: item points to the containing
        Item_subselect); for subselect_union_engine it's additionally possible to use
        subselect_union_engine::unit::thd.
        
        Before this change, Item_subselect::thd and subselect_engine::thd
        were set to NULL in constructors, and Item_subselect::fix_fields()
        corrected them. As Item_subselect::thd is used in subquery
        transformations (in Item_singlerow_subselect::select_transformer()
        called by JOIN::prepare()), it was necessary that
        Item_subselect::fix_fields() was called before JOIN::prepare()
        for the subquery's JOIN. And often it works indeed this way,
        as Item_subselect::fix_fields() calls JOIN::prepare().
        
        But in
          "EXPLAIN select1 UNION select2 ORDER BY select3"
        (case of BUG 49734), we have four SELECT_LEX:
        select1-2-3 and the "fake UNION SELECT".
        EXPLAIN EXTENDED (mysql_explain_union()) calls JOIN::prepare()
        for select1, and select2.
        select3 is not in the ORDER BY clause of select2, so
        JOIN::prepare(select2) does not call Item_subselect::fix_fields().
        Then mysql_explain_union() calls JOIN::exec(select2) which itself
        calls select_describe(select2). That function calls
        mysql_explain_union() on "inner units" of select2, and select3 is
        among them. That is weird, by the way: select3 is not in ORDER BY of
        select2 (ok) but it's a inner unit of select2 (weird); is this a
        parsing artifact? Shouldn't it be a inner unit of the fake SELECT?
        So we go into JOIN::prepare(select3), which calls resolve_subquery()
        to transform, and crashes on a still-NULL Item_subselect::thd pointer.
        If the crash is "leaped over" in gdb, then finally
        JOIN::prepare() is called for the fake UNION SELECT, which owns the ORDER BY
        clause, so it's only at this late moment, precisely in setup_order(), that
        Item_subselect::fix_fields() is called which sets Item_subselect::thd
        correctly. Too late.
        See also the comment at start of st_select_lex::print()
        about thd being NULL.
        The advantage of using SELECT_LEX_UNIT::thd pointers is that they are
        always kept correct by reinit_stmt_before_use(). The NULL thd pointer
        is eliminated, we now use a correct one, which avoids the crash.
        One may wonder whether, even with no crash thanks to this patch, it
        is still a logical problem to have:
        - JOIN::prepare() called before Item_subselect::fix_fields().
        - select3 attached to select2 as inner unit but attached to fake
          SELECT as ORDER BY clause.
        Those oddities are a specificity of EXPLAIN UNION; without a UNION, in
          "EXPLAIN select 1 ORDER BY select3",
        select1 owns the ORDER BY clause, so setup_order() is called
        from JOIN::prepare(select1), which calls Item_subselect::fix_fields(),
        which calls JOIN::prepare() (JOIN of select3).
        It would be interesting to discuss this with reviewers.
        
        We also add assertions that various pointers-to-THD are equal.
     @ sql/sql_select.cc
        don't skip subquery transformations in EXPLAIN, so that the user
        is informed about them. This was skipped by the second part of
        the patch for BUG 49734, but it's not needed anymore, because we
        here fix the bad-THD cause in item_subselect.cc.

    modified:
      mysql-test/r/union.result
      mysql-test/t/union.test
      sql/item_subselect.cc
      sql/item_subselect.h
      sql/sql_select.cc
=== modified file 'mysql-test/r/union.result'
--- a/mysql-test/r/union.result	2011-01-10 12:45:53 +0000
+++ b/mysql-test/r/union.result	2011-01-31 21:51:42 +0000
@@ -1769,3 +1769,28 @@ dev
 SELECT(SELECT 1 AS a FROM dual ORDER BY a DESC LIMIT 1) AS dev;
 dev
 1
+#
+# Bug #59852: EXPLAIN EXTENDED UNION ... ORDER BY ... IN (subquery)
+# does not show IN->EXISTS transformation
+# 
+CREATE TABLE t1 (a VARCHAR(10), FULLTEXT KEY a (a));
+INSERT INTO t1 VALUES (1),(2);
+CREATE TABLE t2 (b INT);
+INSERT INTO t2 VALUES (1),(2);
+EXPLAIN EXTENDED SELECT * FROM t1 ORDER BY a in (SELECT a FROM t2 WHERE b = 1);
+id	select_type	table	type	possible_keys	key	key_len	ref	rows	filtered	Extra
+1	PRIMARY	t1	ALL	NULL	NULL	NULL	NULL	2	100.00	Using filesort
+2	DEPENDENT SUBQUERY	t2	ALL	NULL	NULL	NULL	NULL	2	100.00	Using where
+Warnings:
+Note	1276	Field or reference 'test.t1.a' of SELECT #2 was resolved in SELECT #1
+Note	1003	select `test`.`t1`.`a` AS `a` from `test`.`t1` order by <in_optimizer>(`test`.`t1`.`a`,<exists>(select 1 from `test`.`t2` where ((`test`.`t2`.`b` = 1) and trigcond(((<cache>(`test`.`t1`.`a`) = `test`.`t1`.`a`) or isnull(`test`.`t1`.`a`)))) having trigcond(<is_not_null_test>(`test`.`t1`.`a`))))
+EXPLAIN EXTENDED SELECT * FROM t1 UNION SELECT * FROM t1 ORDER BY a in (SELECT a FROM t2 WHERE b = 1);
+id	select_type	table	type	possible_keys	key	key_len	ref	rows	filtered	Extra
+1	PRIMARY	t1	ALL	NULL	NULL	NULL	NULL	2	100.00	
+2	UNION	t1	ALL	NULL	NULL	NULL	NULL	2	100.00	
+3	SUBQUERY	t2	ALL	NULL	NULL	NULL	NULL	2	100.00	Using where
+NULL	UNION RESULT	<union1,2>	ALL	NULL	NULL	NULL	NULL	NULL	NULL	Using filesort
+Warnings:
+Note	1276	Field or reference 'test.t1.a' of SELECT #3 was resolved in SELECT #2
+Note	1003	select `test`.`t1`.`a` AS `a` from `test`.`t1` union select `test`.`t1`.`a` AS `a` from `test`.`t1` order by <in_optimizer>(`a`,<exists>(select 1 from `test`.`t2` where ((`test`.`t2`.`b` = 1) and trigcond(((<cache>(`a`) = `test`.`t1`.`a`) or isnull(`test`.`t1`.`a`)))) having (trigcond(<is_not_null_test>(`test`.`t1`.`a`)) and trigcond((<cache>(`a`) = <ref_null_helper>(1))))))
+DROP TABLE t1,t2;

=== modified file 'mysql-test/t/union.test'
--- a/mysql-test/t/union.test	2011-01-10 12:45:53 +0000
+++ b/mysql-test/t/union.test	2011-01-31 21:51:42 +0000
@@ -1185,3 +1185,16 @@ SELECT(SELECT 0 AS a FROM dual UNION SEL
 SELECT(SELECT 1 AS a ORDER BY a) AS dev;
 SELECT(SELECT 1 AS a LIMIT 1) AS dev;
 SELECT(SELECT 1 AS a FROM dual ORDER BY a DESC LIMIT 1) AS dev;
+
+--echo #
+--echo # Bug #59852: EXPLAIN EXTENDED UNION ... ORDER BY ... IN (subquery)
+--echo # does not show IN->EXISTS transformation
+--echo # 
+
+CREATE TABLE t1 (a VARCHAR(10), FULLTEXT KEY a (a));
+INSERT INTO t1 VALUES (1),(2);
+CREATE TABLE t2 (b INT);
+INSERT INTO t2 VALUES (1),(2);
+EXPLAIN EXTENDED SELECT * FROM t1 ORDER BY a in (SELECT a FROM t2 WHERE b = 1);
+EXPLAIN EXTENDED SELECT * FROM t1 UNION SELECT * FROM t1 ORDER BY a in (SELECT a FROM t2 WHERE b = 1);
+DROP TABLE t1,t2;

=== modified file 'sql/item_subselect.cc'
--- a/sql/item_subselect.cc	2011-01-18 11:42:09 +0000
+++ b/sql/item_subselect.cc	2011-01-31 21:51:42 +0000
@@ -45,7 +45,7 @@ inline Item * and_items(Item* cond, Item
 }
 
 Item_subselect::Item_subselect():
-  Item_result_field(), value_assigned(0), thd(0), substitution(0),
+  Item_result_field(), value_assigned(0), substitution(0),
   engine(0), old_engine(0), used_tables_cache(0), have_to_be_excluded(0),
   const_item_cache(1), engine_changed(0), changed(0),
   is_correlated(FALSE)
@@ -169,14 +169,25 @@ Item_subselect::select_transformer(JOIN 
 }
 
 
-bool Item_subselect::fix_fields(THD *thd_param, Item **ref)
+bool Item_subselect::fix_fields(THD *thd, Item **ref)
 {
-  char const *save_where= thd_param->where;
+  char const *save_where= thd->where;
   uint8 uncacheable;
   bool res;
 
   DBUG_ASSERT(fixed == 0);
-  engine->set_thd((thd= thd_param));
+  /*
+    Pointers to THD must match. unit::thd may vary over the lifetime of the
+    item (for example triggers, and thus their Item-s, are in a cache shared
+    by all connections), but reinit_stmt_before_use() keeps it up-to-date,
+    which we check here. subselect_union_engine functions also do sanity
+    checks.
+  */
+  DBUG_ASSERT(thd == unit->thd);
+  // Engine accesses THD via its 'item' pointer, check it:
+  engine->assert_item(this);
+
+  engine->set_thd_for_result();
 
   if (check_stack_overrun(thd, STACK_MIN_SIZE, (uchar*)&res))
     return TRUE;
@@ -285,6 +296,7 @@ bool Item_subselect::exec()
     Do not execute subselect in case of a fatal error
     or if the query has been killed.
   */
+  THD * const thd= unit->thd;
   if (thd->is_error() || thd->killed)
     return 1;
 
@@ -436,7 +448,7 @@ table_map Item_subselect::used_tables() 
 
 bool Item_subselect::const_item() const
 {
-  return thd->lex->context_analysis_only ? FALSE : const_item_cache;
+  return unit->thd->lex->context_analysis_only ? FALSE : const_item_cache;
 }
 
 Item *Item_subselect::get_tmp_table_item(THD *thd_arg)
@@ -520,12 +532,6 @@ Item_maxmin_subselect::Item_maxmin_subse
   used_tables_cache= parent->get_used_tables_cache();
   const_item_cache= parent->get_const_item_cache();
 
-  /*
-    this subquery always creates during preparation, so we can assign
-    thd here
-  */
-  thd= thd_param;
-
   DBUG_VOID_RETURN;
 }
 
@@ -580,6 +586,7 @@ Item_singlerow_subselect::select_transfo
     DBUG_RETURN(RES_OK);
 
   SELECT_LEX *select_lex= join->select_lex;
+  THD * const thd= unit->thd;
   Query_arena *arena= thd->stmt_arena;
  
   if (!select_lex->master_unit()->is_union() &&
@@ -1090,6 +1097,8 @@ Item_in_subselect::single_value_transfor
     DBUG_RETURN(RES_ERROR);
   }
 
+  THD * const thd= unit->thd;
+
   /*
     If this is an ALL/ANY single-value subselect, try to rewrite it with
     a MIN/MAX subselect. We can do that if a possible NULL result of the
@@ -1263,6 +1272,7 @@ Item_subselect::trans_res
 Item_in_subselect::single_value_in_to_exists_transformer(JOIN * join, Comp_creator *func)
 {
   SELECT_LEX *select_lex= join->select_lex;
+  THD * const thd= unit->thd;
   DBUG_ENTER("Item_in_subselect::single_value_in_to_exists_transformer");
 
   select_lex->uncacheable|= UNCACHEABLE_DEPENDENT;
@@ -1466,6 +1476,7 @@ Item_in_subselect::row_value_transformer
     SELECT_LEX_UNIT *master_unit= select_lex->master_unit();
     substitution= optimizer;
 
+    THD * const thd= unit->thd;
     SELECT_LEX *current= thd->lex->current_select;
     thd->lex->current_select= current->outer_select();
     //optimizer never use Item **ref => we can pass 0 as parameter
@@ -1525,6 +1536,7 @@ Item_subselect::trans_res
 Item_in_subselect::row_value_in_to_exists_transformer(JOIN * join)
 {
   SELECT_LEX *select_lex= join->select_lex;
+  THD * const thd= unit->thd;
   Item *having_item= 0;
   uint cols_num= left_expr->cols();
   bool is_having_used= (join->having || select_lex->with_sum_func ||
@@ -1755,6 +1767,7 @@ Item_subselect::trans_res
 Item_in_subselect::select_in_like_transformer(JOIN *join, Comp_creator *func)
 {
   Query_arena *arena, backup;
+  THD * const thd= unit->thd;
   SELECT_LEX *current= thd->lex->current_select;
   const char *save_where= thd->where;
   Item_subselect::trans_res res= RES_ERROR;
@@ -1912,6 +1925,7 @@ bool Item_in_subselect::setup_engine()
   if (engine->engine_type() == subselect_engine::SINGLE_SELECT_ENGINE)
   {
     /* Create/initialize objects in permanent memory. */
+    THD * const thd= unit->thd;
     Query_arena *arena= thd->stmt_arena;
     Query_arena backup;
     if (arena->is_conventional())
@@ -2008,7 +2022,7 @@ bool Item_in_subselect::init_left_expr_c
 
   for (uint i= 0; i < left_expr->cols(); i++)
   {
-    Cached_item *cur_item_cache= new_Cached_item(thd,
+    Cached_item *cur_item_cache= new_Cached_item(unit->thd,
                                                  left_expr->element_index(i),
                                                  use_result_field);
     if (!cur_item_cache || left_expr_cache->push_front(cur_item_cache))
@@ -2064,11 +2078,14 @@ void Item_allany_subselect::print(String
 }
 
 
-void subselect_engine::set_thd(THD *thd_arg)
+void subselect_engine::set_thd_for_result()
 {
-  thd= thd_arg;
+  /*
+    select_result's constructor sets neither select_result::thd nor
+    select_result::unit.
+  */
   if (result)
-    result->set_thd(thd_arg);
+    result->set_thd(item->unit->thd);
 }
 
 
@@ -2180,6 +2197,7 @@ int subselect_single_select_engine::prep
 {
   if (prepared)
     return 0;
+  THD * const thd= item->unit->thd;
   join= new JOIN(thd, select_lex->item_list,
 		 select_lex->options | SELECT_NO_UNLOCK, result);
   if (!join || !result)
@@ -2205,6 +2223,9 @@ int subselect_single_select_engine::prep
 
 int subselect_union_engine::prepare()
 {
+  THD * const thd= unit->thd;
+  // We can access THD as above, or via 'item', verify equality:
+  DBUG_ASSERT(thd == item->unit->thd);
   return unit->prepare(thd, result, SELECT_NO_UNLOCK);
 }
 
@@ -2305,6 +2326,7 @@ int subselect_single_select_engine::exec
 {
   DBUG_ENTER("subselect_single_select_engine::exec");
   int rc= 0;
+  THD * const thd= item->unit->thd;
   char const *save_where= thd->where;
   SELECT_LEX *save_select= thd->lex->current_select;
   thd->lex->current_select= select_lex;
@@ -2406,6 +2428,8 @@ exit:
 
 int subselect_union_engine::exec()
 {
+  THD * const thd= unit->thd;
+  DBUG_ASSERT(thd == item->unit->thd);
   char const *save_where= thd->where;
   int res= unit->exec();
   thd->where= save_where;
@@ -2864,7 +2888,7 @@ subselect_single_select_engine::save_joi
         plan to save will not be replaced anyway.
      4) The Item_subselect is cacheable
   */
-  if (thd->lex->describe &&                              // 1
+  if (item->unit->thd->lex->describe &&                  // 1
       !select_lex->uncacheable &&                        // 2
       !(join->select_options & SELECT_DESCRIBE))         // 3
   {
@@ -2907,7 +2931,7 @@ table_map subselect_union_engine::upper_
 void subselect_single_select_engine::print(String *str,
                                            enum_query_type query_type)
 {
-  select_lex->print(thd, str, query_type);
+  select_lex->print(item->unit->thd, str, query_type);
 }
 
 
@@ -3169,6 +3193,7 @@ bool subselect_hash_sj_engine::init_perm
   */
   if (!(tmp_result_sink= new select_union))
     DBUG_RETURN(TRUE);
+  THD * const thd= item->unit->thd;
   if (tmp_result_sink->create_result_table(
                          thd, tmp_columns, TRUE,
                          thd->variables.option_bits | TMP_TABLE_ALL_COLUMNS,
@@ -3323,7 +3348,7 @@ bool subselect_hash_sj_engine::init_runt
     Repeat name resolution for 'cond' since cond is not part of any
     clause of the query, and it is not 'fixed' during JOIN::prepare.
   */
-  if (cond && !cond->fixed && cond->fix_fields(thd, &cond))
+  if (cond && !cond->fixed && cond->fix_fields(item->unit->thd, &cond))
     return TRUE;
   /* Let our engine reuse this query plan for materialization. */
   materialize_join= materialize_engine->join;
@@ -3336,7 +3361,7 @@ subselect_hash_sj_engine::~subselect_has
 {
   delete result;
   if (tab)
-    free_tmp_table(thd, tab->table);
+    free_tmp_table(item->unit->thd, tab->table);
 }
 
 
@@ -3380,6 +3405,7 @@ int subselect_hash_sj_engine::exec()
   if (!is_materialized)
   {
     int res= 0;
+    THD * const thd= item->unit->thd;
     SELECT_LEX *save_select= thd->lex->current_select;
     thd->lex->current_select= materialize_engine->select_lex;
     if ((res= materialize_join->optimize()))

=== modified file 'sql/item_subselect.h'
--- a/sql/item_subselect.h	2011-01-24 14:17:03 +0000
+++ b/sql/item_subselect.h	2011-01-31 21:51:42 +0000
@@ -46,8 +46,6 @@ class Item_subselect :public Item_result
 {
   my_bool value_assigned; /* value already assigned to subselect */
 public:
-  /* thread handler, will be assigned in fix_fields only */
-  THD *thd;
   /* 
     Used inside Item_subselect::fix_fields() according to this scenario:
       > Item_subselect::fix_fields
@@ -443,7 +441,6 @@ class subselect_engine: public Sql_alloc
 {
 protected:
   select_result_interceptor *result; /* results storage class */
-  THD *thd; /* pointer to current THD */
   Item_subselect *item; /* item, that use this engine */
   enum Item_result res_type; /* type of results */
   enum_field_types res_field_type; /* column type of the results */
@@ -455,7 +452,6 @@ public:
                          INDEXSUBQUERY_ENGINE, HASH_SJ_ENGINE};
 
   subselect_engine(Item_subselect *si, select_result_interceptor *res)
-    :thd(0)
   {
     result= res;
     item= si;
@@ -466,12 +462,8 @@ public:
   virtual ~subselect_engine() {}; // to satisfy compiler
   virtual void cleanup()= 0;
 
-  /*
-    Also sets "thd" for subselect_engine::result.
-    Should be called before prepare().
-  */
-  void set_thd(THD *thd_arg);
-  THD * get_thd() { return thd; }
+  /// Sets "thd" for 'result'. Should be called before prepare()
+  void set_thd_for_result();
   virtual int prepare()= 0;
   virtual void fix_length_and_dec(Item_cache** row)= 0;
   /*
@@ -511,6 +503,8 @@ public:
   /* Check if subquery produced any rows during last query execution */
   virtual bool no_rows() = 0;
   virtual enum_engine_type engine_type() { return ABSTRACT_ENGINE; }
+  /// Verifies that the internal item matches the argument
+  void assert_item(const Item_subselect *i) const { DBUG_ASSERT(i == item); }
 
 protected:
   void set_row(List<Item> &item_list, Item_cache **row);
@@ -610,10 +604,7 @@ public:
   // constructor can assign THD because it will be called after JOIN::prepare
   subselect_uniquesubquery_engine(THD *thd_arg, st_join_table *tab_arg,
 				  Item_subselect *subs, Item *where)
-    :subselect_engine(subs, 0), tab(tab_arg), cond(where)
-  {
-    set_thd(thd_arg);
-  }
+    :subselect_engine(subs, 0), tab(tab_arg), cond(where) {}
   void cleanup();
   int prepare();
   void fix_length_and_dec(Item_cache** row);

=== modified file 'sql/sql_select.cc'
--- a/sql/sql_select.cc	2011-01-27 11:38:22 +0000
+++ b/sql/sql_select.cc	2011-01-31 21:51:42 +0000
@@ -596,8 +596,7 @@ JOIN::prepare(Item ***rref_pointer_array
   
   if (select_lex->master_unit()->item &&    // This is a subquery
                                             // Not normalizing a view
-      !(thd->lex->context_analysis_only & CONTEXT_ANALYSIS_ONLY_VIEW) &&
-      !(select_options & SELECT_DESCRIBE))  // Not within a describe
+      !(thd->lex->context_analysis_only & CONTEXT_ANALYSIS_ONLY_VIEW))
   {
     /* Join object is a subquery within an IN/ANY/ALL/EXISTS predicate */
     if (resolve_subquery(thd, this))


Attachment: [text/bzr-bundle] bzr/guilhem.bichot@oracle.com-20110131215142-vxnoyfe8ibj24zsw.bundle
Thread
bzr commit into mysql-trunk branch (guilhem.bichot:3335) Bug#59852Guilhem Bichot31 Jan
  • Re: bzr commit into mysql-trunk branch (guilhem.bichot:3335) Bug#59852Roy Lyseng9 Feb
    • Re: bzr commit into mysql-trunk branch (guilhem.bichot:3335) Bug#59852Guilhem Bichot19 Mar
      • Re: bzr commit into mysql-trunk branch (guilhem.bichot:3335) Bug#59852Roy Lyseng21 Mar
        • Re: bzr commit into mysql-trunk branch (guilhem.bichot:3335) Bug#59852Guilhem Bichot21 Mar
          • Re: bzr commit into mysql-trunk branch (guilhem.bichot:3335) Bug#59852Roy Lyseng22 Mar