MDEV-20661 Virtual fields are not recalculated on system fields value assignment

Fix stale virtual field value in 4 cases: when virtual field depends
on row_start/row_end in timestamp/trx_id versioned table. row_start
dep is recalculated in vers_update_fields() (SQL and InnoDB
layer). row_end dep is recalculated on history row insert.

Author: midenok

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

@@ -335,3 +335,18 @@ insert into t1 (a, b) values (1, 2);
 replace into t1 (a, b) values (3, 2);
 replace into t1 (a, b) values (4, 2);
 drop table t1;
+#
+# MDEV-20661 Virtual fields are not recalculated on system fields value assignment
+#
+create table t1 (
+a int,
+row_start SYS_DATATYPE as row start invisible,
+row_end SYS_DATATYPE as row end invisible,
+period for system_time (row_start, row_end),
+v1 bigint unsigned as (a ^ row_start) unique,
+v2 bigint unsigned as (a ^ row_end) unique
+) engine=innodb with system versioning;
+insert into t1 (a) values (1), (2);
+update ignore t1 set a= 3;
+delete history from t1;
+drop table t1;

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

@@ -265,4 +265,25 @@ replace into t1 (a, b) values (4, 2);
 # cleanup
 drop table t1;
 
+--echo #
+--echo # MDEV-20661 Virtual fields are not recalculated on system fields value assignment
+--echo #
+
+replace_result $sys_datatype_expl SYS_DATATYPE;
+eval create table t1 (
+  a int,
+  row_start $sys_datatype_expl as row start invisible,
+  row_end $sys_datatype_expl as row end invisible,
+  period for system_time (row_start, row_end),
+  v1 bigint unsigned as (a ^ row_start) unique,
+  v2 bigint unsigned as (a ^ row_end) unique
+) engine=innodb with system versioning;
+
+insert into t1 (a) values (1), (2);
+update ignore t1 set a= 3;
+delete history from t1;
+
+# cleanup
+drop table t1;
+
 source suite/versioning/common_finish.inc;

sql/sql_insert.cc

@@ -1658,6 +1658,10 @@ int vers_insert_history_row(TABLE *table)
   if (row_start->cmp(row_start->ptr, row_end->ptr) >= 0)
     return 0;
 
+  if (table->vfield &&
+      table->update_virtual_fields(table->file, VCOL_UPDATE_FOR_READ))
+    return HA_ERR_GENERIC;
+
   return table->file->ha_write_row(table->record[0]);
 }
 

sql/table.cc

@@ -8078,29 +8078,24 @@ void TABLE::vers_update_fields()
   bitmap_set_bit(write_set, vers_start_field()->field_index);
   bitmap_set_bit(write_set, vers_end_field()->field_index);
 
-  if (versioned(VERS_TIMESTAMP))
+  if (!vers_write)
   {
-    if (!vers_write)
-    {
-      file->column_bitmaps_signal();
-      return;
-    }
-    if (vers_start_field()->store_timestamp(in_use->query_start(),
-                                            in_use->query_start_sec_part()))
-      DBUG_ASSERT(0);
+    file->column_bitmaps_signal();
+    return;
   }
-  else
+
+  if (versioned(VERS_TIMESTAMP) &&
+      vers_start_field()->store_timestamp(in_use->query_start(),
+                                          in_use->query_start_sec_part()))
   {
-    if (!vers_write)
-    {
-      file->column_bitmaps_signal();
-      return;
-    }
+    DBUG_ASSERT(0);
   }
 
   vers_end_field()->set_max();
   bitmap_set_bit(read_set, vers_end_field()->field_index);
   file->column_bitmaps_signal();
+  if (vfield)
+    update_virtual_fields(file, VCOL_UPDATE_FOR_READ);
 }
 
 

storage/innobase/handler/ha_innodb.cc

@@ -21814,3 +21814,70 @@ ib_push_frm_error(
 		break;
 	}
 }
+
+/** Writes 8 bytes to nth tuple field
+@param[in]	tuple	where to write
+@param[in]	nth	index in tuple
+@param[in]	data	what to write
+@param[in]	buf	field data buffer */
+static void set_tuple_col_8(dtuple_t *tuple, int col, uint64_t data, byte *buf)
+{
+  dfield_t *dfield= dtuple_get_nth_field(tuple, col);
+  ut_ad(dfield->type.len == 8);
+  if (dfield->len == UNIV_SQL_NULL)
+  {
+    dfield_set_data(dfield, buf, 8);
+  }
+  ut_ad(dfield->len == dfield->type.len && dfield->data);
+  mach_write_to_8(dfield->data, data);
+}
+
+void ins_node_t::vers_update_end(row_prebuilt_t *prebuilt, bool history_row)
+{
+  ut_ad(prebuilt->ins_node == this);
+  trx_t *trx= prebuilt->trx;
+#ifndef DBUG_OFF
+  ut_ad(table->vers_start != table->vers_end);
+  const mysql_row_templ_t *t= prebuilt->get_template_by_col(table->vers_end);
+  ut_ad(t);
+  ut_ad(t->mysql_col_len == 8);
+#endif
+
+  if (history_row)
+  {
+    set_tuple_col_8(row, table->vers_end, trx->id, vers_end_buf);
+  }
+  else /* ROW_INS_VERSIONED */
+  {
+    set_tuple_col_8(row, table->vers_end, TRX_ID_MAX, vers_end_buf);
+#ifndef DBUG_OFF
+    t= prebuilt->get_template_by_col(table->vers_start);
+    ut_ad(t);
+    ut_ad(t->mysql_col_len == 8);
+#endif
+    set_tuple_col_8(row, table->vers_start, trx->id, vers_start_buf);
+  }
+  dict_index_t *clust_index= dict_table_get_first_index(table);
+  THD *thd= trx->mysql_thd;
+  TABLE *mysql_table= prebuilt->m_mysql_table;
+  mem_heap_t *local_heap= NULL;
+  for (ulint col_no= 0; col_no < dict_table_get_n_v_cols(table); col_no++)
+  {
+
+    const dict_v_col_t *v_col= dict_table_get_nth_v_col(table, col_no);
+    for (ulint i= 0; i < unsigned(v_col->num_base); i++)
+    {
+      dict_col_t *base_col= v_col->base_col[i];
+      if (base_col->ind == table->vers_end)
+      {
+        innobase_get_computed_value(row, v_col, clust_index, &local_heap,
+                                    table->heap, NULL, thd, mysql_table,
+                                    mysql_table->record[0], NULL, NULL, NULL);
+      }
+    }
+  }
+  if (local_heap)
+  {
+    mem_heap_free(local_heap);
+  }
+}

