List:Commits« Previous MessageNext Message »
From:Jorgen Loland Date:January 12 2012 10:21am
Subject:bzr push into mysql-trunk branch (jorgen.loland:3515 to 3516)
View as plain text  
 3516 Jorgen Loland	2012-01-12
      Small refactoring of create_duplicate_weedout_tmp_table():
      Simplify logic and remove dead code
     @ sql/sql_tmp_table.cc
        Small refactoring of create_duplicate_weedout_tmp_table():
        Simplify logic and remove dead code

    modified:
      sql/sql_tmp_table.cc
 3515 Roy Lyseng	2012-01-11
      Bug#13541406: Wrong result with loosescan on select .. where .. in 
      
      To explain this problem, let's look at the query in the attached test:
      
      SELECT ot1.col_int_key AS field1
      FROM t2 AS ot1, t2 AS ot2
      WHERE (ot1.col_varchar_key, ot2.col_varchar_nokey) IN (
          SELECT it2.col_varchar_nokey, it1.col_varchar_key
          FROM t2 AS it1 JOIN t1 AS it2 ON it2.col_int_key = it1.pk);
      
      The selected plan for this query is (abbreviated):
       ot2  ALL
       it1  index                      col_varchar_key  ... LooseScan
       it2  ref(it1.pk)                col_int_key      FirstMatch(it1)
       ot1  ref(it2.col_varchar_nokey) col_varchar_key  Using index
      
      In English, this means:
      - The outermost loop is over the outer table ot2.
      - For each row of ot2, we perform a loose scan over it1.
        (Notice that we could have used a ref access using index
         col_varchar_key, but loosescan and ref access cannot be combined yet).
      - We use FirstMatch on table it2 to weed out duplicate rows from the
        subquery.
      - We use a ref access to look up rows in ot1, based on values from it2.
      
      However, this means that there is a dependency from it2 to ot1 and
      FirstMatch is only able to handle dependencies to outer tables that
      are earlier in the join prefix. Hence, LooseScan (combined with FirstMatch)
      is not a valid strategy for this query.
      
      The source of the problem is that get_bound_sj_equalities() sets the
      zeroth bit in bound_sj_equalities, even though the Loose_scan_opt
      function add_keyuse() performs a complete analysis of bound_sj_equalities
      and handled_sj_equalities later. Setting the zeroth bit has fatal
      consequences in few cases, because it is later overwritten or ignored,
      but it did have consequences in this case.
      
      Thus, the problem is solved by removing get_bound_sj_equalities() and
      initializing bound_sj_equalities to the empty set. As for the question
      on equality propagation inside the function, I do not think that is a
      problem because separate keyuse objects are set up for individual
      fields of a multiple equality.
      
      mysql-test/include/subquery_sj.inc
        Added test case for bug#13541406.
        Make sure that LooseScan is not selected.
      
      mysql-test/r/subquery_sj_all.result
      mysql-test/r/subquery_sj_all_bka.result
      mysql-test/r/subquery_sj_all_bka_nixbnl.result
      mysql-test/r/subquery_sj_all_bkaunique.result
      mysql-test/r/subquery_sj_dupsweed.result
      mysql-test/r/subquery_sj_dupsweed_bka.result
      mysql-test/r/subquery_sj_dupsweed_bka_nixbnl.result
      mysql-test/r/subquery_sj_dupsweed_bkaunique.result
      mysql-test/r/subquery_sj_firstmatch.result
      mysql-test/r/subquery_sj_firstmatch_bka.result
      mysql-test/r/subquery_sj_firstmatch_bka_nixbnl.result
      mysql-test/r/subquery_sj_firstmatch_bkaunique.result
      mysql-test/r/subquery_sj_loosescan.result
      mysql-test/r/subquery_sj_loosescan_bka.result
      mysql-test/r/subquery_sj_loosescan_bka_nixbnl.result
      mysql-test/r/subquery_sj_loosescan_bkaunique.result
      mysql-test/r/subquery_sj_mat.result
      mysql-test/r/subquery_sj_mat_bka.result
      mysql-test/r/subquery_sj_mat_bka_nixbnl.result
      mysql-test/r/subquery_sj_mat_bkaunique.result
      mysql-test/r/subquery_sj_mat_nosj.result
      mysql-test/r/subquery_sj_none.result
      mysql-test/r/subquery_sj_none_bka.result
      mysql-test/r/subquery_sj_none_bka_nixbnl.result
      mysql-test/r/subquery_sj_none_bkaunique.result
        Added test case results for bug#13541406.
      
      sql/sql_planner.cc
        Removed the function get_bound_sj_equalities() and initialized
        bound_sj_equalities to an empty set.
        Comment updates.
      
      sql/sql_select.h
        Comment updates in allowed table patterns for semi-join strategies.
      
      sql/sql_select.h
        Fixed a minor documentation inconsistency.

    modified:
      mysql-test/include/subquery_sj.inc
      mysql-test/r/subquery_sj_all.result
      mysql-test/r/subquery_sj_all_bka.result
      mysql-test/r/subquery_sj_all_bka_nixbnl.result
      mysql-test/r/subquery_sj_all_bkaunique.result
      mysql-test/r/subquery_sj_dupsweed.result
      mysql-test/r/subquery_sj_dupsweed_bka.result
      mysql-test/r/subquery_sj_dupsweed_bka_nixbnl.result
      mysql-test/r/subquery_sj_dupsweed_bkaunique.result
      mysql-test/r/subquery_sj_firstmatch.result
      mysql-test/r/subquery_sj_firstmatch_bka.result
      mysql-test/r/subquery_sj_firstmatch_bka_nixbnl.result
      mysql-test/r/subquery_sj_firstmatch_bkaunique.result
      mysql-test/r/subquery_sj_loosescan.result
      mysql-test/r/subquery_sj_loosescan_bka.result
      mysql-test/r/subquery_sj_loosescan_bka_nixbnl.result
      mysql-test/r/subquery_sj_loosescan_bkaunique.result
      mysql-test/r/subquery_sj_mat.result
      mysql-test/r/subquery_sj_mat_bka.result
      mysql-test/r/subquery_sj_mat_bka_nixbnl.result
      mysql-test/r/subquery_sj_mat_bkaunique.result
      mysql-test/r/subquery_sj_mat_nosj.result
      mysql-test/r/subquery_sj_none.result
      mysql-test/r/subquery_sj_none_bka.result
      mysql-test/r/subquery_sj_none_bka_nixbnl.result
      mysql-test/r/subquery_sj_none_bkaunique.result
      sql/sql_planner.cc
      sql/sql_select.cc
      sql/sql_select.h
