MDEV-30483 After upgrade to 10.6 from Mysql 5.7 seeing "InnoDB: Column...

MDEV-30483 After upgrade to 10.6 from Mysql 5.7 seeing "InnoDB: Column last_update in table mysql.innodb_table_stats is BINARY(4) NOT NULL but should be INT UNSIGNED NOT NULL"

Problem:

Field_timestampf implementations differ in MySQL and MariaDB:
- MariaDB sets the UNSIGNED_FLAG in Field::flags
- MySQL does not

The reference table structures
(defined in table_stats_schema and index_stats_schema)
expected the last_update column to have the DATA_UNSIGNED flag,
because MariaDB's Field_timestampf has the UNSIGNED_FLAG.
It worked fine on pure MariaDB installations.

However, if a MariaDB server starts over a MySQL-5.7 data directory during
a migration, the last_update column does not have DATA_UNSIGNED flag,
because MySQL's Field_timestampf does not have the UNSIGNED_FLAG.

This made InnoDB (after the migration from MySQL) complain into the server
error log about the unexpected data type.

The actual fix is done in storage/innobase/dict/dict0stats.cc:
It removes DATA_UNSIGNED from the prtype_mask member of the reference columns,
so now it does not require the underlying columns to have this flag.

The rest of the fix is needed for MTR tests.
The new data type plugin TYPE_MYSQL_TIMESTAMP implements a slightly modified
version of Field_timestampf, which removes the unsigned flag, so
it works like MySQL's Field_timestampf.

The MTR test ALTERs the data type of the columns
table_stats_schema.last_update and index_stats_schema.last_update
from TIMESTAMP to TYPE_MYSQL_TIMESTAMP, then makes InnoDB
verify the structure of the two statistics tables by creating
and populating an InnoDB table t1.

Without the fix made storage/innobase/dict/dict0stats.cc,
MTR complains about unexpected warnings in the server error log:

[ERROR] InnoDB: Column last_update in table mysql.innodb_table_stats is ...
[ERROR] InnoDB: Column last_update in table mysql.innodb_index_stats is ...

With the fix made storage/innobase/dict/dict0stats.cc these warnings
go away.

plugin/type_mysql_timestamp/CMakeLists.txt

+# Copyright (c) 2019, MariaDB corporation
+# 
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; version 2 of the License.
+# 
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+# 
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1335  USA
+
+MYSQL_ADD_PLUGIN(type_mysql_timestamp plugin.cc RECOMPILE_FOR_EMBEDDED
+                 MODULE_ONLY COMPONENT Test)

plugin/type_mysql_timestamp/mysql-test/type_mysql_timestamp/suite.opt

+--plugin-load-add=$TYPE_MYSQL_TIMESTAMP_SO

plugin/type_mysql_timestamp/mysql-test/type_mysql_timestamp/suite.pm

+package My::Suite::Type_test;
+
+@ISA = qw(My::Suite);
+
+return "No TYPE_TEST plugin" unless $ENV{TYPE_MYSQL_TIMESTAMP_SO};
+
+sub is_default { 1 }
+
+bless { };
+

plugin/type_mysql_timestamp/mysql-test/type_mysql_timestamp/type_mysql_timestamp.result

