← Back to team overview

maria-developers team mailing list archive

Re: ca7e6f8d572: MDEV-19632 Replication aborts with ER_SLAVE_CONVERSION_FAILED upon CREATE ... SELECT in ORACLE mode


Hi Sergei,

Thanks for review. Please find comments inline:

On 7/28/20 6:02 PM, Sergei Golubchik wrote:
Hi, Alexander!

On Jul 28, Alexander Barkov wrote:
revision-id: ca7e6f8d572 (mariadb-10.3.21-178-gca7e6f8d572)
parent(s): f3f23b5c4bd
author: Alexander Barkov <bar@xxxxxxxxxxx>
committer: Alexander Barkov <bar@xxxxxxxxxxx>
timestamp: 2020-07-08 12:37:17 +0400

MDEV-19632 Replication aborts with ER_SLAVE_CONVERSION_FAILED upon CREATE ... SELECT in ORACLE mode

diff --git a/mysql-test/main/lowercase_fs_off.result b/mysql-test/main/lowercase_fs_off.result
index f2a8ec14641..3f0b08a78c4 100644
--- a/mysql-test/main/lowercase_fs_off.result
+++ b/mysql-test/main/lowercase_fs_off.result
@@ -158,3 +158,13 @@ show triggers like '%T1%';
  Trigger	Event	Table	Statement	Timing	Created	sql_mode	Definer	character_set_client	collation_connection	Database Collation
  drop table t1;
  set GLOBAL sql_mode=default;
+# MDEV-19632 Replication aborts with ER_SLAVE_CONVERSION_FAILED upon CREATE ... SELECT in ORACLE mode
+# Compatibility schema names respect the filesystem case sensitivity

I agree, it should. But this is not consistent with information_schema.
So I'd suggest a comment in eq_name method, explaining why compatibility
schema names respect the filesystem case sensitivity. (because, e.g., we
eventually turn them into real schemas, on disk).

Good idea. Will do.

+ERROR HY000: Unknown data type: 'MARIADB_SCHEMA.date'
+CREATE TABLE t1 (a Mariadb_schema.date);
+ERROR HY000: Unknown data type: 'Mariadb_schema.date'
+CREATE TABLE t1 (a mariadb_schema.date);
diff --git a/mysql-test/suite/compat/oracle/t/type_date.test b/mysql-test/suite/compat/oracle/t/type_date.test
index 61f7aa53944..34adc5889b4 100644
--- a/mysql-test/suite/compat/oracle/t/type_date.test
+++ b/mysql-test/suite/compat/oracle/t/type_date.test
@@ -2,3 +2,101 @@ SET sql_mode=ORACLE;
+--echo #
+--echo # Qualified syntax is not supported yet in SP
+--echo #

an MDEV for that?

There is no one yet.
Would you like me to create one and mentions it number here?

