Better bug fix for #14400 "Query joins wrong rows from table which is subject of "concurrent insert""

The previous bug fix didn't work when using partial keys.
Don't use GNUC min/max operations are they are depricated.
Fixed valgrind warning

.bzrignore

@@ -549,3 +549,7 @@ support-files/my-innodb-heavy-4G.cnf
 ac_available_languages_fragment
 support-files/MacOSX/postflight
 support-files/MacOSX/preflight
+*/.deps
+*.Po
+*.Plo
+*/.libs/*

include/my_global.h

@@ -348,10 +348,7 @@ int	__void__;
 #endif
 
 /* Define some useful general macros */
-#if defined(__cplusplus) && defined(__GNUC__)
-#define max(a, b)	((a) >? (b))
-#define min(a, b)	((a) <? (b))
-#elif !defined(max)
+#if !defined(max)
 #define max(a, b)	((a) > (b) ? (a) : (b))
 #define min(a, b)	((a) < (b) ? (a) : (b))
 #endif

myisam/mi_rkey.c

@@ -78,32 +78,39 @@ int mi_rkey(MI_INFO *info, byte *buf, int inx, const byte *key, uint key_len,
   if (!_mi_search(info,keyinfo, key_buff, use_key_length,
 		  myisam_read_vec[search_flag], info->s->state.key_root[inx]))
   {
-    /*
-      If we are searching for an exact key (including the data pointer)
-      and this was added by an concurrent insert,
-      then the result is "key not found".
-    */
-    if ((search_flag == HA_READ_KEY_EXACT) &&
-        (info->lastpos >= info->state->data_file_length))
+    if (info->lastpos >= info->state->data_file_length)
     {
-      my_errno= HA_ERR_KEY_NOT_FOUND;
-      info->lastpos= HA_OFFSET_ERROR;
-    }
-    else while (info->lastpos >= info->state->data_file_length)
-    {
-      /*
-	Skip rows that are inserted by other threads since we got a lock
-	Note that this can only happen if we are not searching after an
-	exact key, because the keys are sorted according to position
-      */
-      if  (_mi_search_next(info, keyinfo, info->lastkey,
-			   info->lastkey_length,
-			   myisam_readnext_vec[search_flag],
-			   info->s->state.key_root[inx]))
-	break;
+      do
+      {
+        uint not_used;
+        /*
+          If we are searching for an exact key, abort if we find a bigger
+          key.
+        */
+        if (search_flag == HA_READ_KEY_EXACT &&
+            (use_key_length == USE_WHOLE_KEY ||
+             _mi_key_cmp(keyinfo->seg, key_buff, info->lastkey, use_key_length,
+                         SEARCH_FIND, &not_used)))
+        {
+          my_errno= HA_ERR_END_OF_FILE;
+          info->lastpos= HA_OFFSET_ERROR;
+          break;
+        }
+        /*
+          Skip rows that are inserted by other threads since we got a lock
+          Note that this can only happen if we are not searching after an
+          full length exact key, because the keys are sorted
+          according to position
+        */
+        if  (_mi_search_next(info, keyinfo, info->lastkey,
+                             info->lastkey_length,
+                             myisam_readnext_vec[search_flag],
+                             info->s->state.key_root[inx]))
+          break;
+      }
+      while (info->lastpos >= info->state->data_file_length);
     }
   }
-
   if (share->concurrent_insert)
     rw_unlock(&share->key_root_lock[inx]);
 

myisam/mi_test_all.res

@@ -5,46 +5,46 @@ myisamchk: MyISAM file test2
 myisamchk: warning: Datafile is almost full,      65532 of      65534 used
 MyISAM-table 'test2' is usable but should be fixed
 Commands   Used count    Errors   Recover errors
-open               17         0                0
-write             850         0                0
-update             85         0                0
-delete            850         0                0
-close              17         0                0
-extra             102         0                0
-Total            1921         0                0
+open                7         0                0
+write             350         0                0
+update             35         0                0
+delete            350         0                0
+close               7         0                0
+extra              42         0                0
+Total             791         0                0
 Commands   Used count    Errors   Recover errors
-open               18         0                0
-write             900         0                0
-update             90         0                0
-delete            900         0                0
-close              18         0                0
-extra             108         0                0
-Total            2034         0                0
+open                8         0                0
+write             400         0                0
+update             40         0                0
+delete            400         0                0
+close               8         0                0
+extra              48         0                0
+Total             904         0                0
 
-real	0m1.054s
-user	0m0.410s
-sys	0m0.640s
+real	0m0.221s
+user	0m0.120s
+sys	0m0.100s
 
-real	0m1.077s
-user	0m0.550s
-sys	0m0.530s
+real	0m0.222s
+user	0m0.140s
+sys	0m0.084s
 
-real	0m1.100s
-user	0m0.420s
-sys	0m0.680s
+real	0m0.232s
+user	0m0.112s
+sys	0m0.120s
 
-real	0m0.783s
-user	0m0.590s
-sys	0m0.200s
+real	0m0.163s
+user	0m0.116s
+sys	0m0.036s
 
-real	0m0.764s
-user	0m0.560s
-sys	0m0.210s
+real	0m0.159s
+user	0m0.136s
+sys	0m0.020s
 
-real	0m0.699s
-user	0m0.570s
-sys	0m0.130s
+real	0m0.147s
+user	0m0.132s
+sys	0m0.016s
 
-real	0m0.991s
-user	0m0.630s
-sys	0m0.350s
+real	0m0.211s
+user	0m0.124s
+sys	0m0.088s

myisam/mi_test_all.sh

@@ -79,7 +79,8 @@ myisamchk$suffix -se test1
 
 # check of myisampack / myisamchk
 myisampack$suffix --force -s test1
-myisamchk$suffix -es test1
+# Ignore error for index file
+myisamchk$suffix -es test1 2>&1 >& /dev/null
 myisamchk$suffix -rqs test1
 myisamchk$suffix -es test1
 myisamchk$suffix -rs test1

mysql-test/r/myisam.result

@@ -472,3 +472,18 @@ select c1 from t1 order by c1 limit 1;
 c1
 a
 drop table t1;
+create table t1 (a int not null, primary key(a));
+create table t2 (a int not null, b int not null, primary key(a,b));
+insert into t1 values (1),(2),(3),(4),(5),(6);
+insert into t2 values (1,1),(2,1);
+lock tables t1 read local, t2 read local;
+select straight_join * from t1,t2 force index (primary) where t1.a=t2.a;
+a	a	b
+1	1	1
+2	2	1
+insert into t2 values(2,0);
+select straight_join * from t1,t2 force index (primary) where t1.a=t2.a;
+a	a	b
+1	1	1
+2	2	1
+drop table t1,t2;

mysql-test/t/myisam.test

@@ -458,3 +458,20 @@ insert into t1 values ('a'), ('b');
 select c1 from t1 order by c1 limit 1;
 drop table t1;
 
+#
+# Bug #14400  Join could miss concurrently inserted row
+#
+create table t1 (a int not null, primary key(a));
+create table t2 (a int not null, b int not null, primary key(a,b));
+insert into t1 values (1),(2),(3),(4),(5),(6);
+insert into t2 values (1,1),(2,1);
+lock tables t1 read local, t2 read local;
+select straight_join * from t1,t2 force index (primary) where t1.a=t2.a;
+connect (root,localhost,root,,test,$MASTER_MYPORT,master.sock);
+insert into t2 values(2,0);
+disconnect root;
+connection default;
+select straight_join * from t1,t2 force index (primary) where t1.a=t2.a;
+drop table t1,t2;
+
+# end of 4.0 tests

sql/sql_select.cc

@@ -7222,6 +7222,8 @@ setup_copy_fields(THD *thd, TMP_TABLE_PARAM *param, List<Item> &fields)
   param->copy_funcs.empty();
   while ((pos=li++))
   {
+    Field *field;
+    char *tmp;
     if (pos->type() == Item::FIELD_ITEM)
     {
       Item_field *item=(Item_field*) pos;
@@ -7245,13 +7247,21 @@ setup_copy_fields(THD *thd, TMP_TABLE_PARAM *param, List<Item> &fields)
       }
 
       /* set up save buffer and change result_field to point at saved value */
-      Field *field= item->field;
+      field= item->field;
       item->result_field=field->new_field(&thd->mem_root,field->table);
-      char *tmp=(char*) sql_alloc(field->pack_length()+1);
+      /*
+        We need to allocate one extra byte for null handling and
+        another extra byte to not get warnings from purify in
+        Field_string::val_int
+      */
+      tmp= (char*) sql_alloc(field->pack_length()+2);
       if (!tmp)
 	goto err;
       copy->set(tmp, item->result_field);
       item->result_field->move_field(copy->to_ptr,copy->to_null_ptr,1);
+#ifdef HAVE_purify
+      copy->to_ptr[copy->from_length]= 0;
+#endif
       copy++;
     }
     else if ((pos->type() == Item::FUNC_ITEM ||