Hi Sergei, Thanks for your review. I've addressed your suggestions. Please find a newer version here: https://github.com/MariaDB/server/commit/e4415549e5374f6fcb5d317a7078ead610a... Thanks. On 8/28/19 4:47 PM, Sergei Golubchik wrote:

Hi, Alexander!

On Aug 28, Alexander Barkov wrote:

...

revision-id: 5f1cd8fe7a5 (mariadb-10.2.26-50-g5f1cd8fe7a5) parent(s): 9de2e60d749 author: Alexander Barkov committer: Alexander Barkov timestamp: 2019-08-26 15:28:32 +0400 message:

MDEV-18156 Assertion `0' failed or `btr_validate_index(index, 0, false)' in row_upd_sec_index_entry or error code 126: Index is corrupted upon DELETE with PAD_CHAR_TO_FULL_LENGTH

This is not a trivial one-liner fix. Before pushing, please add a comment explaning the problem and your fix.

...

diff --git a/mysql-test/suite/vcol/r/vcol_sql_mode.result b/mysql-test/suite/vcol/r/vcol_sql_mode.result new file mode 100644 index 00000000000..6af46dffb80 --- /dev/null +++ b/mysql-test/suite/vcol/r/vcol_sql_mode.result @@ -0,0 +1,234 @@ +# +# Start of 10.2 tests +# +# +# MDEV-18156 Assertion `0' failed or `btr_validate_index(index, 0, false)' in row_upd_sec_index_entry or error code 126: Index is corrupted upon DELETE with PAD_CHAR_TO_FULL_LENGTH +# +# +# PAD_CHAR_TO_FULL_LENGTH + various virtual column data types +# +CREATE TABLE t1 (a CHAR(5), v CHAR(5) AS (a) VIRTUAL, KEY(v)); +DROP TABLE t1;

please add SHOW CREATE after every CREATE. To show what automatic expression rewriting does (or does not) happen here.

...

+CREATE TABLE t1 (a CHAR(5), v INT AS (a) VIRTUAL, KEY(v)); +DROP TABLE t1; +CREATE TABLE t1 (a CHAR(5), v TIME AS (a) VIRTUAL, KEY(v)); +DROP TABLE t1; +CREATE TABLE t1 (c CHAR(8), v BINARY(8) AS (c), KEY(v)); +ERROR HY000: Function or expression '`c`' cannot be used in the GENERATED ALWAYS AS clause of `v` +SHOW WARNINGS;

Better do --enable_warnings at the beginning of the test. It'll serve two purposes - you won't need to do SHOW WARNINGS manually and it'll make clear when there are no warnings after a CREATE.

...

+Level Code Message +Error 1901 Function or expression '`c`' cannot be used in the GENERATED ALWAYS AS clause of `v` +Warning 1105 depends on system variable @@sql_mode value PAD_CHAR_TO_FULL_LENGTH

Let's make it a proper sentence: "Expression `c` implicitly depends on the @@sql_mode value PAD_CHAR_TO_FULL_LENGTH"

...

