MDEV-28254 Wrong position for row_start, row_end after adding column to implicit versioned table

Implicit system-versioned table does not contain system fields in SHOW
CREATE. Therefore after mysqldump recovery such table has system
fields in the last place in frm image. The original table meanwhile
does not guarantee these system fields on last place because adding
new fields via ALTER TABLE places them last. Thus the order of fields
may be different between master and slave, so row-based replication
may fail.

To fix this on ALTER TABLE we now place system-invisible fields always
last in frm image. If the table was created via old revision and has
an incorrect order of fields it can be fixed via any copy operation of
ALTER TABLE, f.ex.:

  ALTER TABLE t1 FORCE;

To check the order of fields in frm file one can use hexdump:

  hexdump -C t1.frm

Note, the replication fails only when all 3 conditions are met:

  1. row-based or mixed mode replication;
  2. table has new fields added via ALTER TABLE;
  3. table was rebuilt on some, but not all nodes via mysqldump image.

Otherwise it will operate properly even with incorrect order of
fields.

4 files changed, 107 insertions, 1 deletions

diff --git a/mysql-test/include/grep.inc b/mysql-test/include/grep.inc
new file mode 100644
index 00000000000..9ce7cddb7b7
--- /dev/null
+++ b/mysql-test/include/grep.inc
@@ -0,0 +1,16 @@
+# Grep file for regular expression and output to STDOUT
+#
+# Usage:
+# --let $grep_file= /path/to/your/file
+# --let $grep_regex= your_regex_string
+# --source include/grep.inc
+
+--perl
+  open (my $fh, "<", "$ENV{grep_file}") or die $!;
+  while (<$fh>)
+  {
+    /$ENV{grep_regex}/ &&
+      print;
+  }
+  close $fh;
+EOF
diff --git a/mysql-test/suite/versioning/r/rpl_row.result b/mysql-test/suite/versioning/r/rpl_row.result
index 03ac8dc9eb8..e5db01b9e78 100644
--- a/mysql-test/suite/versioning/r/rpl_row.result
+++ b/mysql-test/suite/versioning/r/rpl_row.result
@@ -11,4 +11,42 @@ connection slave;
 connection master;
 drop table t1;
 set binlog_row_image= @old_row_image;
+#
+# MDEV-28254 Wrong position for row_start, row_end after adding column
+#            to implicit versioned table
+#
+set @@system_versioning_alter_history= keep;
+set @@session.time_zone='+00:00';
+create table t1 (x int) with system versioning engine innodb;
+alter table t1 add column y int, algorithm=inplace;
+check table t1;
+Table	Op	Msg_type	Msg_text
+test.t1	check	status	OK
+connection slave;
+drop table t1;
+show create table t1;
+Table	Create Table
+t1	CREATE TABLE `t1` (
+  `x` int(11) DEFAULT NULL,
+  `y` int(11) DEFAULT NULL
+) ENGINE=InnoDB DEFAULT CHARSET=latin1 WITH SYSTEM VERSIONING
+connection master;
+set timestamp= 12345;
+insert t1 values (1, 1);
+select *, unix_timestamp(row_start) as row_start, unix_timestamp(row_end) as row_end from t1;
+x	y	row_start	row_end
+1	1	12345.000000	2147483647.999999
+set timestamp= default;
+### INSERT INTO `test`.`t1`
+### SET
+###   @1=1
+###   @2=1
+###   @3=12345.000000
+###   @4=2147483647.999999
+connection slave;
+select * from t1;
+x	y
+1	1
+connection master;
+drop table t1;
 include/rpl_end.inc
diff --git a/mysql-test/suite/versioning/t/rpl_row.test b/mysql-test/suite/versioning/t/rpl_row.test
index 17ce2dfdcf8..2e5d4c76f4a 100644
--- a/mysql-test/suite/versioning/t/rpl_row.test
+++ b/mysql-test/suite/versioning/t/rpl_row.test
@@ -1,5 +1,6 @@
 --source include/have_binlog_format_row.inc
 --source include/master-slave.inc
