SQL: fill_record() field-value inconsistency fix [#365 bug 2]

Affected tests (forced mode): binlog_encryption.encrypted_slave

sql/sql_base.cc

@@ -8088,7 +8088,7 @@ void switch_to_nullable_trigger_fields(List<Item> &items, TABLE *table)
   Field** field= table->field_to_fill();
 
  /* True if we have NOT NULL fields and BEFORE triggers */
-  if (field != table->field && field != table->non_generated_field)
+  if (field != table->field)
   {
     List_iterator_fast<Item> it(items);
     Item *item;
@@ -8278,6 +8278,12 @@ fill_record(THD *thd, TABLE *table, Field **ptr, List<Item> &values,
     /* Ensure that all fields are from the same table */
     DBUG_ASSERT(field->table == table);
 
+    if (table->versioned() && field->vers_sys_field() && !ignore_errors)
+    {
+      my_error(ER_VERS_READONLY_FIELD, MYF(0), field->field_name.str);
+      goto err;
+    }
+
     value=v++;
     if (field->field_index == autoinc_index)
       table->auto_increment_field_not_null= TRUE;
@@ -8295,13 +8301,6 @@ fill_record(THD *thd, TABLE *table, Field **ptr, List<Item> &values,
       }
     }
 
-    if (table->versioned() && field->vers_sys_field() &&
-        !ignore_errors)
-    {
-      my_error(ER_VERS_READONLY_FIELD, MYF(0), field->field_name.str);
-      goto err;
-    }
-
     if (use_value)
       value->save_val(field);
     else

sql/sql_insert.cc

@@ -676,6 +676,23 @@ static void save_insert_query_plan(THD* thd, TABLE_LIST *table_list)
 }
 
 
+Field **TABLE::field_to_fill()
+{
+  return triggers && triggers->nullable_fields() ? triggers->nullable_fields() : field;
+}
+
+
+inline
+Field **TABLE::user_fields()
+{
+  if (versioned())
+  {
+    return triggers && triggers->vers_user_fields() ? triggers->vers_user_fields() : vers_user_field;
+  }
+  return field_to_fill();
+}
+
+
 /**
   INSERT statement implementation
 
@@ -1002,7 +1019,7 @@ bool mysql_insert(THD *thd,TABLE_LIST *table_list,
         }
         table->reset_default_fields();
         if (fill_record_n_invoke_before_triggers(thd, table,
-                                                 table->field_to_fill(),
+                                                 table->user_fields(),
                                                  *values, 0, TRG_EVENT_INSERT))
         {
           if (values_list.elements != 1 && ! thd->is_error())
@@ -3875,7 +3892,7 @@ void select_insert::store_values(List<Item> &values)
     fill_record_n_invoke_before_triggers(thd, table, *fields, values, 1,
                                          TRG_EVENT_INSERT);
   else
-    fill_record_n_invoke_before_triggers(thd, table, table->field_to_fill(),
+    fill_record_n_invoke_before_triggers(thd, table, table->user_fields(),
                                          values, 1, TRG_EVENT_INSERT);
 
   DBUG_VOID_RETURN;

sql/sql_trigger.cc

@@ -1249,9 +1249,29 @@ bool Table_triggers_list::prepare_record_accessors(TABLE *table)
     *trg_fld= 0;
     DBUG_ASSERT(null_ptr <= extra_null_bitmap + null_bytes);
     bzero(extra_null_bitmap, null_bytes);
+
+    if (table->versioned())
+    {
+      vers_user_field= (Field **)alloc_root(&table->mem_root,
+                                         (table->s->fields - VERSIONING_FIELDS + 1) *
+                                         sizeof(Field*));
+      if (!vers_user_field)
+        return 1;
+      Field **dst= vers_user_field;
+      for (Field **src= record0_field; *src; src++)
+      {
+        if ((*src)->vers_sys_field())
+          continue;
+        *dst++= *src;
+      }
+      *dst= NULL;
+    }
   }
   else
+  {
     record0_field= table->field;
+    vers_user_field= table->vers_user_field;
+  }
 
   if (has_triggers(TRG_EVENT_UPDATE,TRG_ACTION_BEFORE) ||
       has_triggers(TRG_EVENT_UPDATE,TRG_ACTION_AFTER)  ||

sql/sql_trigger.h

@@ -145,6 +145,10 @@ class Table_triggers_list: public Sql_alloc
   */
   Field             **record0_field;
   uchar              *extra_null_bitmap;
+  /**
+    System Versioning: record0_field without system fields.
+  */
+  Field             **vers_user_field;
   /**
     Copy of TABLE::Field array with field pointers set to TABLE::record[1]
     buffer instead of TABLE::record[0] (used for OLD values in on UPDATE
@@ -208,7 +212,7 @@ class Table_triggers_list: public Sql_alloc
   /* End of character ser context. */
 
   Table_triggers_list(TABLE *table_arg)
-    :record0_field(0), extra_null_bitmap(0), record1_field(0),
+    :record0_field(0), extra_null_bitmap(0), vers_user_field(0), record1_field(0),
     trigger_table(table_arg),
     m_has_unparseable_trigger(false), count(0)
   {
@@ -273,6 +277,7 @@ class Table_triggers_list: public Sql_alloc
                                             TABLE_LIST *table_list);
 
   Field **nullable_fields() { return record0_field; }
+  Field **vers_user_fields() { return vers_user_field; }
   void reset_extra_null_bitmap()
   {
     size_t null_bytes= (trigger_table->s->stored_fields -
@@ -307,13 +312,6 @@ class Table_triggers_list: public Sql_alloc
   }
 };
 
-inline Field **TABLE::field_to_fill()
-{
-  return triggers && triggers->nullable_fields() ? triggers->nullable_fields()
-    : non_generated_field ? non_generated_field : field;
-}
-
-
 bool add_table_for_trigger(THD *thd,
                            const sp_name *trg_name,
                            bool continue_if_not_exist,

sql/table.cc

@@ -3197,25 +3197,25 @@ enum open_frm_error open_table_from_share(THD *thd, TABLE_SHARE *share,
 
   if (share->versioned)
   {
-    Field **fptr = NULL;
-    if (!(fptr = (Field **) alloc_root(&outparam->mem_root,
-                                            (uint) ((share->fields+1)*
-                                                    sizeof(Field*)))))
+    Field **dst= (Field **) alloc_root(&outparam->mem_root,
+                                      (share->fields - VERSIONING_FIELDS + 1) *
+                                      sizeof(Field*));
+    if (!dst)
       goto err;
 
-    outparam->non_generated_field = fptr;
-    for (i=0 ; i < share->fields; i++)
+    outparam->vers_user_field= dst;
+    for (Field **src= outparam->field; *src; src++)
     {
-      if (outparam->field[i]->vers_sys_field())
+      if ((*src)->vers_sys_field())
         continue;
-      *fptr++ = outparam->field[i];
+      *dst++= *src;
     }
-    (*fptr)= 0;                                 // End marker
+    (*dst)= NULL;
     outparam->vers_write= true;
   }
   else
   {
-    outparam->non_generated_field= NULL;
+    outparam->vers_user_field= NULL;
     outparam->vers_write= false;
   }
 
@@ -7698,6 +7698,7 @@ int TABLE::update_default_fields(bool update_command, bool ignore_errors)
   DBUG_RETURN(res);
 }
 
+
 void TABLE::vers_update_fields()
 {
   DBUG_ENTER("vers_update_fields");

sql/table.h

@@ -1130,8 +1130,7 @@ struct TABLE
   Field **default_field;                /* Fields with non-constant DEFAULT */
   Field *next_number_field;		/* Set if next_number is activated */
   Field *found_next_number_field;	/* Set on open */
-  Field **non_generated_field;          /* Like **field but without generated
-					                                 fields */
+  Field **vers_user_field;              /* Non-system fields */
   Virtual_column_info **check_constraints;
 
   /* Table's triggers, 0 if there are no of them */
@@ -1487,7 +1486,8 @@ struct TABLE
   bool prepare_triggers_for_delete_stmt_or_event();
   bool prepare_triggers_for_update_stmt_or_event();
 
-  inline Field **field_to_fill();
+  Field **field_to_fill();
+  Field **user_fields();
   bool validate_default_values_of_unset_fields(THD *thd) const;
 
   bool insert_all_rows_into_tmp_table(THD *thd,