Fixed lock sorting and lock check issues with thr_lock that caused warnings...

Fixed lock sorting and lock check issues with thr_lock that caused warnings when running test suite.
Safety check that could cause core dump when doing create table with virtual column.

mysql-test/mysql-test-run.pl:
  Show also warnings from thr_lock (which starts with just Warning, not Warning:)
mysql-test/r/lock.result:
  Added test that showed not relevant warning when using table locks.
mysql-test/t/lock.test:
  Added test that showed not relevant warning when using table locks.
mysys/thr_lock.c:
  Fixed sorting of locks.
  (Old sort code didn't handle case where TL_WRITE_CONCURRENT_INSERT must be sorted before TL_WRITE)
  Added more information to check_locks warning output.
  Fixed wrong testing of multiple different write locks for same table.
sql/item_cmpfunc.cc:
  Safety check that could cause core dump when doing create table with virtual column.

mysql-test/mysql-test-run.pl

@@ -4383,7 +4383,7 @@ sub extract_warning_lines ($) {
 
   my @patterns =
     (
-     qr/^Warning:|mysqld: Warning|\[Warning\]/,
+     qr/^Warning|mysqld: Warning|\[Warning\]/,
      qr/^Error:|\[ERROR\]/,
      qr/^==\d+==\s+\S/, # valgrind errors
      qr/InnoDB: Warning|InnoDB: Error/,

mysql-test/r/lock.result

@@ -194,3 +194,13 @@ ERROR HY000: Table 't2' was locked with a READ lock and can't be updated
 UNLOCK TABLES;
 DROP TABLE t1,t2;
 End of 5.1 tests.
+create table t1 (a int) engine=myisam;
+lock tables t1 write concurrent, t1 as t2 read;
+lock tables t1 read local;
+unlock tables;
+unlock tables;
+lock tables t1 read local;
+lock tables t1 write concurrent, t1 as t2 read;
+unlock tables;
+unlock tables;
+drop table t1;

mysql-test/t/lock.test

@@ -245,3 +245,28 @@ UNLOCK TABLES;
 DROP TABLE t1,t2;
 
 --echo End of 5.1 tests.
+
+#
+# Test concurrent lock and read locks
+# This gave a warning:
+# Warning at 'read lock with old write lock' for lock: 5:
+# Found lock of type 8 that is write and read locked. Read_no_write_count: 1
+#
+create table t1 (a int) engine=myisam;
+lock tables t1 write concurrent, t1 as t2 read;
+connect (con1,localhost,root,,);
+connection con1;
+lock tables t1 read local;
+unlock tables;
+connection default;
+unlock tables;
+connection con1;
+lock tables t1 read local;
+connection default;
+lock tables t1 write concurrent, t1 as t2 read;
+unlock tables;
+connection con1;
+unlock tables;
+disconnect con1;
+connection default;
+drop table t1;

mysys/thr_lock.c

@@ -112,25 +112,37 @@ static inline pthread_cond_t *get_cond(void)
 
 
 /*
+  Sort locks in priority order
+
+  LOCK_CMP()
+    A       First lock
+    B       Second lock
+
+  Return:
+      0 if A >= B
+      1 if A < B
+
   Priority for locks (decides in which order locks are locked)
   We want all write locks to be first, followed by read locks.
   Locks from MERGE tables has a little lower priority than other
   locks, to allow one to release merge tables without having
   to unlock and re-lock other locks.
   The lower the number, the higher the priority for the lock.
-  Read locks should have 4, write locks should have 0.
-  UNLOCK is 8, to force these last in thr_merge_locks.
   For MERGE tables we add 2 (THR_LOCK_MERGE_PRIV) to the lock priority.
   THR_LOCK_LATE_PRIV (1) is used when one locks other tables to be merged
   with existing locks. This way we prioritize the original locks over the
   new locks.
 */
 
-static uint lock_priority[(uint)TL_WRITE_ONLY+1] =
-{ 8, 4, 4, 4, 4, 4, 0, 0, 0, 0, 0, 0, 0, 0};
-
-#define LOCK_CMP(A,B) ((uchar*) ((A)->lock) + lock_priority[(uint) (A)->type] + (A)->priority < (uchar*) ((B)->lock) + lock_priority[(uint) (B)->type] + (B)->priority)
 
+inline int LOCK_CMP(THR_LOCK_DATA *A, THR_LOCK_DATA *B)
+{
+  if (A->lock != B->lock)
+    return A->lock < B->lock;
+  if (A->type != B->type)
+    return A->type > B->type;                   /* Prioritize read */
+  return A->priority < B->priority;
+}
 
 /*
   For the future (now the thread specific cond is alloced by my_pthread.c)
@@ -215,6 +227,7 @@ static int check_lock(struct st_lock_list *list, const char* lock_type,
 
 
 static void check_locks(THR_LOCK *lock, const char *where,
+                        enum thr_lock_type type,
 			my_bool allow_no_locks)
 {
   uint old_found_errors=found_errors;
@@ -279,6 +292,7 @@ static void check_locks(THR_LOCK *lock, const char *where,
 	    found_errors++;
 	    fprintf(stderr,
 		    "Warning at '%s': Write lock %d waiting while no exclusive read locks\n",where,(int) lock->write_wait.data->type);
+            DBUG_PRINT("warning", ("Warning at '%s': Write lock %d waiting while no exclusive read locks\n",where,(int) lock->write_wait.data->type));
 	  }
 	}	      
       }
@@ -293,8 +307,10 @@ static void check_locks(THR_LOCK *lock, const char *where,
             if (data->type != TL_WRITE_CONCURRENT_INSERT)
             {
               fprintf(stderr,
-                      "Warning at '%s': Found TL_WRITE_CONCURRENT_INSERT lock mixed with other write locks\n",
-                      where);
+                      "Warning at '%s': Found TL_WRITE_CONCURRENT_INSERT lock mixed with other write lock: %d\n",
+                      where, data->type);
+              DBUG_PRINT("warning", ("Warning at '%s': Found TL_WRITE_CONCURRENT_INSERT lock mixed with other write lock: %d\n",
+                                     where, data->type));
               break;
             }
           }
@@ -309,26 +325,34 @@ static void check_locks(THR_LOCK *lock, const char *where,
 	    fprintf(stderr,
 		    "Warning at '%s': Found WRITE_ALLOW_WRITE lock waiting for WRITE_ALLOW_WRITE lock\n",
 		    where);
+            DBUG_PRINT("warning", ("Warning at '%s': Found WRITE_ALLOW_WRITE lock waiting for WRITE_ALLOW_WRITE lock\n",
+                                   where));
+
 	  }
 	}
 	if (lock->read.data)
 	{
-          if (!thr_lock_owner_equal(lock->write.data->owner,
-                                    lock->read.data->owner) &&
+          THR_LOCK_DATA *data;
+          for (data=lock->read.data ; data ; data=data->next)
+          {
+            if (!thr_lock_owner_equal(lock->write.data->owner,
+                                      data->owner) &&
 	      ((lock->write.data->type > TL_WRITE_DELAYED &&
 		lock->write.data->type != TL_WRITE_ONLY) ||
 	       ((lock->write.data->type == TL_WRITE_CONCURRENT_INSERT ||
 		 lock->write.data->type == TL_WRITE_ALLOW_WRITE) &&
-		lock->read_no_write_count)))
-	  {
-	    found_errors++;
-	    fprintf(stderr,
-		    "Warning at '%s': Found lock of type %d that is write and read locked\n",
-		    where, lock->write.data->type);
-	    DBUG_PRINT("warning",("At '%s': Found lock of type %d that is write and read locked\n",
-		    where, lock->write.data->type));
-
-	  }
+                data->type == TL_READ_NO_INSERT)))
+            {
+              found_errors++;
+              fprintf(stderr,
+                      "Warning at '%s' for lock: %d: Found lock of type %d that is write and read locked. Read_no_write_count: %d\n",
+                      where, (int) type, lock->write.data->type,
+                      lock->read_no_write_count);
+              DBUG_PRINT("warning",("At '%s' for lock %d: Found lock of type %d that is write and read locked\n",
+                                    where, (int) type,
+                                    lock->write.data->type));
+            }
+          }
 	}
 	if (lock->read_wait.data)
 	{
@@ -354,7 +378,7 @@ static void check_locks(THR_LOCK *lock, const char *where,
 }
 
 #else /* EXTRA_DEBUG */
-#define check_locks(A,B,C)
+#define check_locks(A,B,C,D)
 #endif
 
 
@@ -531,13 +555,14 @@ wait_for_lock(struct st_lock_list *wait, THR_LOCK_DATA *data,
       else
 	wait->last=data->prev;
       data->type= TL_UNLOCK;                    /* No lock */
-      check_locks(data->lock, "killed or timed out wait_for_lock", 1);
+      check_locks(data->lock, "killed or timed out wait_for_lock", data->type,
+                  1);
       wake_up_waiters(data->lock);
     }
     else
     {
       DBUG_PRINT("thr_lock", ("lock aborted"));
-      check_locks(data->lock, "aborted wait_for_lock", 0);
+      check_locks(data->lock, "aborted wait_for_lock", data->type, 0);
     }
   }
   else
@@ -546,7 +571,7 @@ wait_for_lock(struct st_lock_list *wait, THR_LOCK_DATA *data,
     if (data->lock->get_status)
       (*data->lock->get_status)(data->status_param,
                                 data->type == TL_WRITE_CONCURRENT_INSERT);
-    check_locks(data->lock,"got wait_for_lock",0);
+    check_locks(data->lock,"got wait_for_lock", data->type, 0);
   }
   pthread_mutex_unlock(&data->lock->mutex);
 
@@ -579,7 +604,7 @@ thr_lock(THR_LOCK_DATA *data, THR_LOCK_OWNER *owner,
                      (long) data, data->owner->info->thread_id,
                      (long) lock, (int) lock_type));
   check_locks(lock,(uint) lock_type <= (uint) TL_READ_NO_INSERT ?
-	      "enter read_lock" : "enter write_lock",0);
+	      "enter read_lock" : "enter write_lock", lock_type, 0);
   if ((int) lock_type <= (int) TL_READ_NO_INSERT)
   {
     /* Request for READ lock */
@@ -608,7 +633,7 @@ thr_lock(THR_LOCK_DATA *data, THR_LOCK_OWNER *owner,
 	lock->read.last= &data->next;
 	if (lock_type == TL_READ_NO_INSERT)
 	  lock->read_no_write_count++;
-	check_locks(lock,"read lock with old write lock",0);
+	check_locks(lock,"read lock with old write lock", lock_type, 0);
 	if (lock->get_status)
 	  (*lock->get_status)(data->status_param, 0);
 	statistic_increment(locks_immediate,&THR_LOCK_lock);
@@ -632,7 +657,7 @@ thr_lock(THR_LOCK_DATA *data, THR_LOCK_OWNER *owner,
       lock->read.last= &data->next;
       if (lock_type == TL_READ_NO_INSERT)
 	lock->read_no_write_count++;
-      check_locks(lock,"read lock with no write locks",0);
+      check_locks(lock,"read lock with no write locks", lock_type, 0);
       if (lock->get_status)
 	(*lock->get_status)(data->status_param, 0);
       statistic_increment(locks_immediate,&THR_LOCK_lock);
@@ -717,7 +742,7 @@ thr_lock(THR_LOCK_DATA *data, THR_LOCK_OWNER *owner,
 	(*lock->write.last)=data;	/* Add to running fifo */
 	data->prev=lock->write.last;
 	lock->write.last= &data->next;
-	check_locks(lock,"second write lock",0);
+	check_locks(lock,"second write lock", lock_type, 0);
 	if (lock->get_status)
 	  (*lock->get_status)(data->status_param,
                               lock_type == TL_WRITE_CONCURRENT_INSERT);
@@ -755,7 +780,7 @@ thr_lock(THR_LOCK_DATA *data, THR_LOCK_OWNER *owner,
 	  lock->write.last= &data->next;
 	  if (lock->get_status)
 	    (*lock->get_status)(data->status_param, concurrent_insert);
-	  check_locks(lock,"only write lock",0);
+	  check_locks(lock,"only write lock", lock_type, 0);
 	  statistic_increment(locks_immediate,&THR_LOCK_lock);
 	  goto end;
 	}
@@ -791,7 +816,7 @@ static inline void free_all_read_locks(THR_LOCK *lock,
 {
   THR_LOCK_DATA *data=lock->read_wait.data;
 
-  check_locks(lock,"before freeing read locks",1);
+  check_locks(lock,"before freeing read locks", TL_UNLOCK, 1);
 
   /* move all locks from read_wait list to read list */
   (*lock->read.last)=data;
@@ -833,7 +858,7 @@ static inline void free_all_read_locks(THR_LOCK *lock,
   *lock->read_wait.last=0;
   if (!lock->read_wait.data)
     lock->write_lock_count=0;
-  check_locks(lock,"after giving read locks",0);
+  check_locks(lock,"after giving read locks", TL_UNLOCK, 0);
 }
 
 	/* Unlock lock and free next thread on same lock */
@@ -846,7 +871,7 @@ void thr_unlock(THR_LOCK_DATA *data, uint unlock_flags)
   DBUG_PRINT("lock",("data: 0x%lx  thread: 0x%lx  lock: 0x%lx",
                      (long) data, data->owner->info->thread_id, (long) lock));
   pthread_mutex_lock(&lock->mutex);
-  check_locks(lock,"start of release lock",0);
+  check_locks(lock,"start of release lock", lock_type, 0);
 
   if (((*data->prev)=data->next))		/* remove from lock-list */
     data->next->prev= data->prev;
@@ -880,9 +905,9 @@ void thr_unlock(THR_LOCK_DATA *data, uint unlock_flags)
   if (lock_type == TL_READ_NO_INSERT)
     lock->read_no_write_count--;
   data->type=TL_UNLOCK;				/* Mark unlocked */
-  check_locks(lock,"after releasing lock",1);
+  check_locks(lock,"after releasing lock", lock_type, 1);
   wake_up_waiters(lock);
-  check_locks(lock,"end of thr_unlock",1);
+  check_locks(lock,"end of thr_unlock", lock_type, 1);
   pthread_mutex_unlock(&lock->mutex);
   DBUG_VOID_RETURN;
 }
@@ -1007,7 +1032,7 @@ static void wake_up_waiters(THR_LOCK *lock)
       free_all_read_locks(lock,0);
   }
 end:
-  check_locks(lock, "after waking up waiters", 0);
+  check_locks(lock, "after waking up waiters", TL_UNLOCK, 0);
   DBUG_VOID_RETURN;
 }
 
@@ -1317,7 +1342,7 @@ void thr_downgrade_write_lock(THR_LOCK_DATA *in_data,
   DBUG_ASSERT(old_lock_type == TL_WRITE_ONLY);
   DBUG_ASSERT(old_lock_type > new_lock_type);
   in_data->type= new_lock_type;
-  check_locks(lock,"after downgrading lock",0);
+  check_locks(lock,"after downgrading lock", old_lock_type, 0);
 
 #if TO_BE_REMOVED
   switch (old_lock_type)
@@ -1417,7 +1442,8 @@ void thr_downgrade_write_lock(THR_LOCK_DATA *in_data,
         data->prev= lock->write.last;
         lock->write.last= &data->next;
         data->next= 0;
-        check_locks(lock, "Started write lock after downgrade",0);
+        check_locks(lock, "Started write lock after downgrade", new_lock_type,
+                    0);
         data->cond= 0;
         pthread_cond_signal(cond);
       }
@@ -1470,13 +1496,15 @@ void thr_downgrade_write_lock(THR_LOCK_DATA *in_data,
 
         if (data->type == TL_READ_NO_INSERT)
           lock->read_no_write_count++;
-        check_locks(lock, "Started read lock after downgrade",0);
+        check_locks(lock, "Started read lock after downgrade", new_lock_type,
+                    0);
         data->cond= 0;
         pthread_cond_signal(cond);
       }
     }
   }
-  check_locks(lock,"after starting waiters after downgrading lock",0);
+  check_locks(lock,"after starting waiters after downgrading lock",
+              new_lock_type, 0);
 #endif
   pthread_mutex_unlock(&lock->mutex);
   DBUG_VOID_RETURN;
@@ -1497,7 +1525,7 @@ my_bool thr_upgrade_write_delay_lock(THR_LOCK_DATA *data,
     pthread_mutex_unlock(&lock->mutex);
     DBUG_RETURN(data->type == TL_UNLOCK);	/* Test if Aborted */
   }
-  check_locks(lock,"before upgrading lock",0);
+  check_locks(lock,"before upgrading lock", data->type, 0);
   /* TODO:  Upgrade to TL_WRITE_CONCURRENT_INSERT in some cases */
   data->type= new_lock_type;                    /* Upgrade lock */
 
@@ -1525,11 +1553,11 @@ my_bool thr_upgrade_write_delay_lock(THR_LOCK_DATA *data,
       lock->write_wait.last= &data->next;
     data->prev= &lock->write_wait.data;
     lock->write_wait.data=data;
-    check_locks(lock,"upgrading lock",0);
+    check_locks(lock,"upgrading lock", new_lock_type, 0);
   }
   else
   {
-    check_locks(lock,"waiting for lock",0);
+    check_locks(lock,"waiting for lock", new_lock_type, 0);
   }
   res= wait_for_lock(&lock->write_wait,data,1);
   if (res == THR_LOCK_SUCCESS && lock->start_trans)

sql/item_cmpfunc.cc

@@ -412,7 +412,8 @@ static bool convert_constant_item(THD *thd, Item_field *field_item,
     LINT_INIT(old_maps[0]);
     LINT_INIT(old_maps[1]);
 
-    if (table)
+    /* table->read_set may not be set if we come here from a CREATE TABLE */
+    if (table && table->read_set)
       dbug_tmp_use_all_columns(table, old_maps, 
                                table->read_set, table->write_set);
     /* For comparison purposes allow invalid dates like 2000-01-32 */
@@ -448,7 +449,7 @@ static bool convert_constant_item(THD *thd, Item_field *field_item,
     }
     thd->variables.sql_mode= orig_sql_mode;
     thd->count_cuted_fields= orig_count_cuted_fields;
-    if (table)
+    if (table && table->read_set)
       dbug_tmp_restore_column_maps(table->read_set, table->write_set, old_maps);
   }
   return result;