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


Follow ups

References