storage/innobase/include/row0ins.h

@@ -163,6 +163,8 @@ row_ins_step(
 #define	INS_NODE_INSERT_ENTRIES 3	/* index entries should be built and
 					inserted */
 
+struct row_prebuilt_t;
+
 /** Insert node structure */
 struct ins_node_t
 {
@@ -203,6 +205,7 @@ struct ins_node_t
 				entry_list and sys fields are stored here;
 				if this is NULL, entry list should be created
 				and buffers for sys fields in row allocated */
+        void vers_update_end(row_prebuilt_t *prebuilt, bool history_row);
 };
 
 /** Create an insert object.

storage/innobase/include/row0upd.h

@@ -454,7 +454,32 @@ struct upd_t{
 		fields[n_fields++] = field;
 	}
 
-	/** Determine if the given field_no is modified.
+        void remove_element(ulint i)
+        {
+          ut_ad(n_fields > 0);
+          ut_ad(i < n_fields);
+          while (i < n_fields - 1)
+          {
+            fields[i]= fields[i + 1];
+            i++;
+          }
+          n_fields--;
+        }
+
+        bool remove(const ulint field_no)
+        {
+          for (ulint i= 0; i < n_fields; ++i)
+          {
+            if (field_no == fields[i].field_no)
+            {
+              remove_element(i);
+              return true;
+            }
+          }
+          return false;
+        }
+
+        /** Determine if the given field_no is modified.
 	@return true if modified, false otherwise.  */
 	bool is_modified(const ulint field_no) const
 	{

storage/innobase/row/row0mysql.cc

@@ -1331,23 +1331,6 @@ row_mysql_get_table_status(
 	return(err);
 }
 
-/** Writes 8 bytes to nth tuple field
-@param[in]	tuple	where to write
-@param[in]	nth	index in tuple
-@param[in]	data	what to write
-@param[in]	buf	field data buffer */
-static
-void
-set_tuple_col_8(dtuple_t* tuple, int col, uint64_t data, byte* buf) {
-	dfield_t* dfield = dtuple_get_nth_field(tuple, col);
-	ut_ad(dfield->type.len == 8);
-	if (dfield->len == UNIV_SQL_NULL) {
-		dfield_set_data(dfield, buf, 8);
-	}
-	ut_ad(dfield->len == dfield->type.len && dfield->data);
-	mach_write_to_8(dfield->data, data);
-}
-
 /** Does an insert for MySQL.
 @param[in]	mysql_rec	row in the MySQL format
 @param[in,out]	prebuilt	prebuilt struct in MySQL handle
@@ -1415,29 +1398,8 @@ row_insert_for_mysql(
 					  &blob_heap);
 
 	if (ins_mode != ROW_INS_NORMAL) {
-#ifndef DBUG_OFF
-		ut_ad(table->vers_start != table->vers_end);
-		const mysql_row_templ_t* t
-		    = prebuilt->get_template_by_col(table->vers_end);
-		ut_ad(t);
-		ut_ad(t->mysql_col_len == 8);
-#endif
-
-		if (ins_mode == ROW_INS_HISTORICAL) {
-			set_tuple_col_8(node->row, table->vers_end, trx->id,
-					node->vers_end_buf);
-		} else /* ROW_INS_VERSIONED */ {
-			set_tuple_col_8(node->row, table->vers_end, TRX_ID_MAX,
-					node->vers_end_buf);
-#ifndef DBUG_OFF
-			t = prebuilt->get_template_by_col(table->vers_start);
-			ut_ad(t);
-			ut_ad(t->mysql_col_len == 8);
-#endif
-			set_tuple_col_8(node->row, table->vers_start, trx->id,
-					node->vers_start_buf);
-		}
-	}
+          node->vers_update_end(prebuilt, ins_mode == ROW_INS_HISTORICAL);
+        }
 
 	savept = trx_savept_take(trx);
 

storage/innobase/row/row0upd.cc

@@ -3526,5 +3526,25 @@ void upd_node_t::vers_update_fields(const trx_t *trx, ulint idx)
   }
 
   dfield_set_data(&ufield->new_val, update->vers_sys_value, col->len);
-}
 
+  for (ulint col_no= 0; col_no < dict_table_get_n_v_cols(table); col_no++)
+  {
+
+    const dict_v_col_t *v_col= dict_table_get_nth_v_col(table, col_no);
+    if (!v_col->m_col.ord_part)
+      continue;
+    for (ulint i= 0; i < unsigned(v_col->num_base); i++)
+    {
+      dict_col_t *base_col= v_col->base_col[i];
+      if (base_col->ind == col->ind)
+      {
+        /* Virtual column depends on system field value
+        which we updated above. Remove it from update
+        vector, so it is recalculated in
+        row_upd_store_v_row() (see !update branch). */
+        update->remove(v_col->v_pos);
+        break;
+      }
+    }
+  }
+}