=== modified file 'sql/sql_tmp_table.cc'
--- a/sql/sql_tmp_table.cc	2012-01-04 09:51:39 +0000
+++ b/sql/sql_tmp_table.cc	2012-01-12 10:21:25 +0000
@@ -1194,12 +1194,11 @@ TABLE *create_duplicate_weedout_tmp_tabl
   uchar *bitmaps;
   uint *blob_field;
   MI_COLUMNDEF *recinfo, *start_recinfo;
-  bool using_unique_constraint=FALSE;
-  bool use_packed_rows= FALSE;
+  bool using_unique_constraint=false;
   Field *field, *key_field;
   uint null_pack_length, null_count;
   uchar *null_flags;
-  uchar *pos;
+
   DBUG_ENTER("create_duplicate_weedout_tmp_table");
   DBUG_ASSERT(!sjtbl->is_confluent);
   /*
@@ -1222,7 +1221,7 @@ TABLE *create_duplicate_weedout_tmp_tabl
 
   /* STEP 2: Figure if we'll be using a key or blob+constraint */
   if (uniq_tuple_length_arg >= CONVERT_IF_BIGGER_TO_BLOB)
-    using_unique_constraint= TRUE;
+    using_unique_constraint= true;
 
   /* STEP 3: Allocate memory for temptable description */
   init_sql_alloc(&own_root, TABLE_ALLOC_BLOCK_SIZE, 0);
@@ -1341,12 +1340,12 @@ TABLE *create_duplicate_weedout_tmp_tabl
 
   recinfo= start_recinfo;
   null_flags=(uchar*) table->record[0];
-  pos=table->record[0]+ null_pack_length;
-  if (null_pack_length)
+
   {
+    /* Table description for the NULL bits */
     memset(recinfo, 0, sizeof(*recinfo));
-    recinfo->type=FIELD_NORMAL;
-    recinfo->length=null_pack_length;
+    recinfo->type= FIELD_NORMAL;
+    recinfo->length= null_pack_length;
     recinfo++;
     memset(null_flags, 255, null_pack_length);	// Set null fields
 
@@ -1357,39 +1356,21 @@ TABLE *create_duplicate_weedout_tmp_tabl
   null_count=1;
 
   {
-    //Field *field= *reg_field;
-    uint length;
+    /* Table description for the concatenated rowid column */
     memset(recinfo, 0, sizeof(*recinfo));
-    field->move_field(pos,(uchar*) 0,0);
-
-    field->reset();
-    /*
-      Test if there is a default field value. The test for ->ptr is to skip
-      'offset' fields generated by initalize_tables
+    /* 
+       Don't care about packing the VARCHAR since it's only a
+       concatenation of rowids. @see create_tmp_table() for how
+       packed VARCHARs can be achieved
     */
-    // Initialize the table field:
-    memset(field->ptr, 0, field->pack_length());
-
-    length=field->pack_length();
-    pos+= length;
-
-    /* Make entry for create table */
-    recinfo->length=length;
-    if (field->flags & BLOB_FLAG)
-      recinfo->type= (int) FIELD_BLOB;
-    else if (use_packed_rows &&
-             field->real_type() == MYSQL_TYPE_STRING &&
-	     length >= MIN_STRING_LENGTH_TO_PACK_ROWS)
-      recinfo->type=FIELD_SKIP_ENDSPACE;
-    else
-      recinfo->type=FIELD_NORMAL;
+    recinfo->type= FIELD_NORMAL;
+    recinfo->length= field->pack_length();
 
+    field->move_field(table->record[0] + null_pack_length, 0, 0);
+    field->reset();
     field->table_name= &table->alias;
   }
 
-  //param->recinfo=recinfo;
-  //store_record(table,s->default_values);        // Make empty default record
-
   if (thd->variables.tmp_table_size == ~ (ulonglong) 0)		// No limit
     share->max_rows= ~(ha_rows) 0;
   else
@@ -1401,7 +1382,6 @@ TABLE *create_duplicate_weedout_tmp_tabl
   set_if_bigger(share->max_rows,1);		// For dummy start options
 
 
-  //// keyinfo= param->keyinfo;
   if (TRUE)
   {
     DBUG_PRINT("info",("Creating group key in temporary table"));

No bundle (reason: useless for push emails).
Thread
bzr push into mysql-trunk branch (jorgen.loland:3515 to 3516) Jorgen Loland12 Jan