List:Commits« Previous MessageNext Message »
From:Ole John Aske Date:December 9 2010 1:51pm
Subject:bzr commit into mysql-next-mr branch (ole.john.aske:3207) Bug#58628
View as plain text  
#At file:///net/fimafeng09/export/home/tmp/oleja/mysql/mysql-next-mr/ based on revid:alexander.nozdrin@stripped

 3207 Ole John Aske	2010-12-09
      Provide next-mr version of fix for 
      bug#58628 'Incorrect result for 'WHERE NULL NOT IN (<subquery>)'
      on request from reviewers.
      
      MTR testcase is *not* included as the structure of the MTR tests seems to have changed
      from 5.1 -> 5.6.x - These should be picked from original commit and manually adopted later .
      
      ---- Original commit comments -------
            
            create_ref_for_key() allowed any constant part of a REF key to be evaluated
            and stored into the 'key_buff' during ::optimize(). These 'store_key*' was
            *not* kept in ref.key_copy[] as they where constant and we assumed we
            would not have to reevaluate them during JOIN::exec()
            
            However, during execute NULL values in REF key has to be detected as they may
            need special attention - as in 'Full scan on NULL key'. This is done by 
            subselect_uniquesubquery_engine::copy_ref_key() which check if any keyparts 
            evaluated to a NULL-value.
            
            As we didn't keep a store_key for a constant value, a NULL-constant was not detected
            by subselect_uniquesubquery_engine::copy_ref_key() !
            
            This fixs modifies create_ref_for_key() to check if a NULL-value constant 
            was produced - In these cases it keeps the store_key, which then will be 
            reevaluated in JOIN::exec() and trigger correct handling of NULL-valued keys.

    modified:
      sql/sql_select.cc
      sql/sql_select.h
=== modified file 'sql/sql_select.cc'
--- a/sql/sql_select.cc	2010-11-05 16:23:32 +0000
+++ b/sql/sql_select.cc	2010-12-09 13:51:16 +0000
@@ -8860,29 +8860,37 @@ static bool create_ref_for_key(JOIN *joi
         j->ref.null_rejecting |= 1 << i;
       keyuse_uses_no_tables= keyuse_uses_no_tables && !keyuse->used_tables;
 
+      store_key* key= get_store_key(thd,
+				    keyuse,join->const_table_map,
+				    &keyinfo->key_part[i],
+				    key_buff, maybe_null);
+      if (unlikely(!key || thd->is_fatal_error))
+        DBUG_RETURN(TRUE);
+
       if (keyuse->used_tables || thd->lex->describe)
         /* 
           Comparing against a non-constant or executing an EXPLAIN
           query (which refers to this info when printing the 'ref'
           column of the query plan)
         */
-        *ref_key++= get_store_key(thd,
-                                  keyuse,join->const_table_map,
-                                  &keyinfo->key_part[i],
-                                  key_buff, maybe_null);
+        *ref_key++= key;
       else
-      { // Compare against constant
-        store_key_item tmp(thd, keyinfo->key_part[i].field,
-                           key_buff + maybe_null,
-                           maybe_null ?  key_buff : 0,
-                           keyinfo->key_part[i].length, keyuse->val);
-        if (thd->is_fatal_error)
-          DBUG_RETURN(TRUE);
-        /* 
-          The constant is the value to look for with this key. Copy
-          the value to ref->key_buff
-        */
-        tmp.copy(); 
+      {
+        /* key is constant, copy value now and possibly skip it while ::exec() */
+        enum store_key::store_key_result result= key->copy();
+
+        /* Depending on 'result' it should be reevaluated in ::exec(), if either:
+         *  1) '::copy()' failed, in case we reevaluate - and refail in 
+         *       JOIN::exec() where the error can be handled.
+         *  2)  Constant evaluated to NULL value which we might need to 
+         *      handle as a special case during JOIN::exec()
+         *      (As in : 'Full scan on NULL key')
+         */
+        if (result!=store_key::STORE_KEY_OK  ||    // 1)
+            key->null_key)                         // 2)
+        {
+	  *ref_key++= key;  // Reevaluate in JOIN::exec() 
+        }
       }
       /*
 	Remember if we are going to use REF_OR_NULL

=== modified file 'sql/sql_select.h'
--- a/sql/sql_select.h	2010-10-27 14:46:44 +0000
+++ b/sql/sql_select.h	2010-12-09 13:51:16 +0000
@@ -2139,9 +2139,8 @@ public:
   store_key_const_item(THD *thd, Field *to_field_arg, uchar *ptr,
 		       uchar *null_ptr_arg, uint length,
 		       Item *item_arg)
-    :store_key_item(thd, to_field_arg,ptr,
-		    null_ptr_arg ? null_ptr_arg : item_arg->maybe_null ?
-		    &err : (uchar*) 0, length, item_arg), inited(0)
+    :store_key_item(thd, to_field_arg, ptr,
+                    null_ptr_arg, length, item_arg), inited(0)
   {
   }
   const char *name() const { return "const"; }
@@ -2149,27 +2148,13 @@ public:
 protected:  
   enum store_key_result copy_inner()
   {
-    int res;
     if (!inited)
     {
       inited=1;
-      TABLE *table= to_field->table;
-      my_bitmap_map *old_map= dbug_tmp_use_all_columns(table,
-                                                       table->write_set);
-      if ((res= item->save_in_field(to_field, 1)))
-      {       
-        if (!err)
-          err= res < 0 ? 1 : res; /* 1=STORE_KEY_FATAL */
-      }
-      /*
-        Item::save_in_field() may call Item::val_xxx(). And if this is a subquery
-        we need to check for errors executing it and react accordingly
-        */
-      if (!err && to_field->table->in_use->is_error())
-        err= 1; /* STORE_KEY_FATAL */
-      dbug_tmp_restore_column_map(table->write_set, old_map);
+      int res= store_key_item::copy_inner();
+      if (res && !err)
+        err= res;
     }
-    null_key= to_field->is_null() || item->null_value;
     return (err > 2 ? STORE_KEY_FATAL : (store_key_result) err);
   }
 };


Attachment: [text/bzr-bundle] bzr/ole.john.aske@oracle.com-20101209135116-s87j7if2szb7fipk.bundle
Thread
bzr commit into mysql-next-mr branch (ole.john.aske:3207) Bug#58628Ole John Aske9 Dec