BUG#17138: Crashes in stored procedure

Last round of review fixes


BUILD/compile-pentium-gcov:
  No change
sql/ha_ndbcluster.h:
  Last round of review changes
sql/ha_partition.h:
  Last round of review changes
sql/handler.h:
  Last round of review changes
sql/item_sum.cc:
  Last round of review changes
sql/sql_acl.cc:
  Last round of review changes
sql/sql_insert.cc:
  Last round of review changes
sql/sql_select.cc:
  Last round of review changes
sql/sql_table.cc:
  Last round of review changes
sql/sql_union.cc:
  Last round of review changes
sql/sql_update.cc:
  Last round of review changes

sql/ha_ndbcluster.h

@@ -656,9 +656,8 @@ class ha_ndbcluster: public handler
   void set_auto_partitions(partition_info *part_info);
   virtual bool is_fatal_error(int error, uint flags)
   {
-    if (!handler::is_fatal_error(error, flags))
-      return FALSE;
-    if (error == HA_ERR_NO_PARTITION_FOUND)
+    if (!handler::is_fatal_error(error, flags) ||
+        error == HA_ERR_NO_PARTITION_FOUND)
       return FALSE;
     return TRUE;
   }

sql/ha_partition.h

@@ -304,9 +304,8 @@ public:
 
   virtual bool is_fatal_error(int error, uint flags)
   {
-    if (!handler::is_fatal_error(error, flags))
-      return FALSE;
-    if (error == HA_ERR_NO_PARTITION_FOUND)
+    if (!handler::is_fatal_error(error, flags) ||
+        error == HA_ERR_NO_PARTITION_FOUND)
       return FALSE;
     return TRUE;
   }

sql/handler.h

@@ -235,6 +235,11 @@
 /* Options of START TRANSACTION statement (and later of SET TRANSACTION stmt) */
 #define MYSQL_START_TRANS_OPT_WITH_CONS_SNAPSHOT 1
 
+/* Flags for method is_fatal_error */
+#define HA_CHECK_DUP_KEY 1
+#define HA_CHECK_DUP_UNIQUE 2
+#define HA_CHECK_DUP (HA_CHECK_DUP_KEY + HA_CHECK_DUP_UNIQUE)
+
 enum legacy_db_type
 {
   DB_TYPE_UNKNOWN=0,DB_TYPE_DIAB_ISAM=1,
@@ -972,17 +977,14 @@ public:
     ignorable than others. E.g. the partition handler can get inserts
     into a range where there is no partition and this is an ignorable
     error.
-    HA_ERR_FOUND_DUPP_UNIQUE is a special case in MyISAM that means the
-    same thing as HA_ERR_FOUND_DUPP_KEY but can in some cases lead to
+    HA_ERR_FOUND_DUP_UNIQUE is a special case in MyISAM that means the
+    same thing as HA_ERR_FOUND_DUP_KEY but can in some cases lead to
     a slightly different error message.
   */
-#define HA_CHECK_DUPP_KEY 1
-#define HA_CHECK_DUPP_UNIQUE 2
-#define HA_CHECK_DUPP (HA_CHECK_DUPP_KEY + HA_CHECK_DUPP_UNIQUE)
   virtual bool is_fatal_error(int error, uint flags)
   {
     if (!error ||
-        ((flags & HA_CHECK_DUPP_KEY) &&
+        ((flags & HA_CHECK_DUP_KEY) &&
          (error == HA_ERR_FOUND_DUPP_KEY ||
           error == HA_ERR_FOUND_DUPP_UNIQUE)))
       return FALSE;

sql/item_sum.cc

@@ -2663,7 +2663,7 @@ bool Item_sum_count_distinct::add()
     return tree->unique_add(table->record[0] + table->s->null_bytes);
   }
   if ((error= table->file->ha_write_row(table->record[0])) &&
-      table->file->is_fatal_error(error, HA_CHECK_DUPP))
+      table->file->is_fatal_error(error, HA_CHECK_DUP))
     return TRUE;
   return FALSE;
 }

sql/sql_acl.cc

