- developers - lists.mariadb.org

[Maria-developers] Implementation of Threadpool
by GUESNET, ETIENNE (ext) 19 Sep '19

19 Sep '19

Hi, I am implementing a threadpool system to AIX. The AIX equivalent of epoll / kqueue on AIX is pollset (and IOCP, but partial implementation only). However, pollset has only a level-trigger mode and MariaDB needs edge-trigger (see comments of sql/threadpoll_generic.h file). Adding a pollset support in MariaDB would be difficult, and probably not so efficient, as we need to simulate the edge-trigger behavior. Obviously, AIX has poll and select support. MariaDB has not. Is there a reason to don't implement threadpoll through poll or select? No interest? Performance issues? MariaDB currently works on AIX without threadpool; in term of efficiency, do you know what can be obtained using threadpool with poll/select or a more modern solution? As far I know, SunOS/Solaris/Illumos threadpoll system (called "port") is also level-trigger only, but I do not find specific functions to manage this. Thanks! Etienne Guesnet.

4 6

[Maria-developers] Congratulations on a successful GSoC!
by Vicențiu Ciorbaru 05 Sep '19

05 Sep '19

Hi! By now, all of you have probably received the results of this year's Google Summer of Code. I want to congratulate you all for the work that you have done this year, students and mentors alike. We have had 4 projects this year: 1. EXCEPT ALL and INTERSECT ALL operations - Ruihang Xia https://github.com/MariaDB/server/pull/1378 2. INSERT ... RETURNING - Rucha Deodar https://medium.com/@ruchad1998/google-summer-of-code-2019-add-returning-to-… 3. UPDATE ... RETURNING - Miroslav Koberskii https://gist.github.com/Mup0c/43c781e2135e55bf5126e0f8c60e2a24 4. Support for indexes on expressions - Alexey Mogilyovkin https://gist.github.com/ZeroICQ/f0c35a20cae7065368dae31d7ad9b001 Implementing these projects certainly took a lot of effort. The first one is already merged within MariaDB and the rest will probably follow, after a bit more polishing. The MariaDB community would be grateful if you stuck around to watch over your projects, extend them and improve them over time. I am sure there was a lot to learn from this experience. Now that you have gotten a feel to what it means to contribute significant features to MariaDB you are in a great position to help others! In this sense, I encourage you to keep being active in the community and share what you have learned. :) Thank you for a great summer! Vicențiu Ciorbaru GSoC MariaDB Foundation Admin

1 0

[Maria-developers] Review for MDEV-19708 RBR replication loses data silently ignoring important column attributes
by Alexander Barkov 02 Sep '19

02 Sep '19

Hi Sachin, I've reviewed the code in bb-10.5-19708. It looks good. I suggest some cleanups. - Remove Field::binlog_type_info_slave, it's not used. - Remove Field::binlog_type_info_master. Please don't cache Binlog_type_info inside the Field itself. Let the caller code cache it in an array of Binlog_type_info pointers. - Change this method in Field: virtual Binlog_type_info *binlog_type_info(); to virtual Binlog_type_info *binlog_type_info(MEM_ROOT *) const; Let's pass mem_root as a parameter instead of using table->mem_root. Please add the 'const' qualifier! - Restore the 'const' qualifier to Field::enum_conv_type rpl_conv_type_from - Instead of doing this: Binlog_type_info * Field_new_decimal::binlog_type_info() { Field_num::binlog_type_info(); this->binlog_type_info_master->m_metadata= precision + (decimals() << 8); this->binlog_type_info_master->m_metadata_size= 2; return this->binlog_type_info_master; } Please add a number of Binlog_type_info constructor helpers, in addition to the current one. For example, for numeric types this would be good: Binlog_type_info(uchar type_code, uint16 metadata, uint8 metadata_size, binlog_signess_t signess) :m_type_code(type_code), m_metadata(metadata), m_metadata_size(metadata_size), m_signess(signess), m_cs(NULL), m_enum_typelib(NULL), m_set_typelib(NULL), m_geom_type(GEOM_GEOMETRY) { }; so you can do something simple like this: Binlog_type_info Field_new_decimal::binlog_type_info(MEM_ROOT *root) const { return new (root) Binlog_type_info(type(), precision + (decimals() << 8), 2, signess_arg); } - It seems you always set signess to SIGNESS_NOT_RELEVANT. Where is signess set to SIGNED or UNSIGNED? - This is something we've never used before: +#include <vector> +#include <string> +#include <functional> +#include <memory> We need to discuss this with Serg. Is it possible to avoid this? - There is a new test file mysql-test/suite/rpl/t/rpl_mdev_19708.test.diff but I could not find a corresponding .result.diff. Where is it? - Please use error names instead of numbers in tests: Instead of: --error 1231 SET GLOBAL binlog_row_metadata = -1; it should be: --error ER_WRONG_VALUE_FOR_VAR SET GLOBAL binlog_row_metadata = -1; You can find error names in include/mysqld_ername.h Thanks.