+#
+# MDEV-30483 After upgrade to 10.6 from Mysql 5.7 seeing "InnoDB: Column last_update in table mysql.innodb_table_stats is BINARY(4) NOT NULL but should be INT UNSIGNED NOT NULL"
+#
+SELECT
+PLUGIN_NAME,
+PLUGIN_VERSION,
+PLUGIN_STATUS,
+PLUGIN_TYPE,
+PLUGIN_AUTHOR,
+PLUGIN_DESCRIPTION,
+PLUGIN_LICENSE,
+PLUGIN_MATURITY,
+PLUGIN_AUTH_VERSION
+FROM INFORMATION_SCHEMA.PLUGINS
+WHERE PLUGIN_TYPE='DATA TYPE'
+    AND PLUGIN_NAME LIKE 'type_mysql_timestamp';
+PLUGIN_NAME	type_mysql_timestamp
+PLUGIN_VERSION	1.0
+PLUGIN_STATUS	ACTIVE
+PLUGIN_TYPE	DATA TYPE
+PLUGIN_AUTHOR	MariaDB Corporation
+PLUGIN_DESCRIPTION	Data type TYPE_MYSQL_TIMESTAMP
+PLUGIN_LICENSE	GPL
+PLUGIN_MATURITY	Experimental
+PLUGIN_AUTH_VERSION	1.0
+CREATE TABLE t1 (a TYPE_MYSQL_TIMESTAMP);
+SHOW CREATE TABLE t1;
+Table	Create Table
+t1	CREATE TABLE `t1` (
+  `a` type_mysql_timestamp NOT NULL DEFAULT current_timestamp() ON UPDATE current_timestamp()
+) ENGINE=MyISAM DEFAULT CHARSET=latin1 COLLATE=latin1_swedish_ci
+DROP TABLE t1;
+CREATE TABLE t1 (a TIMESTAMP);
+SHOW CREATE TABLE t1;
+Table	Create Table
+t1	CREATE TABLE `t1` (
+  `a` timestamp NOT NULL DEFAULT current_timestamp() ON UPDATE current_timestamp()
+) ENGINE=MyISAM DEFAULT CHARSET=latin1 COLLATE=latin1_swedish_ci
+ALTER TABLE t1 MODIFY a TYPE_MYSQL_TIMESTAMP;
+SHOW CREATE TABLE t1;
+Table	Create Table
+t1	CREATE TABLE `t1` (
+  `a` type_mysql_timestamp NOT NULL DEFAULT current_timestamp() ON UPDATE current_timestamp()
+) ENGINE=MyISAM DEFAULT CHARSET=latin1 COLLATE=latin1_swedish_ci
+DROP TABLE t1;

plugin/type_mysql_timestamp/mysql-test/type_mysql_timestamp/type_mysql_timestamp.test

+--source include/have_innodb.inc
+
+--echo #
+--echo # MDEV-30483 After upgrade to 10.6 from Mysql 5.7 seeing "InnoDB: Column last_update in table mysql.innodb_table_stats is BINARY(4) NOT NULL but should be INT UNSIGNED NOT NULL"
+--echo #
+
+--vertical_results
+SELECT
+  PLUGIN_NAME,
+  PLUGIN_VERSION,
+  PLUGIN_STATUS,
+  PLUGIN_TYPE,
+  PLUGIN_AUTHOR,
+  PLUGIN_DESCRIPTION,
+  PLUGIN_LICENSE,
+  PLUGIN_MATURITY,
+  PLUGIN_AUTH_VERSION
+FROM INFORMATION_SCHEMA.PLUGINS
+  WHERE PLUGIN_TYPE='DATA TYPE'
+    AND PLUGIN_NAME LIKE 'type_mysql_timestamp';
+--horizontal_results
+
+CREATE TABLE t1 (a TYPE_MYSQL_TIMESTAMP);
+SHOW CREATE TABLE t1;
+DROP TABLE t1;
+
+CREATE TABLE t1 (a TIMESTAMP);
+SHOW CREATE TABLE t1;
+ALTER TABLE t1 MODIFY a TYPE_MYSQL_TIMESTAMP;
+SHOW CREATE TABLE t1;
+DROP TABLE t1;

plugin/type_mysql_timestamp/mysql-test/type_mysql_timestamp/type_mysql_timestamp_stat_tables.result