diff --git a/sql/field.cc b/sql/field.cc
index 8accfb35b0b..4ed38f2c06f 100644
--- a/sql/field.cc
+++ b/sql/field.cc
@@ -10407,6 +10407,19 @@ void Column_definition::set_attributes(const Lex_field_type_st &type,
    charset= cs;
+#if MYSQL_VERSION_ID > 100500
+#error When merging to 10.5, please move the code below to
+#error Type_handler_timestamp_common::Column_definition_set_attributes()


+  /*
+    Unlike other types TIMESTAMP fields are NOT NULL by default.
+    Unless --explicit-defaults-for-timestamp is given.
+  */
+  if (!opt_explicit_defaults_for_timestamp &&
+      type.type_handler()->field_type() == MYSQL_TYPE_TIMESTAMP)
+    flags|= NOT_NULL_FLAG;
    if (type.length())
      int err;
diff --git a/sql/sql_lex.cc b/sql/sql_lex.cc
index 0b423355170..3fb07601f99 100644
--- a/sql/sql_lex.cc
+++ b/sql/sql_lex.cc
@@ -2080,6 +2081,13 @@ int Lex_input_stream::scan_ident_start(THD *thd, Lex_ident_cli_st *str)
uint length= yyLength();
    yyUnget(); // ptr points now after last token char
+  if (thd->lex->parsing_options.lookup_keywords_after_qualifier)
+  {
+    if (int tokval= find_keyword(str, length, 0))
+      return tokval;
+  }

why is that?

This is because our parser has a hack to avoid looking up keywords near a dot.

Here we need to lookup to make all field_type_xxx rules work with the qualified syntax.

Otherwise mariadb_schema.datetime(6) won't parse because
in this rule:

        | DATETIME opt_field_length

the token will be IDENT instead of DATETIME.

So in this patch I allow looking up keywords after the dot
inside the data type grammar.

Btw, I suggest to remove this hack eventually.
So keywords are always looked up.

    str->set_ident(m_tok_start, length, is_8bit);
    m_cpp_text_start= m_cpp_tok_start;
    m_cpp_text_end= m_cpp_text_start + length;
@@ -8284,3 +8292,32 @@ bool LEX::tvc_finalize_derived()
    current_select->linkage= DERIVED_TABLE_TYPE;
    return tvc_finalize();
+bool LEX::map_data_type(const Lex_ident_sys_st &schema_name,
+                        Lex_field_type_st *type) const
+  const Schema *schema= schema_name.str ?
+                        Schema::find_by_name(schema_name) :
+                        Schema::find_implied(thd);
+  if (!schema)
+  {
+    char buf[128];
+    const Name type_name= type->type_handler()->name();
+    my_snprintf(buf, sizeof(buf), "%.*s.%.*s",
+                (int) schema_name.length, schema_name.str,
+                (int) type_name.length(), type_name.ptr());
+#if MYSQL_VERSION_ID > 100500
+#error Please remove the old code
+    my_error(ER_UNKNOWN_DATA_TYPE, MYF(0), buf);
+    my_printf_error(ER_UNKNOWN_ERROR, "Unknown data type: '%-.64s'",
+                    MYF(0), buf);
+    return true;
+  }
+  const Type_handler *mapped= schema->map_data_type(thd, type->type_handler());
+  if (mapped != type->type_handler())
+    type->set_handler(mapped);

why do you need an if() here? type->set_handler(mapped) is not that

Will fix.

+  return false;
diff --git a/sql/sql_show.cc b/sql/sql_show.cc
index 3dbc7724928..fcc2d44fdd6 100644
--- a/sql/sql_show.cc
+++ b/sql/sql_show.cc
@@ -2224,6 +2224,13 @@ int show_create_table(THD *thd, TABLE_LIST *table_list, String *packet,
      append_identifier(thd, packet, &field->field_name);
      packet->append(' ');
+ const Type_handler *th= field->type_handler();
+    const Schema *implied_schema= Schema::find_implied(thd);
+    if (th != implied_schema->map_data_type(thd, th))
+    {
+      packet->append(th->schema()->name(), system_charset_info);

sorry, I don't see it. How can th->schema() return not mariadb_schema?

Currently th->schema() cannot return not "mariadb_schema".

But the intent is to have schema specific data types in the future.

For example, Oracle's DATE may become a real separate data type
(with Type_handler_date_oracle, Field_date_oracle, all the other stuff)
in the future, instead of just an alias for MariaDB's DATETIME.

So this code just takes this into account.

+      packet->append(STRING_WITH_LEN("."), system_charset_info);
+    }
      type.set(tmp, sizeof(tmp), system_charset_info);
      packet->append(type.ptr(), type.length(), system_charset_info);
diff --git a/storage/test_sql_discovery/mysql-test/sql_discovery/simple.result b/storage/test_sql_discovery/mysql-test/sql_discovery/simple.result
index 23b7804638f..1feea5e47ee 100644
--- a/storage/test_sql_discovery/mysql-test/sql_discovery/simple.result
+++ b/storage/test_sql_discovery/mysql-test/sql_discovery/simple.result
@@ -82,7 +82,7 @@ select * from t1;
  ERROR HY000: Engine TEST_SQL_DISCOVERY failed to discover table `test`.`t1` with 'create table t1 (a uint)'
  show warnings;
  Level	Code	Message
-Error	1064	You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near 'uint)' at line 1
+Error	1064	You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near ')' at line 1

this is not ideal :(
can you try to keep the old error message?

I'm afraid this is not easy. The error happens inside bison.
'uint' is now a known data type, so it tries to parse it
as a schema qualified data type,
but there is no a dot after the schema name 'uint'.
So the error is now quite correct.

Note, starting from 10.5 it works as follows:

MariaDB [test]> create table t1 (a uint);
ERROR 4161 (HY000): Unknown data type: 'uint'

So probably not worthy efforts.

What do you think?


VP of MariaDB Server Engineering
and security@xxxxxxxxxxx

Follow ups