+CREATE TABLE t1 (a CHAR(5), v BIT(64) AS (a) VIRTUAL, KEY(v)); +ERROR HY000: Function or expression '`a`' cannot be used in the GENERATED ALWAYS AS clause of `v` +SHOW WARNINGS; +Level Code Message +Error 1901 Function or expression '`a`' cannot be used in the GENERATED ALWAYS AS clause of `v` +Warning 1105 depends on system variable @@sql_mode value PAD_CHAR_TO_FULL_LENGTH +CREATE TABLE t1 (a CHAR(5), v VARCHAR(5) AS (a) VIRTUAL, KEY(v)); +ERROR HY000: Function or expression '`a`' cannot be used in the GENERATED ALWAYS AS clause of `v` +SHOW WARNINGS; +Level Code Message +Error 1901 Function or expression '`a`' cannot be used in the GENERATED ALWAYS AS clause of `v` +Warning 1105 depends on system variable @@sql_mode value PAD_CHAR_TO_FULL_LENGTH +CREATE TABLE t1 (a CHAR(5), v TEXT AS (a) VIRTUAL, KEY(v(100))); +ERROR HY000: Function or expression '`a`' cannot be used in the GENERATED ALWAYS AS clause of `v` +SHOW WARNINGS; +Level Code Message +Error 1901 Function or expression '`a`' cannot be used in the GENERATED ALWAYS AS clause of `v` +Warning 1105 depends on system variable @@sql_mode value PAD_CHAR_TO_FULL_LENGTH +# PAD_CHAR_TO_FULL_LENGTH + TRIM resolving dependency +CREATE TABLE t1 (a CHAR(5), v VARCHAR(5) AS (RTRIM(a)) VIRTUAL, KEY(v)); +DROP TABLE t1; +CREATE TABLE t1 (a CHAR(5), v TEXT AS (RTRIM(a)) VIRTUAL, KEY(v(100))); +DROP TABLE t1; +CREATE TABLE t1 (a CHAR(5), v VARCHAR(5) AS (TRIM(TRAILING ' ' FROM a)) VIRTUAL, KEY(v)); +DROP TABLE t1; +CREATE TABLE t1 (a CHAR(5), v TEXT AS (TRIM(TRAILING ' ' FROM a)) VIRTUAL, KEY(v(100))); +DROP TABLE t1; +CREATE TABLE t1 (a CHAR(5), v VARCHAR(5) AS (TRIM(BOTH ' ' FROM a)) VIRTUAL, KEY(v)); +DROP TABLE t1; +CREATE TABLE t1 (a CHAR(5), v TEXT AS (TRIM(BOTH ' ' FROM a)) VIRTUAL, KEY(v(100))); +DROP TABLE t1; +CREATE TABLE t1 (a CHAR(5), v VARCHAR(5) AS (TRIM(TRAILING NULL FROM a)) VIRTUAL, KEY(v)); +DROP TABLE t1; +CREATE TABLE t1 (a CHAR(5), v VARCHAR(5) AS (TRIM(BOTH NULL FROM a)) VIRTUAL, KEY(v)); +DROP TABLE t1; +# PAD_CHAR_TO_FULL_LENGTH + TRIM not resolving dependency +CREATE TABLE t1 (a CHAR(5), v VARCHAR(5) AS (TRIM(LEADING ' ' FROM a)) VIRTUAL, KEY(v)); +ERROR HY000: Function or expression 'trim(leading ' ' from `a`)' cannot be used in the GENERATED ALWAYS AS clause of `v`

nice :)

...

