MDEV-14792 INSERT without column list into table with explicit versioning columns produces bad data

include/mysql.h

@@ -145,7 +145,6 @@ typedef unsigned long long my_ulonglong;
 #define ER_VIRTUAL_COLUMN_FUNCTION_IS_NOT_ALLOWED ER_GENERATED_COLUMN_FUNCTION_IS_NOT_ALLOWED
 #define ER_PRIMARY_KEY_BASED_ON_VIRTUAL_COLUMN ER_PRIMARY_KEY_BASED_ON_GENERATED_COLUMN
 #define ER_WRONG_FK_OPTION_FOR_VIRTUAL_COLUMN ER_WRONG_FK_OPTION_FOR_GENERATED_COLUMN
-#define ER_WARNING_NON_DEFAULT_VALUE_FOR_VIRTUAL_COLUMN ER_WARNING_NON_DEFAULT_VALUE_FOR_GENERATED_COLUMN
 #define ER_UNSUPPORTED_ACTION_ON_VIRTUAL_COLUMN ER_UNSUPPORTED_ACTION_ON_GENERATED_COLUMN
 #define ER_UNSUPPORTED_ENGINE_FOR_VIRTUAL_COLUMNS ER_UNSUPPORTED_ENGINE_FOR_GENERATED_COLUMNS
 

mysql-test/suite/versioning/r/create.result

@@ -323,8 +323,8 @@ t2	CREATE TABLE `t2` (
 ) ENGINE=NON_DEFAULT_ENGINE DEFAULT CHARSET=latin1 WITH SYSTEM VERSIONING
 create or replace table t1 (
 x26 int,
-st bigint unsigned as row start invisible,
-en bigint unsigned as row end invisible,
+st bigint unsigned as row start,
+en bigint unsigned as row end,
 period for system_time (st, en)
 ) with system versioning engine innodb;
 create or replace table t2 with system versioning engine myisam

mysql-test/suite/versioning/r/cte.result

@@ -47,24 +47,6 @@ emp_id	name	mgr	salary
 1	bill	NULL	1000
 20	john	1	500
 30	jane	1	750
-with recursive
-ancestors
-as
-(
-select e.emp_id, e.name, e.mgr, e.salary
-from emp as e
-where name = 'bill'
-  union
-select e.emp_id, e.name, e.mgr, e.salary
-from emp as e,
-ancestors as a
-where e.mgr = a.emp_id
-)
-select * from ancestors
-for system_time as of timestamp @ts_1;
-emp_id	name	mgr	salary
-1	bill	NULL	1000
-30	jane	1	750
 /* Expected 3 rows */
 with recursive
 ancestors

mysql-test/suite/versioning/r/insert.result

@@ -304,13 +304,38 @@ select x, y, sys_trx_end = 18446744073709551615 as current from t1 for system_ti
 x	y	current
 2	2	1
 1	1	0
-create or replace table t1 (i int) with system versioning engine innodb;
-insert into t1 values  (1),(2);
+create or replace table t1 (x int) with system versioning engine innodb;
+insert into t1 values  (1), (2);
 insert into t1 (sys_trx_start) select sys_trx_end from t1;
-ERROR HY000: Column 'sys_trx_start' is not updatable
+ERROR HY000: The value specified for generated column 'sys_trx_start' in table 't1' ignored
 insert into t1 (sys_trx_start, sys_trx_end) values (DEFAULT, 1);
-ERROR HY000: Column 'sys_trx_end' is not updatable
+ERROR HY000: The value specified for generated column 'sys_trx_end' in table 't1' ignored
+select @@sql_mode into @saved_mode;
+set sql_mode= '';
+insert into t1 (x, sys_trx_start, sys_trx_end) values (3, 4, 5);
+Warnings:
+Warning	1906	The value specified for generated column 'sys_trx_start' in table 't1' ignored
+Warning	1906	The value specified for generated column 'sys_trx_end' in table 't1' ignored
+set sql_mode= @saved_mode;
 insert into t1 (sys_trx_start, sys_trx_end) values (DEFAULT, DEFAULT);
