← 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,


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

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


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 <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/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@xxxxxxxxxxx



References