+SHOW WARNINGS; +Level Code Message +Error 1901 Function or expression 'trim(leading ' ' from `a`)' cannot be used in the GENERATED ALWAYS AS clause of `v` +Warning 1105 depends on system variable @@sql_mode value PAD_CHAR_TO_FULL_LENGTH +CREATE TABLE t1 (a CHAR(5), v TEXT AS (TRIM(LEADING ' ' FROM a)) VIRTUAL, KEY(v(100))); +ERROR HY000: Function or expression 'trim(leading ' ' from `a`)' cannot be used in the GENERATED ALWAYS AS clause of `v` +SHOW WARNINGS; +Level Code Message +Error 1901 Function or expression 'trim(leading ' ' from `a`)' cannot be used in the GENERATED ALWAYS AS clause of `v` +Warning 1105 depends on system variable @@sql_mode value PAD_CHAR_TO_FULL_LENGTH +CREATE TABLE t1 (a CHAR(5), v VARCHAR(5) AS (TRIM(TRAILING '' FROM a)) VIRTUAL, KEY(v)); +ERROR HY000: Function or expression 'trim(trailing '' from `a`)' cannot be used in the GENERATED ALWAYS AS clause of `v` +SHOW WARNINGS; +Level Code Message +Error 1901 Function or expression 'trim(trailing '' from `a`)' cannot be used in the GENERATED ALWAYS AS clause of `v` +Warning 1105 depends on system variable @@sql_mode value PAD_CHAR_TO_FULL_LENGTH +CREATE TABLE t1 (a CHAR(5), v VARCHAR(5) AS (TRIM(BOTH '' FROM a)) VIRTUAL, KEY(v)); +ERROR HY000: Function or expression 'trim(both '' from `a`)' cannot be used in the GENERATED ALWAYS AS clause of `v` +SHOW WARNINGS; +Level Code Message +Error 1901 Function or expression 'trim(both '' from `a`)' cannot be used in the GENERATED ALWAYS AS clause of `v` +Warning 1105 depends on system variable @@sql_mode value PAD_CHAR_TO_FULL_LENGTH +CREATE TABLE t1 (a CHAR(5), v VARCHAR(5) AS (TRIM(TRAILING 'x' FROM a)) VIRTUAL, KEY(v)); +ERROR HY000: Function or expression 'trim(trailing 'x' from `a`)' cannot be used in the GENERATED ALWAYS AS clause of `v` +SHOW WARNINGS; +Level Code Message +Error 1901 Function or expression 'trim(trailing 'x' from `a`)' cannot be used in the GENERATED ALWAYS AS clause of `v` +Warning 1105 depends on system variable @@sql_mode value PAD_CHAR_TO_FULL_LENGTH +CREATE TABLE t1 (a CHAR(5), v VARCHAR(5) AS (TRIM(BOTH 'x' FROM a)) VIRTUAL, KEY(v)); +ERROR HY000: Function or expression 'trim(both 'x' from `a`)' cannot be used in the GENERATED ALWAYS AS clause of `v` +SHOW WARNINGS; +Level Code Message +Error 1901 Function or expression 'trim(both 'x' from `a`)' cannot be used in the GENERATED ALWAYS AS clause of `v` +Warning 1105 depends on system variable @@sql_mode value PAD_CHAR_TO_FULL_LENGTH +# PAD_CHAR_TO_FULL_LENGTH + TRIM(... non_constant FROM a) +CREATE TABLE t1 ( +a CHAR(5), +b CHAR(5), +v TEXT AS (TRIM(TRAILING b FROM a)) VIRTUAL, KEY(v(100))); +ERROR HY000: Function or expression 'trim(trailing `b` from `a`)' cannot be used in the GENERATED ALWAYS AS clause of `v` +SHOW WARNINGS; +Level Code Message +Error 1901 Function or expression 'trim(trailing `b` from `a`)' cannot be used in the GENERATED ALWAYS AS clause of `v` +Warning 1105 depends on system variable @@sql_mode value PAD_CHAR_TO_FULL_LENGTH +# PAD_CHAR_TO_FULL_LENGTH + RPAD resolving dependency +CREATE TABLE t1 (a CHAR(5), v VARCHAR(5) AS (RPAD(a,5,' ')) VIRTUAL, KEY(v)); +DROP TABLE t1; +CREATE TABLE t1 (a CHAR(5), v VARCHAR(5) AS (RPAD(a,6,' ')) VIRTUAL, KEY(v)); +DROP TABLE t1; +CREATE TABLE t1 (a CHAR(5), v VARCHAR(5) AS (RPAD(a,6,NULL)) VIRTUAL, KEY(v)); +DROP TABLE t1; +CREATE TABLE t1 (a CHAR(5), v VARCHAR(5) AS (RPAD(a,NULL,' ')) VIRTUAL, KEY(v)); +DROP TABLE t1; +# PAD_CHAR_TO_FULL_LENGTH + RPAD not resolving dependency +CREATE TABLE t1 (a CHAR(5), v VARCHAR(5) AS (RPAD(a,4,' ')) VIRTUAL, KEY(v)); +ERROR HY000: Function or expression 'rpad(`a`,4,' ')' cannot be used in the GENERATED ALWAYS AS clause of `v` +SHOW WARNINGS; +Level Code Message +Error 1901 Function or expression 'rpad(`a`,4,' ')' cannot be used in the GENERATED ALWAYS AS clause of `v` +Warning 1105 depends on system variable @@sql_mode value PAD_CHAR_TO_FULL_LENGTH +CREATE TABLE t1 ( +a CHAR(5), +b CHAR(5), +v VARCHAR(5) AS (RPAD(a,NULL,b)) VIRTUAL, +KEY(v) +); +ERROR HY000: Function or expression 'rpad(`a`,NULL,`b`)' cannot be used in the GENERATED ALWAYS AS clause of `v` +SHOW WARNINGS; +Level Code Message +Error 1901 Function or expression 'rpad(`a`,NULL,`b`)' cannot be used in the GENERATED ALWAYS AS clause of `v` +Warning 1105 depends on system variable @@sql_mode value PAD_CHAR_TO_FULL_LENGTH +# PAD_CHAR_TO_FULL_LENGTH + comparison +CREATE TABLE t1 (a CHAR(5), v INT AS (a='a') VIRTUAL, KEY(v)); +DROP TABLE t1; +CREATE TABLE t1 ( +a CHAR(5) CHARACTER SET latin1 COLLATE latin1_nopad_bin, +v INT AS (a='a') VIRTUAL, KEY(v) +); +ERROR HY000: Function or expression '`a` = 'a'' cannot be used in the GENERATED ALWAYS AS clause of `v` +SHOW WARNINGS; +Level Code Message +Error 1901 Function or expression '`a` = 'a'' cannot be used in the GENERATED ALWAYS AS clause of `v` +Warning 1105 depends on system variable @@sql_mode value PAD_CHAR_TO_FULL_LENGTH +# PAD_CHAR_TO_FULL_LENGTH + LIKE +CREATE TABLE t1 (a CHAR(5), v INT AS (a LIKE 'a%') VIRTUAL, KEY(v)); +DROP TABLE t1; +CREATE TABLE t1 (a CHAR(5), v INT AS (a LIKE NULL) VIRTUAL, KEY(v)); +DROP TABLE t1; +CREATE TABLE t1 (a CHAR(5), v INT AS (a LIKE 'a') VIRTUAL, KEY(v)); +ERROR HY000: Function or expression '`a` like 'a'' cannot be used in the GENERATED ALWAYS AS clause of `v` +SHOW WARNINGS; +Level Code Message +Error 1901 Function or expression '`a` like 'a'' cannot be used in the GENERATED ALWAYS AS clause of `v` +Warning 1105 depends on system variable @@sql_mode value PAD_CHAR_TO_FULL_LENGTH +# +# Testing NO_UNSIGNED_SUBTRACTION +# +CREATE TABLE t1 ( +a INT UNSIGNED, +b INT UNSIGNED, +c INT GENERATED ALWAYS AS (a-b) VIRTUAL, +KEY (c) +); +ERROR HY000: Function or expression '`a` - `b`' cannot be used in the GENERATED ALWAYS AS clause of `c` +SHOW WARNINGS; +Level Code Message +Error 1901 Function or expression '`a` - `b`' cannot be used in the GENERATED ALWAYS AS clause of `c` +Warning 1105 depends on system variable @@sql_mode value NO_UNSIGNED_SUBTRACTION +CREATE TABLE t1 ( +a INT UNSIGNED, +b INT UNSIGNED, +c INT GENERATED ALWAYS AS (CAST(a AS SIGNED)-b) VIRTUAL, +KEY (c) +); +ERROR HY000: Function or expression 'cast(`a` as signed) - `b`' cannot be used in the GENERATED ALWAYS AS clause of `c` +SHOW WARNINGS; +Level Code Message +Error 1901 Function or expression 'cast(`a` as signed) - `b`' cannot be used in the GENERATED ALWAYS AS clause of `c` +Warning 1105 depends on system variable @@sql_mode value NO_UNSIGNED_SUBTRACTION +CREATE TABLE t1 ( +a INT UNSIGNED, +b INT UNSIGNED, +c INT GENERATED ALWAYS AS (a-CAST(b AS SIGNED)) VIRTUAL, +KEY (c) +); +ERROR HY000: Function or expression '`a` - cast(`b` as signed)' cannot be used in the GENERATED ALWAYS AS clause of `c` +SHOW WARNINGS; +Level Code Message +Error 1901 Function or expression '`a` - cast(`b` as signed)' cannot be used in the GENERATED ALWAYS AS clause of `c` +Warning 1105 depends on system variable @@sql_mode value NO_UNSIGNED_SUBTRACTION +CREATE TABLE t1 ( +a INT UNSIGNED, +b INT UNSIGNED, +c INT GENERATED ALWAYS AS (CAST(a AS SIGNED)-CAST(b AS SIGNED)) VIRTUAL, +KEY (c) +); +DROP TABLE t1; +CREATE TABLE t1 ( +a INT UNSIGNED, +b INT UNSIGNED, +c INT GENERATED ALWAYS AS (CAST(a AS DECIMAL(20,0))-CAST(b AS DECIMAL(20,0))) VIRTUAL, +KEY (c) +); +DROP TABLE t1; +# +# Comnination: PAD_CHAR_TO_FULL_LENGTH + NO_UNSIGNED_SUBTRACTION +# +CREATE TABLE t1 ( +a INT UNSIGNED, +b INT UNSIGNED, +c CHAR(5), +v VARCHAR(5) GENERATED ALWAYS AS (RPAD(c,a-b,' ')) VIRTUAL, +KEY (v) +); +ERROR HY000: Function or expression 'rpad(`c`,`a` - `b`,' ')' cannot be used in the GENERATED ALWAYS AS clause of `v` +SHOW WARNINGS; +Level Code Message +Error 1901 Function or expression 'rpad(`c`,`a` - `b`,' ')' cannot be used in the GENERATED ALWAYS AS clause of `v` +Warning 1105 depends on system variable @@sql_mode value NO_UNSIGNED_SUBTRACTION +Warning 1105 depends on system variable @@sql_mode value PAD_CHAR_TO_FULL_LENGTH +CREATE TABLE t1 ( +a INT UNSIGNED, +b INT UNSIGNED, +c CHAR(5), +v VARCHAR(5) GENERATED ALWAYS AS (RPAD(c,CAST(a AS DECIMAL(20,1))-b,' ')) VIRTUAL, +KEY (v) +); +ERROR HY000: Function or expression 'rpad(`c`,cast(`a` as decimal(20,1)) - `b`,' ')' cannot be used in the GENERATED ALWAYS AS clause of `v` +SHOW WARNINGS; +Level Code Message +Error 1901 Function or expression 'rpad(`c`,cast(`a` as decimal(20,1)) - `b`,' ')' cannot be used in the GENERATED ALWAYS AS clause of `v` +Warning 1105 depends on system variable @@sql_mode value PAD_CHAR_TO_FULL_LENGTH +# +# End of 10.2 tests +# diff --git a/sql/field.cc b/sql/field.cc index e9eaa440952..28caea78c7d 100644 --- a/sql/field.cc +++ b/sql/field.cc @@ -1428,6 +1428,35 @@ void Field::load_data_set_value(const char *pos, uint length, }