+select * from t1;
+x
+1
+2
+3
+NULL
+# MDEV-14792 INSERT without column list into table with explicit versioning columns produces bad data
+create or replace table t1 (
+i int,
+s timestamp(6) as row start,
+e timestamp(6) as row end,
+c varchar(8),
+period for system_time(s, e))
+with system versioning;
+insert into t1 values (1, null, null, 'foo');
+select i, c, current_row(e) from t1;
+i	c	current_row(e)
+1	foo	1
 drop table t1;
 drop table t2;
 drop procedure test_01;

mysql-test/suite/versioning/t/create.test

@@ -289,8 +289,8 @@ show create table t2;
 
 create or replace table t1 (
   x26 int,
-  st bigint unsigned as row start invisible,
-  en bigint unsigned as row end invisible,
+  st bigint unsigned as row start,
+  en bigint unsigned as row end,
   period for system_time (st, en)
 ) with system versioning engine innodb;
 --error ER_VERS_FIELD_WRONG_TYPE

mysql-test/suite/versioning/t/cte.test

@@ -51,22 +51,6 @@ as
 )
 select * from ancestors;
 
-with recursive
-ancestors
-as
-(
-  select e.emp_id, e.name, e.mgr, e.salary
-  from emp as e
-  where name = 'bill'
-  union
-  select e.emp_id, e.name, e.mgr, e.salary
-  from emp as e,
-       ancestors as a
-  where e.mgr = a.emp_id
-)
-select * from ancestors
-for system_time as of timestamp @ts_1;
-
 /* Expected 3 rows */
 with recursive
 ancestors

mysql-test/suite/versioning/t/insert.test

@@ -206,13 +206,29 @@ insert into t1 values (1, null);
 update t1 set x= x + 1;
 select x, y, sys_trx_end = 18446744073709551615 as current from t1 for system_time all;
 
-create or replace table t1 (i int) with system versioning engine innodb;
-insert into t1 values  (1),(2);
---error ER_NONUPDATEABLE_COLUMN
+create or replace table t1 (x int) with system versioning engine innodb;
+insert into t1 values  (1), (2);
+--error ER_WARNING_NON_DEFAULT_VALUE_FOR_GENERATED_COLUMN
 insert into t1 (sys_trx_start) select sys_trx_end from t1;
---error ER_NONUPDATEABLE_COLUMN
+--error ER_WARNING_NON_DEFAULT_VALUE_FOR_GENERATED_COLUMN
 insert into t1 (sys_trx_start, sys_trx_end) values (DEFAULT, 1);
+select @@sql_mode into @saved_mode;
+set sql_mode= '';
+insert into t1 (x, sys_trx_start, sys_trx_end) values (3, 4, 5);
+set sql_mode= @saved_mode;
 insert into t1 (sys_trx_start, sys_trx_end) values (DEFAULT, DEFAULT);
+select * from t1;
+
+--echo # MDEV-14792 INSERT without column list into table with explicit versioning columns produces bad data
+create or replace table t1 (
+  i int,
+  s timestamp(6) as row start,
+  e timestamp(6) as row end,
+  c varchar(8),
+  period for system_time(s, e))
+with system versioning;
+insert into t1 values (1, null, null, 'foo');
+select i, c, current_row(e) from t1;
 
 drop table t1;
 drop table t2;

sql/sql_base.cc

@@ -7500,6 +7500,7 @@ bool Field::vers_sys_invisible(THD *thd) const
   SELECT_LEX *slex= thd->lex->current_select;
   ulong vers_hide= thd->variables.vers_hide;
   DBUG_ASSERT(table);
+  DBUG_ASSERT(table->versioned());
   DBUG_ASSERT(table->pos_in_table_list);
   TABLE_LIST *tl= table->pos_in_table_list;
   vers_system_time_t vers_type= tl->vers_conditions.type;