2 1

Re: [Maria-developers] 899c0b3ec6e: Part2: 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
by Sergei Golubchik 30 Aug '19

30 Aug '19

Hi, Alexander! On Aug 30, Alexander Barkov wrote: > revision-id: 899c0b3ec6e (mariadb-10.2.26-51-g899c0b3ec6e) > parent(s): e4415549e53 > author: Alexander Barkov <bar(a)mariadb.com> > committer: Alexander Barkov <bar(a)mariadb.com> > timestamp: 2019-08-29 12:35:19 +0400 > message: > > Part2: 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 patch allows the server to open old tables that have > "bad" generated columns (i.e. indexed virtual generated columns, > persistent generated columns) that depend on sql_mode, > for general things like SELECT, INSERT, DROP, etc. > Warning are issued in such cases. > > Only these commands are now disallowed and return an error: > - CREATE TABLE introducing a "bad" generated column > - ALTER TABLE introducing a "bad" generated column > - CREATE INDEX introdicing a "bad" generated column > (i.e. adding an index on a virtual generated column > that depends on sql_mode). > > Note, these commands are allowed: > - ALTER TABLE removing a "bad" generate column > - ALTER TABLE removing an index from a "bad" virtual generated column > - DROP INDEX removing an index from a "bad" virtual generated column > but only if the table does not have any "bad" columns as a result. > > diff --git a/sql/field.cc b/sql/field.cc > index e2b745743d2..b720db19ebf 100644 > --- a/sql/field.cc > +++ b/sql/field.cc > @@ -1428,16 +1428,24 @@ void Field::load_data_set_value(const char *pos, uint length, > } > > > -void Field::error_generated_column_function_is_not_allowed(THD *thd) const > +void Field::error_generated_column_function_is_not_allowed(THD *thd, > + bool error) const > { > StringBuffer<64> tmp; > vcol_info->expr->print(&tmp, (enum_query_type) > (QT_TO_SYSTEM_CHARSET | > QT_ITEM_IDENT_SKIP_DB_NAMES | > QT_ITEM_IDENT_SKIP_TABLE_NAMES)); > - my_error(ER_GENERATED_COLUMN_FUNCTION_IS_NOT_ALLOWED, MYF(0), > - tmp.c_ptr(), vcol_info->get_vcol_type_name(), > - const_cast<const char*>(field_name)); > + if (error) > + my_error(ER_GENERATED_COLUMN_FUNCTION_IS_NOT_ALLOWED, MYF(0), > + tmp.c_ptr(), vcol_info->get_vcol_type_name(), > + const_cast<const char*>(field_name)); > + else > + push_warning_printf(thd, Sql_condition::WARN_LEVEL_WARN, > + ER_GENERATED_COLUMN_FUNCTION_IS_NOT_ALLOWED, > + ER_THD(thd, ER_GENERATED_COLUMN_FUNCTION_IS_NOT_ALLOWED), > + tmp.c_ptr(), vcol_info->get_vcol_type_name(), > + const_cast<const char*>(field_name)); an easier way of doing it would be my_error(ER_GENERATED_COLUMN_FUNCTION_IS_NOT_ALLOWED, MYF(error ? 0 : ME_WARNING), tmp.c_ptr(), vcol_info->get_vcol_type_name(), const_cast<const char*>(field_name)); > } > > > diff --git a/sql/table.h b/sql/table.h > index 7786679982f..23a61345242 100644 > --- a/sql/table.h > +++ b/sql/table.h > @@ -325,6 +325,20 @@ enum tmp_table_type > }; > enum release_type { RELEASE_NORMAL, RELEASE_WAIT_FOR_DROP }; > > + > +enum vcol_init_mode > +{ > + VCOL_INIT_DEPENDENCY_FAILURE_IS_WARNING= 1, > + VCOL_INIT_DEPENDENCY_FAILURE_IS_ERROR= 2 > + /* > + There will be a new flags soon, better say "There may be new flags here" because may be there won't be any new flags soon after all, if this your fix will be sufficient :) > + e.g. to automatically remove sql_mode dependency: > + GENERATED ALWAYS AS (char_col) -> > + GENERATED ALWAYS AS (RTRIM(char_col)) > + */ > +}; > + > + > enum enum_vcol_update_mode > { > VCOL_UPDATE_FOR_READ= 0, Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0

Re: [Maria-developers] 5ba0b11c5d1: MDEV-20257 Fix crash in Grant_table_base::init_read_record
by Sergei Golubchik 29 Aug '19

29 Aug '19

Hi, Robert! On Aug 28, Robert Bindar wrote: > revision-id: 5ba0b11c5d1 (mariadb-10.4.7-35-g5ba0b11c5d1) > parent(s): 4a490d1a993 > author: Robert Bindar <robert(a)mariadb.org> > committer: Robert Bindar <robert(a)mariadb.org> > timestamp: 2019-08-27 11:38:52 +0300 > message: > > MDEV-20257 Fix crash in Grant_table_base::init_read_record > > The bug shows up when a 10.4+ server starts on a <10.4 datadir > with a crashed mysql.user table. > Grant_tables::open_and_lock tries to open the list of requested > tables (host, db, global_priv,..) and because global_priv doesn't > exist (pre-10.4 datadir), it falls back to opening mysql.user. > The first open_tables call for [host, db, global_priv,..] works > just fine. The second open_tables call (trying to open mysql.user > alone) sees the crashed user table and performs 3 steps: > - closes all the open tables > - attempts a table fix for mysql.user (succeeds) > - opens the tables initially passed as arguments > In an ideal world, the first step should only close the tables > passed as arguments (i.e. mysql.user), but for some reasons > close_tables_for_reopen makes a close_thread_tables call which > closes everything in THD::open_tables (nevertheless, this is > probably the intended behavior). > This side effect causes all the tables opened in the first > open_tables call to now be closed and Grant_table_base::init_read_record > tries to read from a released table. > > diff --git a/mysql-test/main/mdev20257.test b/mysql-test/main/mdev20257.test > new file mode 100644 > index 00000000000..8506292d57c > --- /dev/null > +++ b/mysql-test/main/mdev20257.test > @@ -0,0 +1,51 @@ > +--source include/not_embedded.inc > +--echo # > +--echo # MDEV 20257 Server crashes in Grant_table_base::init_read_record > +--echo # upon crash-upgrade > +--echo # > + > +let $MYSQLD_DATADIR= `select @@datadir`; > + > +rename table mysql.global_priv to mysql.global_priv_bak; > +rename table mysql.user to mysql.user_bak; > +rename table mysql.db to mysql.db_bak; > +rename table mysql.proxies_priv to mysql.proxies_priv_bak; > +rename table mysql.roles_mapping to mysql.roles_mapping_bak; > + > +--source include/shutdown_mysqld.inc > + > +# Bring in a crashed user table > +# Ideally we should've copied only the crashed mysql.user, but this > +# would make mysqld crash in some other place before hitting the > +# crash spot described in MDEV-20257 which is what we're trying to > +# test against. I don't understand that. Where does it crash then? What difference does it make that you replace all tables - are they all corrupted? > +--copy_file std_data/mdev20257/user.frm $MYSQLD_DATADIR/mysql/user.frm > +--copy_file std_data/mdev20257/user.MYD $MYSQLD_DATADIR/mysql/user.MYD > +--copy_file std_data/mdev20257/user.MYI $MYSQLD_DATADIR/mysql/user.MYI > + > +--copy_file std_data/mdev20257/host.frm $MYSQLD_DATADIR/mysql/host.frm > +--copy_file std_data/mdev20257/host.MYD $MYSQLD_DATADIR/mysql/host.MYD > +--copy_file std_data/mdev20257/host.MYI $MYSQLD_DATADIR/mysql/host.MYI > + > +--copy_file std_data/mdev20257/db.frm $MYSQLD_DATADIR/mysql/db.frm > +--copy_file std_data/mdev20257/db.MYD $MYSQLD_DATADIR/mysql/db.MYD > +--copy_file std_data/mdev20257/db.MYI $MYSQLD_DATADIR/mysql/db.MYI > +--copy_file std_data/mdev20257/db.opt $MYSQLD_DATADIR/mysql/db.opt > + > +--source include/start_mysqld.inc > + > +call mtr.add_suppression("Table \'\..mysql\.user\' is marked as crashed and should be repaired"); > +call mtr.add_suppression("Checking table: \'\..mysql\.user\'"); > +call mtr.add_suppression("mysql.user: 1 client is using or hasn't closed the table properly"); > +call mtr.add_suppression("Missing system table mysql..*; please run mysql_upgrade to create it"); > + > +# Cleanup > +drop table mysql.user; > +drop table mysql.db; > +drop table mysql.host; > +rename table mysql.global_priv_bak to mysql.global_priv; > +rename table mysql.user_bak to mysql.user; > +rename table mysql.db_bak to mysql.db; > +rename table mysql.proxies_priv_bak to mysql.proxies_priv; > +rename table mysql.roles_mapping_bak to mysql.roles_mapping; > + > diff --git a/mysql-test/std_data/mdev20257/db.opt b/mysql-test/std_data/mdev20257/db.opt > --- /dev/null > +++ b/mysql-test/std_data/mdev20257/db.opt > @@ -0,0 +1,2 @@ > +default-character-set=latin1 > +default-collation=latin1_swedish_ci why is that? > diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc > index 847d2bd777b..e028d084703 100644 > --- a/sql/sql_acl.cc > +++ b/sql/sql_acl.cc > - DBUG_RETURN(res); > + first = build_open_list(tables, which_tables, lock_type, true); > + > + uint counter; > + if (int rv= really_open(thd, first, &counter)) > + DBUG_RETURN(rv); > + > + TABLE *user_table= tables[USER_TABLE].table; > + if ((which_tables & Table_user) && !user_table) > + { > + close_thread_tables(thd); > + first = build_open_list(tables, which_tables, lock_type, false); Hmm... Here you always close/reopen all tables, just to account for the case when the user table is found corrupted. I'd argue that a corrupted user table happens rarely (compared to the non-corrupted), so it makes sense to optimize for the normal use case. That is, only open the user table as before. After it's opened you check if other tables are still opened - if they aren't you can close and reopen everything. This way you only close/reopen all tables if the user table was corrupted. Makes sense? > + if (int rv= really_open(thd, first, &counter)) > + DBUG_RETURN(rv); > + > + p_user_table= &m_user_table_tabular; > + user_table= tables[USER_TABLE + 1].table; > + } Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

2 1

Re: [Maria-developers] 5f1cd8fe7a5: 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
by Sergei Golubchik 28 Aug '19

28 Aug '19

Hi, Alexander! On Aug 28, Alexander Barkov wrote: > revision-id: 5f1cd8fe7a5 (mariadb-10.2.26-50-g5f1cd8fe7a5) > parent(s): 9de2e60d749 > author: Alexander Barkov <bar(a)mariadb.com> > committer: Alexander Barkov <bar(a)mariadb.com> > 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<const char*>(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(a)mariadb.org

2 1

Re: [Maria-developers] 25d6f575a50: MDEV-19705: Assertion `tmp >= 0' failed in best_access_path
by Sergei Golubchik 26 Aug '19

26 Aug '19

Hi, Varun! On Aug 26, Varun Gupta wrote: > revision-id: 25d6f575a50 (mariadb-10.4.7-31-g25d6f575a50) > parent(s): 7b4de10477a > author: Varun Gupta <varun.gupta(a)mariadb.com> > committer: Varun Gupta <varun.gupta(a)mariadb.com> > timestamp: 2019-08-22 02:38:38 +0530 > message: > > MDEV-19705: Assertion `tmp >= 0' failed in best_access_path > > The reason for hitting the assert is that rec_per_key estimates have some garbage value. > So the solution to fix this would be for long unique keys to use use rec_per_key for only 1 keypart > > --- > mysql-test/main/long_unique.result | 15 +++++++++++++++ > mysql-test/main/long_unique.test | 14 ++++++++++++++ > sql/table.cc | 13 ++++++++++++- > 3 files changed, 41 insertions(+), 1 deletion(-) > > diff --git a/sql/table.cc b/sql/table.cc > index 1ab4df0f7cf..fa57693e7eb 100644 > --- a/sql/table.cc > +++ b/sql/table.cc > @@ -828,6 +829,16 @@ static bool create_key_infos(const uchar *strpos, const uchar *frm_image_end, > { > keyinfo->key_length= HA_HASH_KEY_LENGTH_WITHOUT_NULL; > key_part++; // reserved for the hash value > + /* > + but hash keys have a flag "only whole key", so for hash keys one should never > + look at keyinfo->rec_per_key for partial keys, and they cannot possibly be calculated > + so here we make sure that the keyinfo->rec_per_key[keyinfo->user_defined_key_parts-1] > + would point to the slot we reserved for the keyinfo which holds the unique key with > + hash > + */ > + ulong *dest= (keyinfo->rec_per_key + keyinfo->user_defined_key_parts - 1); > + memcpy(&dest, &rec_per_key, sizeof(ulong*)); Sorry, I don't understand what you're doing here. I only meant something like keyinfo->rec_per_key-= keyinfo->user_defined_key_parts - 1; > + *rec_per_key++= 0; > } > > /* > Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0

[Maria-developers] Weekly Report
by Rucha Deodhar 19 Aug '19

19 Aug '19

Report for week 12: Hi! This week I merged some test cases in respective files into one so that the tests can be under 80 lines and also improved the documentation. Documentation for INSERT...RETURNING: https://docs.google.com/document/d/1EknNlh-J72cUlCg9rvcujUeZdzu5uzm0hmHG3qg… REPLACE...RETURNING: https://docs.google.com/document/d/1UMy5fi-j6yIObYIRwTteaJKHU5F-wPoEAVgVbeu… I also fixed feature_insert_returning system variable by adding some code and improving the test file. The test is passing. insert_returning_datatypes.result and replace_returning_datatypes.result files were showing binary data. To fix it, it was suggested to create a .reject and .result files, hexdump of the files, diff and giving a printable ASCII character as the input in BIT field because BIT is printed as VARCHAR. One thing I noticed was, after fixing the files they were still showing as binary files, which was probably because of the diffs. So I removed the existing files from my repo and pushed new files then cleaned up the commits again and fixed the 80 character per line rule for new tests. My github repo is up to date with latest changes. Regards, Rucha Deodhar

1 0

Re: [Maria-developers] e2a82094332: MENT-253 Server Audit v2 does not work with PS protocol: server crashes in filter_query_type.
by Sergei Golubchik 14 Aug '19

14 Aug '19

Hi, Alexey! On Aug 14, Alexey Botchkov wrote: > revision-id: e2a82094332 (mariadb-10.4.7-123-ge2a82094332) > parent(s): 691654721b3 > author: Alexey Botchkov <holyfoot(a)mariadb.com> > committer: Alexey Botchkov <holyfoot(a)mariadb.com> > timestamp: 2019-08-07 11:49:54 +0400 > message: > > MENT-253 Server Audit v2 does not work with PS protocol: server crashes in filter_query_type. > > Set the thd->query_string where possible. > server_audit2 should not crash when gets empty event->query. > > --- > plugin/server_audit2/server_audit.c | 15 ++++++++++++--- > sql/sql_prepare.cc | 22 ++++++++++++++++++++++ > 2 files changed, 34 insertions(+), 3 deletions(-) Test case? With normal and error code paths? > diff --git a/sql/sql_prepare.cc b/sql/sql_prepare.cc > index 74723d5bd91..d2cb4e9b372 100644 > --- a/sql/sql_prepare.cc > +++ b/sql/sql_prepare.cc > @@ -2600,6 +2600,14 @@ void mysqld_stmt_prepare(THD *thd, const char *packet, uint packet_length) > > if (stmt->prepare(packet, packet_length)) > { > + /* > + Set the thd->query_string with the current query so the > + audit plugin gets the meaningful notification. say that it's an error case. Like Prepare failed. Set the thd->query_string with the current query so the audit plugin gets the meaningful notification when it gets the error notification. > + */ > + if (alloc_query(thd, stmt->query_string.str(), stmt->query_string.length())) > + { > + thd->set_query(0, 0); > + } > /* Statement map deletes statement on erase */ > thd->stmt_map.erase(stmt); > thd->clear_last_stmt(); > @@ -3184,6 +3192,13 @@ static void mysql_stmt_execute_common(THD *thd, > char llbuf[22]; > my_error(ER_UNKNOWN_STMT_HANDLER, MYF(0), static_cast<int>(sizeof(llbuf)), > llstr(stmt_id, llbuf), "mysqld_stmt_execute"); > + /* > + Set thd->query_string with the stmt_id so the > + audit plugin gets the meaningful notification. Same here. > + */ > + if (alloc_query(thd, llbuf, strlen(llbuf))) > + thd->set_query(0, 0); > + > DBUG_VOID_RETURN; > } > stmt->read_types= read_types; > @@ -3939,6 +3954,13 @@ bool Prepared_statement::prepare(const char *packet, uint packet_len) > DBUG_RETURN(TRUE); > } > > + /* > + We'd like to have thd->query to be set to the actual query > + after the function ends. > + This value will be sent to audit pulgins later. 1. typo: "pulgins" 2. Mention explicitly that "query string is allocated in the stmt arena, not in the thd arena. But it's safe here, because stmt always has a longer lifetime than thd arena." - or something along these lines. > + */ > + stmt_backup.query_string= thd->query_string; > + > old_stmt_arena= thd->stmt_arena; > thd->stmt_arena= this; > > Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0

[Maria-developers] Weekly Report
by Rucha Deodhar 12 Aug '19

12 Aug '19

Report for week 11: Hello! This week I worked on cleaning up the code, commits and test cases. I was a little less familiar with git rebase -i so tried a couple of things on a dummy repo first and then cleaned up the commits on my repo. I also worked on the coding style review and removed all the trailing white spaces and tabs mentioned in the review. To remove the last argument of mysql_prepare_insert() I tried a couple of things like not calling setup_fields() and setup_wild() separately for INSERT...SELECT...RETURNING and calling std::swap if select_insert is true. Like so if(!lex->returning_list.is_empty()) { if (select_insert) //std::swap() setup_fields() and setup_wild() if (selec_insert) //std::swap() } and also changing where the above functions are called. But that didn't work. So now calling it separately for INSERT..SELECT..RETURNING and other variants. I added separate function to sql_base.cc and sql_base.h which calls setup_fields() and setup_wild() so that not only the last argument can be removed but also lines of code can be reduced. All the occurrences of (wild_num && setup_wild(thd, table_list, field_list, NULL, wild_num, &select_lex->hidden_bit_fields)) || setup_fields(thd, Ref_ptr_array(), field_list, MARK_COLUMNS_READ, NULL, NULL, 0) can be replaced with setup_returning_fields(). (Since this is used in insert..returning, insert..select..returning, delete...returning and update...returning too). On some lines of the test cases, the characters were coming to more than 80 characters. So fixed that and added some more test cases (INSERT...IGNORE..RETURNING and fields with auto_increment). Finally I merged all the commits made this week into one commit. My github repo is up to date with the latest changes. Here is the link for the same: https://github.com/rucha174/server/commit/0d7eb4198cc0aadba42acb4960c175702… Regards, Rucha Deodhar

1 0