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

31 Jul 2020

Hi Sergei,


The updated patch is in bb-10.3-bar on buildbot, here:

https://github.com/MariaDB/server/commit/59730a09b7bea68f28841cc4b1973038a45...


I did the following:


1. Added a comment in Schema::eq_name(), about case sensitivity
2. Removed the unnecessary condition in LEX::map_data_type()

- if (mapped != type->type_handler())
-   type->set_handler(mapped);
+ type->set_handler(mapped);


3. Created  MDEV-23353 Qualified data types in SP
   and mentioned it in the test



I also tried your proposal with testing
   lex->parsing_options.lookup_keywords_after_qualifier
in a different place. It crashed on assert:

sql_lex.cc:2108: int Lex_input_stream::scan_ident_middle(THD*, 
Lex_ident_cli_st*, CHARSET_INFO**, my_lex_states*): Assertion `m_ptr == 
m_tok_start + 1' failed.

because we have an asymmetry in the parser:
Some scan_ident_xxx() require m_ptr == m_tok_start + 1
Other scan_ident_xxx() require m_ptr == m_tok_start

So some additional changes will be needed, and it will be more complex 
at the end. I suggest to keep it as is.

Thanks.


On 7/31/20 1:38 AM, Sergei Golubchik wrote:
...
Hi, Alexander!
On Jul 29, Alexander Barkov wrote:
...
...
On Jul 28, Alexander Barkov wrote:
...
revision-id: ca7e6f8d572 (mariadb-10.3.21-178-gca7e6f8d572)
parent(s): f3f23b5c4bd
author: Alexander Barkov 
committer: Alexander Barkov 
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/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?
Yes, please.
Just don't want it to be forgotten
...
...
...
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.
may be a simpler patch can do? like this:
case MY_LEX_IDENT_SEP:                  // Found ident and now '.'
       yylval->lex_str.str= (char*) get_ptr();
       yylval->lex_str.length= 1;
       c= yyGet();                          // should be '.'
       next_state= MY_LEX_IDENT_START;      // Next is ident (not keyword)
+     if (lex->parsing_options.lookup_keywords_after_qualifier)
+       next_state= MY_LEX_IDENT;
       if (!ident_map[(uchar) yyPeek()])    // Probably ` or "
         next_state= MY_LEX_START;
...
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;
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".
Hmm, indeed. Sorry. I thought you have oracle_schema and maxdb_schema
appearing in SHOW CREATE TABLE output in your tests.
But now I've looked again and indeed, you only have mariadb_schema in
the output.
...
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?
ok, as it's fixed in 10.5, then, I agree, not worth the efforts.
Regards,
Sergei
VP of MariaDB Server Engineering
and security@mariadb.org


    