@@ -7508,6 +7509,7 @@ bool Field::vers_sys_invisible(THD *thd) const
             slex->nest_level > 0 ||
             vers_hide == VERS_HIDE_FULL ||
             (invisible && (
+              thd->lex->sql_command != SQLCOM_SELECT ||
               vers_hide == VERS_HIDE_IMPLICIT ||
                 (vers_hide == VERS_HIDE_AUTO && (
                   vers_type == SYSTEM_TIME_UNSPECIFIED ||
@@ -7649,8 +7651,6 @@ insert_fields(THD *thd, Name_resolution_context *context, const char *db_name,
             field->vers_sys_invisible(thd) :
             field->invisible))
       {
-        if (thd->lex->sql_command != SQLCOM_CREATE_TABLE ||
-          !(thd->lex->create_info.options & HA_VERSIONED_TABLE))
         continue;
       }
 
@@ -8059,25 +8059,21 @@ fill_record(THD *thd, TABLE *table_arg, List<Item> &fields, List<Item> &values,
         rfield->field_index ==  table->next_number_field->field_index)
       table->auto_increment_field_not_null= TRUE;
     Item::Type type= value->type();
-    if (rfield->vcol_info &&
+    bool vers_sys_field= table->versioned() && rfield->vers_sys_field();
+    if ((rfield->vcol_info || vers_sys_field) &&
         type != Item::DEFAULT_VALUE_ITEM &&
         type != Item::NULL_ITEM &&
         table->s->table_category != TABLE_CATEGORY_TEMPORARY)
     {
       push_warning_printf(thd, Sql_condition::WARN_LEVEL_WARN,
-                          ER_WARNING_NON_DEFAULT_VALUE_FOR_VIRTUAL_COLUMN,
-                          ER_THD(thd, ER_WARNING_NON_DEFAULT_VALUE_FOR_VIRTUAL_COLUMN),
+                          ER_WARNING_NON_DEFAULT_VALUE_FOR_GENERATED_COLUMN,
+                          ER_THD(thd, ER_WARNING_NON_DEFAULT_VALUE_FOR_GENERATED_COLUMN),
                           rfield->field_name.str, table->s->table_name.str);
+      if (vers_sys_field)
+        continue;
     }
     if (only_unvers_fields && !rfield->vers_update_unversioned())
       only_unvers_fields= false;
-    if (table->versioned() && rfield->vers_sys_field())
-    {
-      if (type == Item::DEFAULT_VALUE_ITEM)
-        continue;
-      my_error(ER_NONUPDATEABLE_COLUMN, MYF(0), rfield->field_name.str);
-      goto err;
-    }
 
     if (rfield->stored_in_db() &&
         (value->save_in_field(rfield, 0)) < 0 && !ignore_errors)
@@ -8310,23 +8306,18 @@ 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);
 
-    bool vers_sys_field= table->versioned() && field->vers_sys_field();
-
-    if (vers_sys_field && !ignore_errors)
-    {
-      my_error(ER_NONUPDATEABLE_COLUMN, MYF(0), field->field_name.str);
-      goto err;
-    }
-
-    if (field->invisible && !vers_sys_field)
+    if (field->invisible)
     {
       continue;
     }
     else
       value=v++;
+
+    bool vers_sys_field= table->versioned() && field->vers_sys_field();
+
     if (field->field_index == autoinc_index)
       table->auto_increment_field_not_null= TRUE;
-    if (field->vcol_info)
+    if (field->vcol_info || (vers_sys_field && !ignore_errors))
     {
       Item::Type type= value->type();
       if (type != Item::DEFAULT_VALUE_ITEM &&
@@ -8334,9 +8325,11 @@ fill_record(THD *thd, TABLE *table, Field **ptr, List<Item> &values,
           table->s->table_category != TABLE_CATEGORY_TEMPORARY)
       {
         push_warning_printf(thd, Sql_condition::WARN_LEVEL_WARN,
-                            ER_WARNING_NON_DEFAULT_VALUE_FOR_VIRTUAL_COLUMN,
-                            ER_THD(thd, ER_WARNING_NON_DEFAULT_VALUE_FOR_VIRTUAL_COLUMN),
+                            ER_WARNING_NON_DEFAULT_VALUE_FOR_GENERATED_COLUMN,
+                            ER_THD(thd, ER_WARNING_NON_DEFAULT_VALUE_FOR_GENERATED_COLUMN),
                             field->field_name.str, table->s->table_name.str);
+        if (vers_sys_field)
+          continue;
       }
     }
 

sql/sql_insert.cc

@@ -682,17 +682,6 @@ Field **TABLE::field_to_fill()
 }
 
 
-inline
-Field **TABLE::vers_user_field_to_fill()
-{
-  if (versioned())
-  {
-    return triggers && triggers->vers_user_fields() ? triggers->vers_user_fields() : vers_user_field;
-  }
-  return field_to_fill();
-}
-
-
 /**
   INSERT statement implementation
 
@@ -1020,7 +1009,7 @@ bool mysql_insert(THD *thd,TABLE_LIST *table_list,
         }
         table->reset_default_fields();
         if (fill_record_n_invoke_before_triggers(thd, table,
-                                                 table->vers_user_field_to_fill(),
+                                                 table->field_to_fill(),
                                                  *values, 0, TRG_EVENT_INSERT))
         {
           if (values_list.elements != 1 && ! thd->is_error())
@@ -2612,9 +2601,6 @@ TABLE *Delayed_insert::get_local_table(THD* client_thd)
   }
   *field=0;
 
-  if (copy->versioned() && copy->vers_update_user_field(client_thd->mem_root))
-    goto error;
-
   if (share->virtual_fields || share->default_expressions ||
       share->default_fields)
   {
@@ -3913,7 +3899,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->vers_user_field_to_fill(),
+    fill_record_n_invoke_before_triggers(thd, table, table->field_to_fill(),
                                          values, 1, TRG_EVENT_INSERT);
 
   DBUG_VOID_RETURN;

sql/sql_trigger.cc

@@ -1249,28 +1249,10 @@ 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) ||

sql/sql_trigger.h

@@ -145,10 +145,6 @@ 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
@@ -212,7 +208,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), vers_user_field(0), record1_field(0),
+    :record0_field(0), extra_null_bitmap(0), record1_field(0),
     trigger_table(table_arg),
     m_has_unparseable_trigger(false), count(0)
   {
@@ -277,7 +273,6 @@ 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 -

sql/table.cc

@@ -3225,17 +3225,7 @@ enum open_frm_error open_table_from_share(THD *thd, TABLE_SHARE *share,
   }
   (*field_ptr)= 0;                              // End marker
 
-  if (share->versioned)
-  {
-    if (outparam->vers_update_user_field())
-      goto err;
-    outparam->vers_write= true;
-  }
-  else
-  {
-    outparam->vers_user_field= NULL;
-    outparam->vers_write= false;
-  }
+  outparam->vers_write= share->versioned;
 
   if (share->found_next_number_field)
     outparam->found_next_number_field=
@@ -7792,27 +7782,6 @@ void TABLE::vers_update_fields()
 }
 
 
-bool TABLE::vers_update_user_field(MEM_ROOT *_mem_root)
-{
-  DBUG_ASSERT(versioned());
-  Field **dst= (Field **) alloc_root(_mem_root ? _mem_root : &mem_root,
-                                    (s->fields - VERSIONING_FIELDS + 1) *
-                                    sizeof(Field*));
-  if (!dst)
-    return true;
-
-  vers_user_field= dst;
-  for (Field **src= field; *src; src++)
-  {
-    if ((*src)->vers_sys_field())
-      continue;
-    *dst++= *src;
-  }
-  (*dst)= NULL;
-  return false;
-}
-
-
 bool TABLE_LIST::vers_vtmd_name(String& out) const
 {
   static const char *vtmd_suffix= "_vtmd";

sql/table.h

@@ -1151,7 +1151,6 @@ 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 **vers_user_field;              /* Non-system fields */
   Virtual_column_info **check_constraints;
 
   /* Table's triggers, 0 if there are no of them */
@@ -1509,8 +1508,6 @@ struct TABLE
   bool prepare_triggers_for_update_stmt_or_event();
 
   Field **field_to_fill();
-  Field **vers_user_field_to_fill();
-  bool vers_update_user_field(MEM_ROOT *mem_root= NULL);
   bool validate_default_values_of_unset_fields(THD *thd) const;
 
   bool insert_all_rows_into_tmp_table(THD *thd,