Hi, Alexander,
Few questions and suggestions below.
Nothing major.
On Oct 27, Alexander Barkov wrote:
> commit 2d937b62c33
> Author: Alexander Barkov <b...@mariadb.com>
> Date: Mon Apr 4 14:50:21 2022 +0400
>
> MDEV-27744 InnoDB: Failing assertion: !cursor->index->is_committed() in
> row0ins.cc (from row_ins_sec_index_entry_by_modify) | Assertion `0' failed in
> row_upd_sec_index_entry (debug) | Corruption
>
would it make sense to change the MDEV title in Jira to better
describe the problem? I sometimes do it with my bugs.
Like "LPAD in vcol created in ORACLE mode makes table corrupted in non-ORACLE"
(I tried to make it short, appropriate for a title)
and, of course, change it in the comment and in the test file to match.
> The crash happened with an indexed virtual column whose
> value is evaluated using a function that has a different meaning
> in sql_mode='' vs sql_mode=ORACLE:
>
> diff --git a/mysql-test/suite/compat/oracle/r/func_concat.result
> b/mysql-test/suite/compat/oracle/r/func_concat.result
> index 392d579707a..17ca4be078a 100644
> --- a/mysql-test/suite/compat/oracle/r/func_concat.result
> +++ b/mysql-test/suite/compat/oracle/r/func_concat.result
> @@ -211,14 +211,14 @@ SET sql_mode=ORACLE;
> CREATE VIEW v1 AS SELECT 'foo'||NULL||'bar' AS test;
> SHOW CREATE VIEW v1;
> View Create View character_set_client collation_connection
> -v1 CREATE VIEW "v1" AS select
> concat_operator_oracle(concat_operator_oracle('foo',NULL),'bar') AS "test"
> latin1 latin1_swedish_ci
> +v1 CREATE VIEW "v1" AS select concat(concat('foo',NULL),'bar') AS "test"
> latin1 latin1_swedish_ci
> SELECT * FROM v1;
> test
> foobar
> SET sql_mode=DEFAULT;
> SHOW CREATE VIEW v1;
> View Create View character_set_client collation_connection
> -v1 CREATE ALGORITHM=UNDEFINED DEFINER=`root`@`localhost` SQL SECURITY
> DEFINER VIEW `v1` AS select
> concat_operator_oracle(concat_operator_oracle('foo',NULL),'bar') AS `test`
> latin1 latin1_swedish_ci
> +v1 CREATE ALGORITHM=UNDEFINED DEFINER=`root`@`localhost` SQL SECURITY
> DEFINER VIEW `v1` AS select
> oracle_schema.concat(oracle_schema.concat('foo',NULL),'bar') AS `test`
> latin1 latin1_swedish_ci
please, add a mysqldump test. Like, create a table
with virtual columns, check, defaults in the default sql mode.
and create a table in oracle mode. Also, a view in the default mode and
a view in oracle mode.
and then mysqldump, to see that it dumps and restores everything correctly
may be stored routines/triggers/etc, if you'd like, but
they aren't directly relevant to this MDEV, as far as I understand.
> SELECT * FROM v1;
> test
> foobar
> diff --git a/mysql-test/suite/compat/oracle/r/func_decode.result
> b/mysql-test/suite/compat/oracle/r/func_decode.result
> index 2809e971be3..1870a1ec0d5 100644
> --- a/mysql-test/suite/compat/oracle/r/func_decode.result
> +++ b/mysql-test/suite/compat/oracle/r/func_decode.result
> @@ -1,8 +1,8 @@
> SET sql_mode=ORACLE;
> SELECT DECODE(10);
> -ERROR 42000: Incorrect parameter count in the call to native function
> 'DECODE'
> +ERROR 42000: Incorrect parameter count in the call to native function
> 'oracle_schema.DECODE'
Hmm, may be this should say DECODE as before?
you know, falls under the case "if you don't change sql_mode back and
forth, you won't see schema-qualified names"
> SELECT DECODE(10,10);
> -ERROR 42000: Incorrect parameter count in the call to native function
> 'DECODE'
> +ERROR 42000: Incorrect parameter count in the call to native function
> 'oracle_schema.DECODE'
> SELECT DECODE(10,10,'x10');
> DECODE(10,10,'x10')
> x10
> diff --git a/mysql-test/suite/compat/oracle/r/vcol_innodb.result
> b/mysql-test/suite/compat/oracle/r/vcol_innodb.result
> new file mode 100644
> index 00000000000..9fa97c75c10
> --- /dev/null
> +++ b/mysql-test/suite/compat/oracle/r/vcol_innodb.result
> @@ -0,0 +1,51 @@
> +SET @table_open_cache=@@GLOBAL.table_open_cache;
why do you need to manipulate table_open_cache?
to trigger a reopen? Just do flush tables, it's explicit, more readable
and more... controllable.
> +SET sql_mode='';
> +CREATE TABLE t (d INT,b VARCHAR(1),c CHAR(1),g CHAR(1) GENERATED ALWAYS AS
> (SUBSTR(b,0,0)) VIRTUAL,PRIMARY KEY(b),KEY g(g)) ENGINE=InnoDB;
> +INSERT INTO t VALUES (0);
> +ERROR 21S01: Column count doesn't match value count at row 1
> +SET sql_mode='ORACLE';
> +INSERT INTO t SET c=REPEAT (1,0);
> +Warnings:
> +Warning 1364 Field 'b' doesn't have a default value
> +ALTER TABLE t CHANGE COLUMN a b INT;
> diff --git a/sql/item_func.h b/sql/item_func.h
> index 76a997c33fb..cdbefb82541 100644
> --- a/sql/item_func.h
> +++ b/sql/item_func.h
> @@ -56,8 +56,40 @@ class Item_func :public Item_func_or_sum,
> bool check_argument_types_can_return_date(uint start, uint end) const;
> bool check_argument_types_can_return_time(uint start, uint end) const;
> void print_cast_temporal(String *str, enum_query_type query_type);
> +
> + void print_schema_qualified_name(String *to,
> + const LEX_CSTRING &schema_name,
> + const char *function_name) const
I don't see why you'd need this helper.
is it something that was used in earlier versions of the patch?
> + {
> + // e.g. oracle_schema.func()
> + to->append(schema_name);
> + to->append('.');
> + to->append(function_name);
> + }
> +
> + void print_sql_mode_qualified_name(String *to,
> + enum_query_type query_type,
> + const char *function_name) const
> + {
> + const Schema *func_schema= schema();
> + if (!func_schema || func_schema == Schema::find_implied(current_thd))
> + to->append(function_name);
> + else
> + print_schema_qualified_name(to, func_schema->name(), function_name);
> + }
> +
> + void print_sql_mode_qualified_name(String *to, enum_query_type query_type)
> + const
> + {
> + return print_sql_mode_qualified_name(to, query_type, func_name());
> + }
I don't see why you need this helper either, you never use
print_sql_mode_qualified_name with the last argument being not func_name().
So you can remove this helper and the third argument of
print_sql_mode_qualified_name.
> +
> public:
>
> + // Print an error message for a builtin-schema qualified function call
> + static void wrong_param_count_error(const LEX_CSTRING &schema_name,
> + const LEX_CSTRING &func_name);
> +
> table_map not_null_tables_cache;
>
> enum Functype { UNKNOWN_FUNC,EQ_FUNC,EQUAL_FUNC,NE_FUNC,LT_FUNC,LE_FUNC,
> diff --git a/sql/item_strfunc.cc b/sql/item_strfunc.cc
> index ae078dbb22f..92d5e196da4 100644
> --- a/sql/item_strfunc.cc
> +++ b/sql/item_strfunc.cc
> @@ -2170,13 +2170,31 @@ bool Item_func_trim::fix_length_and_dec()
>
> void Item_func_trim::print(String *str, enum_query_type query_type)
> {
> + LEX_CSTRING suffix= {STRING_WITH_LEN("_oracle")};
> if (arg_count == 1)
> {
> - Item_func::print(str, query_type);
> + if (query_type & QT_FOR_FRM)
> + {
> + // 10.3 downgrade compatibility for FRM
> + str->append(func_name());
> + if (schema() == &oracle_schema_ref)
> + str->append(suffix);
> + }
> + else
> + print_sql_mode_qualified_name(str, query_type, func_name());
> + print_args_parenthesized(str, query_type);
> return;
> }
> - str->append(Item_func_trim::func_name());
> - str->append(func_name_ext());
> +
> + if (query_type & QT_FOR_FRM)
> + {
> + // 10.3 downgrade compatibility for FRM
> + str->append(Item_func_trim::func_name());
> + if (schema() == &oracle_schema_ref)
> + str->append(suffix);
> + }
> + else
> + print_sql_mode_qualified_name(str, query_type,
> Item_func_trim::func_name());
it'd be simpler if you move the above block that prints the
function name before if (arg_count == 1)
also you won't need suffix, but can do like in all other functions
str->append(STRING_WITH_LEN("trim_oracle");
> str->append('(');
> str->append(mode_name());
> str->append(' ');
> diff --git a/sql/sql_lex.cc b/sql/sql_lex.cc
> index 71f592a3852..bb53d1a510a 100644
> --- a/sql/sql_lex.cc
> +++ b/sql/sql_lex.cc
> @@ -2084,7 +2100,64 @@ bool Lex_input_stream::get_7bit_or_8bit_ident(THD
> *thd, uchar *last_char)
> }
>
>
> -int Lex_input_stream::scan_ident_sysvar(THD *thd, Lex_ident_cli_st *str)
> +/*
> + Resolve special SQL functions that have a qualified syntax in sql_yacc.yy.
> + These functions are not listed in the native function registry
> + because of a special syntax, or a reserved keyword:
> +
> + mariadb_schema.SUBSTRING('a' FROM 1 FOR 2) -- Special syntax
I didn't find it in Oracle's manual, by the way
> + mariadb_schema.TRIM(BOTH ' ' FROM 'a') -- Special syntax
> + mariadb_schema.REPLACE('a','b','c') -- Verb keyword
> +*/
> +
> +int Lex_input_stream::find_keyword_qualified_special_func(Lex_ident_cli_st
> *str,
> + uint length) const
> +{
> + /*
> + There are many other special functions, see the following grammar rules:
> + function_call_keyword
> + function_call_nonkeyword
> + Here we resolve only those that have a qualified syntax to handle
> + different behavior in different @@sql_mode settings.
> +
> + Other special functions do not work in qualified context:
> + SELECT mariadb_schema.year(now()); -- Function year is not defined
> + SELECT mariadb_schema.now(); -- Function now is not defined
> +
> + We don't resolve TRIM_ORACLE here, because it does not have
> + a qualified syntax yet. Search for "trim_operands" in sql_yacc.yy
> + to find more comments.
> + */
> diff --git a/sql/sql_schema.h b/sql/sql_schema.h
> index 1174bc7a83f..2c52646f2ea 100644
> --- a/sql/sql_schema.h
> +++ b/sql/sql_schema.h
> @@ -77,5 +98,6 @@ class Schema
>
>
> extern Schema mariadb_schema;
> +extern const Schema &oracle_schema_ref;
What's the difference between these two definitions.
Do you expect someone will need to change mariadb_schema?
>
> #endif // SQL_SCHEMA_H_INCLUDED
Regards,
Sergei
Chief Architect, MariaDB Server
and secur...@mariadb.org
_______________________________________________
developers mailing list -- developers@lists.mariadb.org
To unsubscribe send an email to developers-le...@lists.mariadb.org
