MDEV-6668: Server crashes in check_view_single_update on concurrent DDL/DML...

MDEV-6668: Server crashes in check_view_single_update on concurrent DDL/DML flow with views and triggers

Call mysql_derived_reinit() if we are reusing view.
This is needed as during a previous error condition the view may not have been reset

sql/sql_derived.cc:
  More DBUG_PRINT
  Always reset merged_for_insert (no reason to not do that)
sql/sql_derived.h:
  Added prototype
sql/sql_insert.cc:
  More DBUG_PRINT
  Added DBUG_ASSERT
sql/sql_view.cc:
  Call mysql_derived_reinit() if we are reusing view.
  This is needed as during a previous error condition the view may not have been reset
sql/table.cc:
  More DBUG_PRINT

sql/sql_derived.cc

@@ -503,6 +503,15 @@ bool mysql_derived_merge(THD *thd, LEX *lex, TABLE_LIST *derived)
 bool mysql_derived_merge_for_insert(THD *thd, LEX *lex, TABLE_LIST *derived)
 {
   DBUG_ENTER("mysql_derived_merge_for_insert");
+  DBUG_PRINT("enter", ("derived: %p", derived));
+  DBUG_PRINT("info", ("merged_for_insert: %d  is_materialized_derived: %d  "
+                      "is_multitable: %d  single_table_updatable: %d  "
+                      "merge_underlying_list: %d",
+                      derived->merged_for_insert,
+                      derived->is_materialized_derived(),
+                      derived->is_multitable(),
+                      derived->single_table_updatable(),
+                      derived->merge_underlying_list != 0));
   if (derived->merged_for_insert)
     DBUG_RETURN(FALSE);
   if (derived->is_materialized_derived())
@@ -516,6 +525,7 @@ bool mysql_derived_merge_for_insert(THD *thd, LEX *lex, TABLE_LIST *derived)
       derived->table= derived->merge_underlying_list->table;
       derived->schema_table= derived->merge_underlying_list->schema_table;
       derived->merged_for_insert= TRUE;
+      DBUG_ASSERT(derived->table);
     }
   }
   DBUG_RETURN(FALSE);
@@ -544,6 +554,7 @@ bool mysql_derived_init(THD *thd, LEX *lex, TABLE_LIST *derived)
 {
   SELECT_LEX_UNIT *unit= derived->get_unit();
   DBUG_ENTER("mysql_derived_init");
+  DBUG_PRINT("enter", ("derived: %p", derived));
 
   // Skip already prepared views/DT
   if (!unit || unit->prepared)
@@ -689,6 +700,7 @@ bool mysql_derived_prepare(THD *thd, LEX *lex, TABLE_LIST *derived)
   thd->create_tmp_table_for_derived= FALSE;
 
   derived->table= derived->derived_result->table;
+  DBUG_ASSERT(derived->table);
   if (derived->is_derived() && derived->is_merged_derived())
     first_select->mark_as_belong_to_derived(derived);
 
@@ -956,7 +968,6 @@ bool mysql_derived_reinit(THD *thd, LEX *lex, TABLE_LIST *derived)
   DBUG_ENTER("mysql_derived_reinit");
   st_select_lex_unit *unit= derived->get_unit();
 
-  if (derived->table)
   derived->merged_for_insert= FALSE;
   unit->unclean();
   unit->types.empty();
@@ -965,4 +976,3 @@ bool mysql_derived_reinit(THD *thd, LEX *lex, TABLE_LIST *derived)
   unit->set_thd(thd);
   DBUG_RETURN(FALSE);
 }
-

sql/sql_derived.h

@@ -23,6 +23,7 @@ struct LEX;
 bool mysql_handle_derived(LEX *lex, uint phases);
 bool mysql_handle_single_derived(LEX *lex, TABLE_LIST *derived, uint phases);
 bool mysql_handle_list_of_derived(LEX *lex, TABLE_LIST *dt_list, uint phases);
+bool mysql_derived_reinit(THD *thd, LEX *lex, TABLE_LIST *derived);
 
 /**
    Cleans up the SELECT_LEX_UNIT for the derived table (if any).

sql/sql_insert.cc

@@ -148,9 +148,11 @@ bool check_view_single_update(List<Item> &fields, List<Item> *values,
   if (view->check_single_table(&tbl, tables, view) || tbl == 0)
     goto error;
 
+  /* view->table should have been set in mysql_derived_merge_for_insert */
+  DBUG_ASSERT(view->table);
+
   /*
-    A buffer for the insert values was allocated for the merged view.
-    Use it.
+    Use buffer for the insert values that was allocated for the merged view.
   */
   tbl->table->insert_values= view->table->insert_values;
   view->table= tbl->table;
