maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

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).

> +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?

> 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?

> +
>    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.

> +  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?

> +      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?

Regards,
Sergei
VP of MariaDB Server Engineering
and security@xxxxxxxxxxx