maria-developers team mailing list archive
-
maria-developers team
-
Mailing list archive
-
Message #12328
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
message:
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.
+CREATE TABLE t1 (a MARIADB_SCHEMA.date);
+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);
+DROP TABLE t1;
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,
set_handler(type.type_handler());
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()
good
+#else
+ /*
+ 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;
+#endif
+
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);
+#else
+ my_printf_error(ER_UNKNOWN_ERROR, "Unknown data type: '%-.64s'",
+ MYF(0), buf);
+#endif
+ 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
expensive.
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);
field->sql_type(type);
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?
Thanks.
Regards,
Sergei
VP of MariaDB Server Engineering
and security@xxxxxxxxxxx
Follow ups
References