+--source include/have_innodb.inc
 
 --echo # MDEV-16252: MINIMAL binlog_row_image does not work for versioned tables
 set @old_row_image= @@binlog_row_image;
@@ -15,4 +16,44 @@ update t1 set i = 0;
 drop table t1;
 set binlog_row_image= @old_row_image;
 
+--echo #
+--echo # MDEV-28254 Wrong position for row_start, row_end after adding column
+--echo #            to implicit versioned table
+--echo #
+--let TMP= $MYSQLTEST_VARDIR/tmp
+--let $MYSQLD_DATADIR= `select @@datadir`
+set @@system_versioning_alter_history= keep;
+set @@session.time_zone='+00:00';
+
+create table t1 (x int) with system versioning engine innodb;
+alter table t1 add column y int, algorithm=inplace;
+check table t1;
+--exec $MYSQL_DUMP --databases test > $TMP/dump.sql
+
+--sync_slave_with_master
+drop table t1;
+--exec $MYSQL_SLAVE test < $TMP/dump.sql
+show create table t1;
+
+--connection master
+set timestamp= 12345;
+--let $start_pos= query_get_value("SHOW MASTER STATUS", Position, 1)
+insert t1 values (1, 1);
+select *, unix_timestamp(row_start) as row_start, unix_timestamp(row_end) as row_end from t1;
+--let $stop_pos= query_get_value("SHOW MASTER STATUS", Position, 1)
+set timestamp= default;
+
+# NOTE: pipe grep is not Windows-compatible
+--let grep_file= $TMP/out.txt
+--let grep_regex= ^###
+--exec $MYSQL_BINLOG -v -j $start_pos --stop-position=$stop_pos -o 3 $MYSQLD_DATADIR/master-bin.000001 > $grep_file
+--source include/grep.inc
+--sync_slave_with_master
+select * from t1;
+
+--connection master
+drop table t1;
+--remove_files_wildcard $TMP *.txt
+--remove_files_wildcard $TMP *.sql
+
 --source include/rpl_end.inc
diff --git a/sql/sql_table.cc b/sql/sql_table.cc
index 648b2f3cba5..62b9cb22df0 100644
--- a/sql/sql_table.cc
+++ b/sql/sql_table.cc
@@ -8027,6 +8027,8 @@ mysql_prepare_alter_table(THD *thd, TABLE *table,
 {
   /* New column definitions are added here */
   List<Create_field> new_create_list;
+  /* System-invisible fields must be added last */
+  List<Create_field> new_create_tail;
   /* New key definitions are added here */
   List<Key> new_key_list;
   List_iterator<Alter_drop> drop_it(alter_info->drop_list);
@@ -8251,7 +8253,7 @@ mysql_prepare_alter_table(THD *thd, TABLE *table,
       dropped_sys_vers_fields|= field->flags;
       drop_it.remove();
     }
-    else
+    else if (field->invisible < INVISIBLE_SYSTEM)
     {
       /*
         This field was not dropped and not changed, add it to the list
@@ -8276,6 +8278,12 @@ mysql_prepare_alter_table(THD *thd, TABLE *table,
 	alter_it.remove();
       }
     }
+    else
+    {
+      DBUG_ASSERT(field->invisible == INVISIBLE_SYSTEM);
+      def= new (thd->mem_root) Create_field(thd, field, field);
+      new_create_tail.push_back(def, thd->mem_root);
+    }
   }
   dropped_sys_vers_fields &= VERS_SYSTEM_FIELD;
   if ((dropped_sys_vers_fields ||
@@ -8408,6 +8416,9 @@ mysql_prepare_alter_table(THD *thd, TABLE *table,
       alter_it.remove();
     }
   }
+
+  new_create_list.append(&new_create_tail);
+
   if (unlikely(alter_info->alter_list.elements))
   {
     my_error(ER_BAD_FIELD_ERROR, MYF(0),