+void Field::error_generated_column_function_is_not_allowed(THD *thd) const +{ + StringBuffer<64> tmp; + vcol_info->expr->print(&tmp, QT_TO_SYSTEM_CHARSET); + my_error(ER_GENERATED_COLUMN_FUNCTION_IS_NOT_ALLOWED, MYF(0), + tmp.c_ptr(), vcol_info->get_vcol_type_name(), + (const char *) field_name /*add .str on merge to 10.3 */);

better const_cast(field_name) and it won't need a comment - the compiler will catch it

...

+} + +

this needs a comment. May be as simple as "see sql_mode.h" btw, you might need this one-line comment in few other places too

...

+bool Field::check_vcol_sql_mode_dependency(THD *thd) const +{ + DBUG_ASSERT(vcol_info); + if (!(part_of_key.is_clear_all() && key_start.is_clear_all()))

1. why not for persistent? 2. why both part_of_key and key_start? I though part_of_key means "anywhere in the key", that is it's a superset of key_start.

...

+ { + Sql_mode_dependency dep= + vcol_info->expr->value_depends_on_sql_mode() & + Sql_mode_dependency(~0, ~can_handle_sql_mode_dependency_on_store()); + if (dep) + { + error_generated_column_function_is_not_allowed(thd); + dep.push_dependency_warnings(thd); + return true; + } + } + return false; +} + + /** Numeric fields base class constructor. */ diff --git a/sql/item_func.h b/sql/item_func.h index 81a7e948085..5be62427852 100644 --- a/sql/item_func.h +++ b/sql/item_func.h @@ -773,11 +773,16 @@ class Item_func_plus :public Item_func_additive_op