@@ -195,11 +197,12 @@ static int check_insert_fields(THD *thd, TABLE_LIST *table_list,
                                table_map *map)
 {
   TABLE *table= table_list->table;
+  DBUG_ENTER("check_insert_fields");
 
   if (!table_list->single_table_updatable())
   {
     my_error(ER_NON_INSERTABLE_TABLE, MYF(0), table_list->alias, "INSERT");
-    return -1;
+    DBUG_RETURN(-1);
   }
 
   if (fields.elements == 0 && values.elements != 0)
@@ -208,18 +211,18 @@ static int check_insert_fields(THD *thd, TABLE_LIST *table_list,
     {
       my_error(ER_VIEW_NO_INSERT_FIELD_LIST, MYF(0),
                table_list->view_db.str, table_list->view_name.str);
-      return -1;
+      DBUG_RETURN(-1);
     }
     if (values.elements != table->s->fields)
     {
       my_error(ER_WRONG_VALUE_COUNT_ON_ROW, MYF(0), 1L);
-      return -1;
+      DBUG_RETURN(-1);
     }
 #ifndef NO_EMBEDDED_ACCESS_CHECKS
     Field_iterator_table_ref field_it;
     field_it.set(table_list);
     if (check_grant_all_columns(thd, INSERT_ACL, &field_it))
-      return -1;
+      DBUG_RETURN(-1);
 #endif
     /*
       No fields are provided so all fields must be provided in the values.
@@ -237,7 +240,7 @@ static int check_insert_fields(THD *thd, TABLE_LIST *table_list,
     if (fields.elements != values.elements)
     {
       my_error(ER_WRONG_VALUE_COUNT_ON_ROW, MYF(0), 1L);
-      return -1;
+      DBUG_RETURN(-1);
     }
 
     thd->dup_field= 0;
@@ -263,7 +266,7 @@ static int check_insert_fields(THD *thd, TABLE_LIST *table_list,
     thd->lex->select_lex.no_wrap_view_item= FALSE;
 
     if (res)
-      return -1;
+      DBUG_RETURN(-1);
 
     if (table_list->is_view() && table_list->is_merged_derived())
     {
@@ -271,14 +274,14 @@ static int check_insert_fields(THD *thd, TABLE_LIST *table_list,
                                    fields_and_values_from_different_maps ?
                                    (List<Item>*) 0 : &values,
                                    table_list, map, true))
-        return -1;
+        DBUG_RETURN(-1);
       table= table_list->table;
     }
 
     if (check_unique && thd->dup_field)
     {
       my_error(ER_FIELD_SPECIFIED_TWICE, MYF(0), thd->dup_field->field_name);
-      return -1;
+      DBUG_RETURN(-1);
     }
     if (table->default_field)
       table->mark_default_fields_for_write();
@@ -296,10 +299,10 @@ static int check_insert_fields(THD *thd, TABLE_LIST *table_list,
        check_view_insertability(thd, table_list)))
   {
     my_error(ER_NON_INSERTABLE_TABLE, MYF(0), table_list->alias, "INSERT");
-    return -1;
+    DBUG_RETURN(-1);
   }
 
-  return 0;
+  DBUG_RETURN(0);
 }
 
 

sql/sql_view.cc

@@ -34,6 +34,7 @@
 #include "sp.h"
 #include "sp_cache.h"
 #include "datadict.h"   // dd_frm_is_view()
+#include "sql_derived.h"
 
 #define MD5_BUFF_LENGTH 33
 
@@ -1063,6 +1064,15 @@ bool mysql_make_view(THD *thd, File_parser *parser, TABLE_LIST *table,
     DBUG_PRINT("info",
                ("VIEW %s.%s is already processed on previous PS/SP execution",
                 table->view_db.str, table->view_name.str));
+
+    /*
+      Clear old variables in the TABLE_LIST that could be left from an old view
+      This is only needed if there was an error at last usage of view,
+      in which case the reinit call wasn't done.
+      See MDEV-6668 for details.
+    */
+    mysql_derived_reinit(thd, NULL, table);
+
     DBUG_RETURN(0);
   }
 

sql/table.cc

@@ -4728,23 +4728,26 @@ bool TABLE_LIST::check_single_table(TABLE_LIST **table_arg,
 
 bool TABLE_LIST::set_insert_values(MEM_ROOT *mem_root)
 {
+  DBUG_ENTER("set_insert_values");
   if (table)
   {
+    DBUG_PRINT("info", ("setting insert_value for table"));
     if (!table->insert_values &&
         !(table->insert_values= (uchar *)alloc_root(mem_root,
                                                    table->s->rec_buff_length)))
-      return TRUE;
+      DBUG_RETURN(TRUE);
   }
   else
   {
+    DBUG_PRINT("info", ("setting insert_value for view"));
     DBUG_ASSERT(is_view_or_derived() && is_merged_derived());
     for (TABLE_LIST *tbl= (TABLE_LIST*)view->select_lex.table_list.first;
          tbl;
          tbl= tbl->next_local)
       if (tbl->set_insert_values(mem_root))
-        return TRUE;
+        DBUG_RETURN(TRUE);
   }
-  return FALSE;
+  DBUG_RETURN(FALSE);
 }
 
 
@@ -6897,15 +6900,16 @@ void TABLE_LIST::reset_const_table()
 
 bool TABLE_LIST::handle_derived(LEX *lex, uint phases)
 {
-  SELECT_LEX_UNIT *unit= get_unit();
-  if (unit)
+  SELECT_LEX_UNIT *unit;
+  DBUG_ENTER("handle_derived");
+  if ((unit= get_unit()))
   {
     for (SELECT_LEX *sl= unit->first_select(); sl; sl= sl->next_select())
       if (sl->handle_derived(lex, phases))
-        return TRUE;
-    return mysql_handle_single_derived(lex, this, phases);
+        DBUG_RETURN(TRUE);
+    DBUG_RETURN(mysql_handle_single_derived(lex, this, phases));
   }
-  return FALSE;
+  DBUG_RETURN(FALSE);
 }