+#
+# MDEV-30483 After upgrade to 10.6 from Mysql 5.7 seeing "InnoDB: Column last_update in table mysql.innodb_table_stats is BINARY(4) NOT NULL but should be INT UNSIGNED NOT NULL"
+#
+SET @@global.innodb_stats_persistent=0;
+SHOW CREATE TABLE mysql.innodb_table_stats;
+Table	Create Table
+innodb_table_stats	CREATE TABLE `innodb_table_stats` (
+  `database_name` varchar(64) NOT NULL,
+  `table_name` varchar(199) NOT NULL,
+  `last_update` timestamp NOT NULL DEFAULT current_timestamp() ON UPDATE current_timestamp(),
+  `n_rows` bigint(20) unsigned NOT NULL,
+  `clustered_index_size` bigint(20) unsigned NOT NULL,
+  `sum_of_other_index_sizes` bigint(20) unsigned NOT NULL,
+  PRIMARY KEY (`database_name`,`table_name`)
+) ENGINE=InnoDB DEFAULT CHARSET=utf8mb3 COLLATE=utf8mb3_bin STATS_PERSISTENT=0
+ALTER TABLE mysql.innodb_table_stats MODIFY last_update TYPE_MYSQL_TIMESTAMP;
+SHOW CREATE TABLE mysql.innodb_table_stats;
+Table	Create Table
+innodb_table_stats	CREATE TABLE `innodb_table_stats` (
+  `database_name` varchar(64) NOT NULL,
+  `table_name` varchar(199) NOT NULL,
+  `last_update` type_mysql_timestamp NOT NULL DEFAULT current_timestamp() ON UPDATE current_timestamp(),
+  `n_rows` bigint(20) unsigned NOT NULL,
+  `clustered_index_size` bigint(20) unsigned NOT NULL,
+  `sum_of_other_index_sizes` bigint(20) unsigned NOT NULL,
+  PRIMARY KEY (`database_name`,`table_name`)
+) ENGINE=InnoDB DEFAULT CHARSET=utf8mb3 COLLATE=utf8mb3_bin STATS_PERSISTENT=0
+ALTER TABLE mysql.innodb_index_stats MODIFY last_update TYPE_MYSQL_TIMESTAMP;
+SHOW CREATE TABLE mysql.innodb_index_stats;
+Table	Create Table
+innodb_index_stats	CREATE TABLE `innodb_index_stats` (
+  `database_name` varchar(64) NOT NULL,
+  `table_name` varchar(199) NOT NULL,
+  `index_name` varchar(64) NOT NULL,
+  `last_update` type_mysql_timestamp NOT NULL DEFAULT current_timestamp() ON UPDATE current_timestamp(),
+  `stat_name` varchar(64) NOT NULL,
+  `stat_value` bigint(20) unsigned NOT NULL,
+  `sample_size` bigint(20) unsigned DEFAULT NULL,
+  `stat_description` varchar(1024) NOT NULL,
+  PRIMARY KEY (`database_name`,`table_name`,`index_name`,`stat_name`)
+) ENGINE=InnoDB DEFAULT CHARSET=utf8mb3 COLLATE=utf8mb3_bin STATS_PERSISTENT=0
+SET @@global.innodb_stats_persistent=1;
+CREATE TABLE t1 (a INT, KEY(a)) ENGINE=InnoDB;
+INSERT INTO t1 VALUES (10);
+DROP TABLE t1;
+SET @@global.innodb_stats_persistent=0;
+ALTER TABLE mysql.innodb_table_stats MODIFY last_update TIMESTAMP;
+SHOW CREATE TABLE mysql.innodb_table_stats;
+Table	Create Table
+innodb_table_stats	CREATE TABLE `innodb_table_stats` (
+  `database_name` varchar(64) NOT NULL,
+  `table_name` varchar(199) NOT NULL,
+  `last_update` timestamp NOT NULL DEFAULT current_timestamp() ON UPDATE current_timestamp(),
+  `n_rows` bigint(20) unsigned NOT NULL,
+  `clustered_index_size` bigint(20) unsigned NOT NULL,
+  `sum_of_other_index_sizes` bigint(20) unsigned NOT NULL,
+  PRIMARY KEY (`database_name`,`table_name`)
+) ENGINE=InnoDB DEFAULT CHARSET=utf8mb3 COLLATE=utf8mb3_bin STATS_PERSISTENT=0
+ALTER TABLE mysql.innodb_index_stats MODIFY last_update TIMESTAMP;
+SHOW CREATE TABLE mysql.innodb_index_stats;
+Table	Create Table
+innodb_index_stats	CREATE TABLE `innodb_index_stats` (
+  `database_name` varchar(64) NOT NULL,
+  `table_name` varchar(199) NOT NULL,
+  `index_name` varchar(64) NOT NULL,
+  `last_update` timestamp NOT NULL DEFAULT current_timestamp() ON UPDATE current_timestamp(),
+  `stat_name` varchar(64) NOT NULL,
+  `stat_value` bigint(20) unsigned NOT NULL,
+  `sample_size` bigint(20) unsigned DEFAULT NULL,
+  `stat_description` varchar(1024) NOT NULL,
+  PRIMARY KEY (`database_name`,`table_name`,`index_name`,`stat_name`)
+) ENGINE=InnoDB DEFAULT CHARSET=utf8mb3 COLLATE=utf8mb3_bin STATS_PERSISTENT=0
+SET @@global.innodb_stats_persistent=1;