class Item_func_minus :public Item_func_additive_op { + Sql_mode_dependency m_sql_mode_dependency; public: Item_func_minus(THD *thd, Item *a, Item *b): Item_func_additive_op(thd, a, b) {} const char *func_name() const { return "-"; } enum precedence precedence() const { return ADD_PRECEDENCE; } + Sql_mode_dependency value_depends_on_sql_mode() const + { + return m_sql_mode_dependency;

1. no soft_to_hard here? 2. If you do need soft_to_hard here, then, may be, better to do soft_to_hard not in value_depends_on_sql_mode() at all, but just in one place - in Field::check_vcol_sql_mode_dependency() ?

...

+ } longlong int_op(); double real_op(); my_decimal *decimal_op(my_decimal *); diff --git a/sql/item_strfunc.cc b/sql/item_strfunc.cc index 753b6134419..846920bc8c2 100644 --- a/sql/item_strfunc.cc +++ b/sql/item_strfunc.cc @@ -2103,6 +2103,38 @@ void Item_func_trim::print(String *str, enum_query_type query_type) }

+/* + RTRIM(expr) + TRIM(TRAILING ' ' FROM expr) + remove argument's soft dependency on PAD_CHAR_TO_FULL_LENGTH: +*/ +Sql_mode_dependency Item_func_trim::value_depends_on_sql_mode() const +{ + DBUG_ASSERT(fixed); + if (arg_count == 1) // RTRIM(expr) + return (args[0]->value_depends_on_sql_mode() & + Sql_mode_dependency(~0, ~MODE_PAD_CHAR_TO_FULL_LENGTH)). + soft_to_hard(); + // TRIM(... FROM expr) + DBUG_ASSERT(arg_count == 2); + if (!args[1]->value_depends_on_sql_mode_const_item()) + return Item_func::value_depends_on_sql_mode(); + StringBuffer<64> trimstrbuf; + String *trimstr= args[1]->val_str(&trimstrbuf); + if (!trimstr) + return Sql_mode_dependency(); // will return NULL + if (trimstr->length() == 0) + return Item_func::value_depends_on_sql_mode(); // will trim nothing + if (trimstr->lengthsp() != 0) + return Item_func::value_depends_on_sql_mode(); // will trim not only spaces

what about TRIM(TRAILING ' ' FROM expr) ? it'll trim an even number of spaces, might leave one space untrimmed.

...

+ // TRIM(TRAILING ' ' FROM expr) + return ((args[0]->value_depends_on_sql_mode() | + args[1]->value_depends_on_sql_mode()) & + Sql_mode_dependency(~0, ~MODE_PAD_CHAR_TO_FULL_LENGTH)). + soft_to_hard(); +} + + /* Item_func_password */

bool Item_func_password::fix_fields(THD *thd, Item **ref)

Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org