@@ -2049,7 +2049,7 @@ static int replace_user_table(THD *thd, TABLE *table, const LEX_USER &combo,
   }
   else if ((error=table->file->ha_write_row(table->record[0]))) // insert
   {						// This should never happen
-    if (table->file->is_fatal_error(error, HA_CHECK_DUPP))
+    if (table->file->is_fatal_error(error, HA_CHECK_DUP))
     {
       table->file->print_error(error,MYF(0));	/* purecov: deadcode */
       error= -1;				/* purecov: deadcode */
@@ -2171,7 +2171,7 @@ static int replace_db_table(TABLE *table, const char *db,
   }
   else if (rights && (error= table->file->ha_write_row(table->record[0])))
   {
-    if (table->file->is_fatal_error(error, HA_CHECK_DUPP_KEY))
+    if (table->file->is_fatal_error(error, HA_CHECK_DUP_KEY))
       goto table_error; /* purecov: deadcode */
   }
 
@@ -2743,7 +2743,7 @@ static int replace_table_table(THD *thd, GRANT_TABLE *grant_table,
   else
   {
     error=table->file->ha_write_row(table->record[0]);
-    if (table->file->is_fatal_error(error, HA_CHECK_DUPP_KEY))
+    if (table->file->is_fatal_error(error, HA_CHECK_DUP_KEY))
       goto table_error;				/* purecov: deadcode */
   }
 
@@ -2861,7 +2861,7 @@ static int replace_routine_table(THD *thd, GRANT_NAME *grant_name,
   else
   {
     error=table->file->ha_write_row(table->record[0]);
-    if (table->file->is_fatal_error(error, HA_CHECK_DUPP_KEY))
+    if (table->file->is_fatal_error(error, HA_CHECK_DUP_KEY))
       goto table_error;
   }
 

sql/sql_insert.cc

@@ -977,20 +977,24 @@ int write_record(THD *thd, TABLE *table,COPY_INFO *info)
     {
       uint key_nr;
       bool is_duplicate_key_error;
-      if (table->file->is_fatal_error(error, HA_CHECK_DUPP))
+      if (table->file->is_fatal_error(error, HA_CHECK_DUP))
 	goto err;
       table->file->restore_auto_increment(); // it's too early here! BUG#20188
       is_duplicate_key_error= table->file->is_fatal_error(error, 0);
       if (!is_duplicate_key_error)
       {
+        /*
+          We come here when we had an ignorable error which is not a duplicate
+          key error. In this we ignore error if ignore flag is set, otherwise
+          report error as usual. We will not do any duplicate key processing.
+        */
         if (info->ignore)
-          goto ok_or_after_trg_err;
-        else
+          goto ok_or_after_trg_err; /* Ignoring a not fatal error, return 0 */
         goto err;
       }
       if ((int) (key_nr = table->file->get_dup_key(error)) < 0)
       {
-	error=HA_ERR_FOUND_DUPP_KEY;         /* Database can't find key */
+	error= HA_ERR_FOUND_DUPP_KEY;         /* Database can't find key */
 	goto err;
       }
       /* Read all columns for the row we are going to replace */
@@ -1072,7 +1076,7 @@ int write_record(THD *thd, TABLE *table,COPY_INFO *info)
                                               table->record[0])))
 	{
           if (info->ignore &&
-              !table->file->is_fatal_error(error, HA_CHECK_DUPP_KEY))
+              !table->file->is_fatal_error(error, HA_CHECK_DUP_KEY))
             goto ok_or_after_trg_err;
           goto err;
 	}
@@ -1155,7 +1159,7 @@ int write_record(THD *thd, TABLE *table,COPY_INFO *info)
   else if ((error=table->file->ha_write_row(table->record[0])))
   {
     if (!info->ignore ||
-        table->file->is_fatal_error(error, HA_CHECK_DUPP))
+        table->file->is_fatal_error(error, HA_CHECK_DUP))
       goto err;
     table->file->restore_auto_increment();
     goto ok_or_after_trg_err;

sql/sql_select.cc

@@ -9354,7 +9354,7 @@ bool create_myisam_from_heap(THD *thd, TABLE *table, TMP_TABLE_PARAM *param,
   /* copy row that filled HEAP table */
   if ((write_err=new_table.file->write_row(table->record[0])))
   {
-    if (new_table.file->is_fatal_error(write_err, HA_CHECK_DUPP) ||
+    if (new_table.file->is_fatal_error(write_err, HA_CHECK_DUP) ||
 	!ignore_last_dupp_key_error)
       goto err;
   }
@@ -10777,7 +10777,7 @@ end_write(JOIN *join, JOIN_TAB *join_tab __attribute__((unused)),
       join->found_records++;
       if ((error=table->file->write_row(table->record[0])))
       {
-        if (table->file->is_fatal_error(error, HA_CHECK_DUPP))
+        if (!table->file->is_fatal_error(error, HA_CHECK_DUP))
 	  goto end;
 	if (create_myisam_from_heap(join->thd, table, &join->tmp_table_param,
 				    error,1))

sql/sql_table.cc

@@ -6322,11 +6322,10 @@ copy_data_between_tables(TABLE *from,TABLE *to,
     }
     if ((error=to->file->ha_write_row((byte*) to->record[0])))
     {
-      if (!ignore ||
-          handle_duplicates != DUP_REPLACE || /* Currently always false */
-          to->file->is_fatal_error(error, HA_CHECK_DUPP))
+      if (!ignore || handle_duplicates != DUP_ERROR ||
+          to->file->is_fatal_error(error, HA_CHECK_DUP))
       {
-         if (!to->file->is_fatal_error(error, HA_CHECK_DUPP))
+         if (!to->file->is_fatal_error(error, HA_CHECK_DUP))
          {
            uint key_nr= to->file->get_dup_key(error);
            if ((int) key_nr >= 0)

sql/sql_union.cc

@@ -65,7 +65,7 @@ bool select_union::send_data(List<Item> &values)
   if ((error= table->file->ha_write_row(table->record[0])))
   {
     /* create_myisam_from_heap will generate error if needed */
-    if (table->file->is_fatal_error(error, HA_CHECK_DUPP) &&
+    if (table->file->is_fatal_error(error, HA_CHECK_DUP) &&
         create_myisam_from_heap(thd, table, &tmp_table_param, error, 1))
       return 1;
   }

sql/sql_update.cc

@@ -542,13 +542,13 @@ int mysql_update(THD *thd,
           }
 	}
  	else if (!ignore ||
-                 table->file->is_fatal_error(error, HA_CHECK_DUPP_KEY))
+                 table->file->is_fatal_error(error, HA_CHECK_DUP_KEY))
 	{
           /*
             If (ignore && error is ignorable) we don't have to
             do anything; otherwise...
           */
-          if (table->file->is_fatal_error(error, HA_CHECK_DUPP_KEY))
+          if (table->file->is_fatal_error(error, HA_CHECK_DUP_KEY))
             thd->fatal_error(); /* Other handler errors are fatal */
 	  table->file->print_error(error,MYF(0));
 	  error= 1;
@@ -1424,13 +1424,13 @@ bool multi_update::send_data(List<Item> &not_used_values)
 	{
 	  updated--;
           if (!ignore ||
-              table->file->is_fatal_error(error, HA_CHECK_DUPP_KEY))
+              table->file->is_fatal_error(error, HA_CHECK_DUP_KEY))
 	  {
             /*
               If (ignore && error == is ignorable) we don't have to
               do anything; otherwise...
             */
-            if (table->file->is_fatal_error(error, HA_CHECK_DUPP_KEY))
+            if (table->file->is_fatal_error(error, HA_CHECK_DUP_KEY))
               thd->fatal_error(); /* Other handler errors are fatal */
 	    table->file->print_error(error,MYF(0));
 	    DBUG_RETURN(1);
@@ -1459,7 +1459,7 @@ bool multi_update::send_data(List<Item> &not_used_values)
       /* Write row, ignoring duplicated updates to a row */
       if ((error= tmp_table->file->ha_write_row(tmp_table->record[0])))
       {
-        if (tmp_table->file->is_fatal_error(error, HA_CHECK_DUPP) &&
+        if (tmp_table->file->is_fatal_error(error, HA_CHECK_DUP) &&
             create_myisam_from_heap(thd, tmp_table,
                                          tmp_table_param + offset, error, 1))
 	{
@@ -1583,7 +1583,7 @@ int multi_update::do_updates(bool from_send_error)
 						    table->record[0])))
 	{
 	  if (!ignore ||
-              table->file->is_fatal_error(local_error, HA_CHECK_DUPP_KEY))
+              table->file->is_fatal_error(local_error, HA_CHECK_DUP_KEY))
 	    goto err;
 	}
 	updated++;