plugin/type_mysql_timestamp/mysql-test/type_mysql_timestamp/type_mysql_timestamp_stat_tables.test

+--source include/have_innodb.inc
+
+--echo #
+--echo # MDEV-30483 After upgrade to 10.6 from Mysql 5.7 seeing "InnoDB: Column last_update in table mysql.innodb_table_stats is BINARY(4) NOT NULL but should be INT UNSIGNED NOT NULL"
+--echo #
+
+SET @@global.innodb_stats_persistent=0;
+SHOW CREATE TABLE mysql.innodb_table_stats;
+ALTER TABLE mysql.innodb_table_stats MODIFY last_update TYPE_MYSQL_TIMESTAMP;
+SHOW CREATE TABLE mysql.innodb_table_stats;
+ALTER TABLE mysql.innodb_index_stats MODIFY last_update TYPE_MYSQL_TIMESTAMP;
+SHOW CREATE TABLE mysql.innodb_index_stats;
+SET @@global.innodb_stats_persistent=1;
+
+CREATE TABLE t1 (a INT, KEY(a)) ENGINE=InnoDB;
+INSERT INTO t1 VALUES (10);
+DROP TABLE t1;
+
+SET @@global.innodb_stats_persistent=0;
+ALTER TABLE mysql.innodb_table_stats MODIFY last_update TIMESTAMP;
+SHOW CREATE TABLE mysql.innodb_table_stats;
+ALTER TABLE mysql.innodb_index_stats MODIFY last_update TIMESTAMP;
+SHOW CREATE TABLE mysql.innodb_index_stats;
+SET @@global.innodb_stats_persistent=1;

plugin/type_mysql_timestamp/plugin.cc

+/*
+   Copyright (c) 2023, MariaDB Corporation
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; version 2 of the License.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program; if not, write to the Free Software
+   Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA */
+
+#include <my_global.h>
+#include <sql_class.h>
+#include <mysql/plugin_data_type.h>
+#include "sql_type.h"
+
+
+class Type_collection_local: public Type_collection
+{
+protected:
+  const Type_handler *aggregate_common(const Type_handler *h1,
+                                       const Type_handler *h2) const
+  {
+    if (h1 == h2)
+      return h1;
+    return NULL;
+  }
+public:
+  const Type_handler *handler_by_name(const LEX_CSTRING &name) const override
+  {
+    return NULL;
+  }
+
+  const Type_handler *aggregate_for_result(const Type_handler *h1,
+                                           const Type_handler *h2)
+                                           const override
+  {
+    return aggregate_common(h1, h2);
+  }
+
+  const Type_handler *aggregate_for_comparison(const Type_handler *h1,
+                                               const Type_handler *h2)
+                                               const override
+  {
+    return aggregate_common(h1, h2);
+  }
+
+  const Type_handler *aggregate_for_min_max(const Type_handler *h1,
+                                            const Type_handler *h2)
+                                            const override
+  {
+    return aggregate_common(h1, h2);
+  }
+
+  const Type_handler *aggregate_for_num_op(const Type_handler *h1,
+                                           const Type_handler *h2)
+                                           const override
+  {
+    return aggregate_common(h1, h2);
+  }
+};
+
+
+static Type_collection_local type_collection_local;
+
+
+/*
+  A more MySQL compatible Field:
+  it does not set the UNSIGNED_FLAG.
+  This is how MySQL's Field_timestampf works.
+*/
+class Field_mysql_timestampf :public Field_timestampf
+{
+public:
+  Field_mysql_timestampf(const LEX_CSTRING &name,
+                         const Record_addr &addr,
+                         enum utype unireg_check_arg,
+                         TABLE_SHARE *share, decimal_digits_t dec_arg)
+   :Field_timestampf(addr.ptr(), addr.null_ptr(), addr.null_bit(),
+                     unireg_check_arg, &name, share, dec_arg)
+  {
+    flags&= ~UNSIGNED_FLAG; // MySQL compatibility
+  }
+  void sql_type(String &str) const override
+  {
+    sql_type_opt_dec_comment(str,
+                             Field_mysql_timestampf::type_handler()->name(),
+                             dec, type_version_mysql56());
+  }
+  const Type_handler *type_handler() const override;
+};
+
+
+class Type_handler_mysql_timestamp2: public Type_handler_timestamp2
+{
+public:
+  const Type_collection *type_collection() const override
+  {
+    return &type_collection_local;
+  }
+  Field *make_table_field_from_def(TABLE_SHARE *share, MEM_ROOT *root,
+                                   const LEX_CSTRING *name,
+                                   const Record_addr &rec, const Bit_addr &bit,
+                                   const Column_definition_attributes *attr,
+                                   uint32 flags) const override
+  {
+    return new (root)
+      Field_mysql_timestampf(*name, rec, attr->unireg_check, share,
+                             attr->temporal_dec(MAX_DATETIME_WIDTH));
+  }
+  void Column_definition_implicit_upgrade(Column_definition *c) const override
+  {
+    /*
+      Suppress the automatic upgrade depending on opt_mysql56_temporal_format,
+      derived from Type_handler_timestamp_common.
+    */
+  }
+};
+
+
+static Type_handler_mysql_timestamp2 type_handler_mysql_timestamp2;
+
+
+const Type_handler *Field_mysql_timestampf::type_handler() const
+{
+  return &type_handler_mysql_timestamp2;
+}
+
+
+static struct st_mariadb_data_type plugin_descriptor_type_mysql_timestamp=
+{
+  MariaDB_DATA_TYPE_INTERFACE_VERSION,
+  &type_handler_mysql_timestamp2
+};
+
+
+
+/*************************************************************************/
+
+maria_declare_plugin(type_mysql_timestamp)
+{
+  MariaDB_DATA_TYPE_PLUGIN,     // the plugin type (see include/mysql/plugin.h)
+  &plugin_descriptor_type_mysql_timestamp, // pointer to type-specific plugin descriptor
+  "type_mysql_timestamp",       // plugin name
+  "MariaDB Corporation",        // plugin author
+  "Data type TYPE_MYSQL_TIMESTAMP", // the plugin description
+  PLUGIN_LICENSE_GPL,           // the plugin license (see include/mysql/plugin.h)
+  0,                            // Pointer to plugin initialization function
+  0,                            // Pointer to plugin deinitialization function
+  0x0100,                       // Numeric version 0xAABB means AA.BB version
+  NULL,                         // Status variables
+  NULL,                         // System variables
+  "1.0",                        // String version representation
+  MariaDB_PLUGIN_MATURITY_EXPERIMENTAL // Maturity(see include/mysql/plugin.h)*/
+}
+maria_declare_plugin_end;

sql/field.h

@@ -3336,7 +3336,7 @@ class Field_timestamp_hires :public Field_timestamp_with_dec {
 /**
   TIMESTAMP(0..6) - MySQL56 version
 */
-class Field_timestampf final :public Field_timestamp_with_dec {
+class Field_timestampf :public Field_timestamp_with_dec {
   void store_TIMEVAL(const timeval &tv) override;
 public:
   Field_timestampf(uchar *ptr_arg,

sql/sql_type.h

@@ -7593,8 +7593,9 @@ extern Named_type_handler<Type_handler_time>        type_handler_time;
 extern Named_type_handler<Type_handler_time2>       type_handler_time2;
 extern Named_type_handler<Type_handler_datetime>    type_handler_datetime;
 extern Named_type_handler<Type_handler_datetime2>   type_handler_datetime2;
-extern Named_type_handler<Type_handler_timestamp>   type_handler_timestamp;
-extern Named_type_handler<Type_handler_timestamp2>  type_handler_timestamp2;
+
+extern MYSQL_PLUGIN_IMPORT Named_type_handler<Type_handler_timestamp>   type_handler_timestamp;
+extern MYSQL_PLUGIN_IMPORT Named_type_handler<Type_handler_timestamp2>  type_handler_timestamp2;
 
 extern Type_handler_interval_DDhhmmssff type_handler_interval_DDhhmmssff;
 

storage/innobase/dict/dict0stats.cc

@@ -204,7 +204,17 @@ static const dict_table_schema_t table_stats_schema =
   {
     {"database_name", DATA_VARMYSQL, DATA_NOT_NULL, 192},
     {"table_name", DATA_VARMYSQL, DATA_NOT_NULL, 597},
-    {"last_update", DATA_INT, DATA_NOT_NULL | DATA_UNSIGNED, 4},
+    /*
+      Don't check the DATA_UNSIGNED flag in last_update.
+      It presents if the server is running in a pure MariaDB installation,
+      because MariaDB's Field_timestampf::flags has UNSIGNED_FLAG.
+      But DATA_UNSIGNED misses when the server starts on a MySQL-5.7 directory
+      (during a migration), because MySQL's Field_timestampf::flags does not
+      have UNSIGNED_FLAG.
+      This is fine not to check DATA_UNSIGNED, because Field_timestampf
+      in both MariaDB and MySQL support only non-negative time_t values.
+    */
+    {"last_update", DATA_INT, DATA_NOT_NULL, 4},
     {"n_rows", DATA_INT, DATA_NOT_NULL | DATA_UNSIGNED, 8},
     {"clustered_index_size", DATA_INT, DATA_NOT_NULL | DATA_UNSIGNED, 8},
     {"sum_of_other_index_sizes", DATA_INT, DATA_NOT_NULL | DATA_UNSIGNED, 8},
@@ -218,7 +228,11 @@ static const dict_table_schema_t index_stats_schema =
     {"database_name", DATA_VARMYSQL, DATA_NOT_NULL, 192},
     {"table_name", DATA_VARMYSQL, DATA_NOT_NULL, 597},
     {"index_name", DATA_VARMYSQL, DATA_NOT_NULL, 192},
-    {"last_update", DATA_INT, DATA_NOT_NULL | DATA_UNSIGNED, 4},
+    /*
+      Don't check the DATA_UNSIGNED flag in last_update.
+      See comments about last_update in table_stats_schema above.
+    */
+    {"last_update", DATA_INT, DATA_NOT_NULL, 4},
     {"stat_name", DATA_VARMYSQL, DATA_NOT_NULL, 64*3},
     {"stat_value", DATA_INT, DATA_NOT_NULL | DATA_UNSIGNED, 8},
     {"sample_size", DATA_INT, DATA_UNSIGNED, 8},