- developers - lists.mariadb.org

Re: [Maria-developers] MDEV-17399: JSON_TABLE, commit 85757ecbef44484270e385a8d52998c67f72d11a
by Sergey Petrunia 23 Feb '21

23 Feb '21

Hi Alexey, First reactions to the JSON_TABLE patch: == CI Build is failing == Please check http://buildbot.askmonty.org/buildbot/grid?category=main&branch=bb-10.6-mde… the JSON_TABLE tests are failing. == On testcases == I see now the patch has tests in 3 files: 1. mysql-test/main/json_table_mysql.test 2. mysql-test/main/json_table_mysql2.test 3. mysql-test/suite/json/t/json_table.test #1 and #2 are both fixed copies of MySQL's test. Please remove one of them (the one that has less coverage). Then, can we have both files in the same testsuite? == On Table_function_json_table::ready_for_lookup() == Now that we have end_lateral_table, do we need to use Table_function_json_table::ready_for_lookup? This looks like two solutions for [almost] the same problem? I see that this example from json_table_mysql.test still uses the code path with ready_for_lookup() : --error ER_BAD_FIELD_ERROR UPDATE t1, JSON_TABLE(t1.c1,'$[*]' COLUMNS (a INT PATH '$.a')) AS jt1 SET jt1.a=1; Here, the name resolution for "jt1.a" is done without adjustment for end_lateral_table. Do we need the ready_for_lookup() trick here? (If we do, are we fine with m_setup_done to be set to true but never reset back to false? How will this work with PS?) == On opt_sum.cc == The diff has this: diff --git a/sql/opt_sum.cc b/sql/opt_sum.cc index af2d9ddc2e7..d021c651196 100644 --- a/sql/opt_sum.cc +++ b/sql/opt_sum.cc @@ -299,6 +299,7 @@ int opt_sum_query(THD *thd, tl->schema_table) { maybe_exact_count&= MY_TEST(!tl->schema_table && +//mdev-17399 !tl->table_function && (tl->table->file->ha_table_flags() & HA_HAS_RECORDS)); is_exact_count= FALSE; Please remove this if it is no longer needed. == On table dependencies: table elimination == create table t20 (a int not null); create table t21 (a int not null primary key, js varchar(100)); insert into t20 values (1),(2); insert into t21 values (1, '{"a":100}'); explain select t20.a, jt1.ab from t20 left join t21 on t20.a=t21.a join JSON_TABLE(t21.js,'$' COLUMNS (ab INT PATH '$.a')) AS jt1; +------+-------------+-------+------+---------------+------+---------+------+------+----------------------------+ | id | select_type | table | type | possible_keys | key | key_len | ref | rows | Extra | +------+-------------+-------+------+---------------+------+---------+------+------+----------------------------+ | 1 | SIMPLE | t20 | ALL | NULL | NULL | NULL | NULL | 2 | | | 1 | SIMPLE | jt1 | ALL | NULL | NULL | NULL | NULL | 40 | Table function: json_table | +------+-------------+-------+------+---------------+------+---------+------+------+----------------------------+ Here we can see an apparently invalid query plan: table t21 was eleminated even if JSON_TABLE uses it. == On table dependencies: regular subqueries == But it doesn't work for regular subqueries either (and this is unexpected): insert into t21 values (3, '{"a":300}'); explain select a, (select jt1.ab from JSON_TABLE(t21.js,'$' COLUMNS (ab INT PATH '$.a')) as jt1 ) from t21; +------+-------------+-------+------+---------------+------+---------+------+------+----------------------------+ | id | select_type | table | type | possible_keys | key | key_len | ref | rows | Extra | +------+-------------+-------+------+---------------+------+---------+------+------+----------------------------+ | 1 | PRIMARY | t21 | ALL | NULL | NULL | NULL | NULL | 2 | | | 2 | SUBQUERY | jt1 | ALL | NULL | NULL | NULL | NULL | 40 | Table function: json_table | +------+-------------+-------+------+---------------+------+---------+------+------+----------------------------+ Note the 'SUBQUERY', that is, the subquery is considered uncorrelated? This needs to be investigated. BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://petrunia.net

2 3

Re: [Maria-developers] MDEV-17399: Columns of JSON datatype?
by Sergey Petrunia 16 Feb '21

16 Feb '21

On Sun, Feb 14, 2021 at 05:14:10PM +0400, Alexey Botchkov wrote: > One question left about the json_table_mysql.text. > > --error ER_WRONG_OUTER_JOIN > SELECT * FROM t1 RIGHT JOIN > (SELECT o FROM t2, JSON_TABLE(j, '$[*]' COLUMNS (o FOR ORDINALITY)) AS jt) > AS t3 > ON (t3.o = t1.x); > > What is exactly wrong with the above join? It should work. The reason it doesn't work was described by me in the email with subject "MDEV-17399: JSON_TABLE's dependencies are not updated after derived merge" dated Feb, 8th. What happens here is: Thread 14 "mysqld" hit Breakpoint 1, Table_function_json_table::setup() (gdb) list 1091 } 1092 } 1093 > 1094 m_dep_tables= m_json->used_tables(); 1095 (gdb) p m_dep_tables $44 = 1 If you go one frame up, you will see that we are in select#2. tables are: (gdb) p join->select_lex->leaf_tables.elem(0)->alias.str $56 = 0x7ffeb0016550 "t2" (gdb) p join->select_lex->leaf_tables.elem(0)->table->map $58 = 1 (gdb) p join->select_lex->leaf_tables.elem(1)->alias.str $60 = 0x7ffeb0017d70 "jt" (gdb) p join->select_lex->leaf_tables.elem(1)->table->map $62 = 2 Ok, jt depends on t2. Then: Thread 14 "mysqld" hit Breakpoint 2, simplify_joins (gdb) p join->select_lex->leaf_tables.elements $65 = 3 (gdb) p join->select_lex->leaf_tables.elem(0)->alias.str $66 = 0x7ffeb0015870 "t1" (gdb) p join->select_lex->leaf_tables.elem(0)->table->map $67 = 1 (gdb) p join->select_lex->leaf_tables.elem(1)->alias.str $68 = 0x7ffeb0016550 "t2" (gdb) p join->select_lex->leaf_tables.elem(1)->table->map $69 = 2 (gdb) p join->select_lex->leaf_tables.elem(2)->alias.str $70 = 0x7ffeb0017d70 "jt" (gdb) p join->select_lex->leaf_tables.elem(2)->table->map $71 = 4 But the m_dep_tables still has the old value: (gdb) p join->select_lex->leaf_tables.elem(2)->table_function->m_dep_tables $72 = 1 BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://petrunia.net

1 0

[Maria-developers] MDEV-8306: EXPLAIN JSON output is not JSON-ish
by Sergey Petrunia 15 Feb '21

15 Feb '21

Hi Varun, (This email is not the complete input, and not the most important part of the input. More to follow. I think it's better to split input into multiple smaller pieces, it's easier to track, and you get to see the first portions of the input sooner) EXPLAIN EXTENDED SELECT t3.b, t2.a, t1.b, t1.a FROM t1,t2,t3 WHERE t1.b=t3.b ORDER BY t1.b DESC, t2.a DESC LIMIT 3; +------+-------------+-------------+------+---------------+------+---------+------+------+----------+------------------------------------+ | id | select_type | table | type | possible_keys | key | key_len | ref | rows | filtered | Extra | +------+-------------+-------------+------+---------------+------+---------+------+------+----------+------------------------------------+ | 1 | SIMPLE | t2 | ALL | NULL | NULL | NULL | NULL | 10 | 100.00 | | | 1 | SIMPLE | t1 | ALL | NULL | NULL | NULL | NULL | 20 | 100.00 | Using join buffer (flat, BNL join) | | 1 | SIMPLE | <sort-nest> | ALL | NULL | NULL | NULL | NULL | 3 | 100.00 | Using filesort | | 1 | SIMPLE | t3 | ALL | NULL | NULL | NULL | NULL | 100 | 5.00 | Using where | +------+-------------+-------------+------+---------------+------+---------+------+------+----------+------------------------------------+ So, here <sort-nest> includes t2 and t1, and then t3 is joined with it. However in EXPLAIN FORMAT=JSON output they all look like peers: { "query_block": { "select_id": 1, "table": { "table_name": "t2", "access_type": "ALL", "rows": 10, "filtered": 100 }, "block-nl-join": { "table": { "table_name": "t1", "access_type": "ALL", "rows": 20, "filtered": 100 }, "buffer_type": "flat", "buffer_size": "65", "join_type": "BNL" }, "read_sorted_file": { "filesort": { "sort_key": "`sort-nest`.b desc, `sort-nest`.a desc", "table": { "table_name": "<sort-nest>", "access_type": "ALL", "rows": 3, "filtered": 100 } } }, "table": { "table_name": "t3", "access_type": "ALL", "rows": 100, "filtered": 5, "attached_condition": "t3.b = `sort-nest`.b" } } } which is counter-intuitive. Also, `sort-nest` is quoted. I assume it is because of the '-' in it? Maybe we should change the name to avoid the quoting? BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://petrunia.net

2 3

Re: [Maria-developers] MDEV-17399 New patch for JSON_TABLE
by Sergey Petrunia 14 Feb '21

14 Feb '21

Hi Alexey, Input on the latest patch for MDEV-17399. It's good to see the patch taking shape, but have you tried running MySQL's tests for JSON_TABLE on it? I did, and it has caught several crashes, as well as error-no-error discrepancies and different query results. Some of these were expected, but some show that something needs to be fixed. Please find the first few cases below. It seems there is something odd going odd with the name resolution, both in MariaDB and MySQL - I don't have explanation for some of the errors. I intend to study the issue more and elaborate in a later email. Meanwhile, first obvious cases: === Crash in Name resolution == CREATE TABLE t1 (a INT, b INT); CREATE VIEW v2 AS SELECT * FROM t1 LIMIT 2; SELECT b FROM (SELECT * FROM v2) vq1, JSON_TABLE(CONCAT(vq1.b,'[{\"a\":\"3\"}]'), '$[*]' COLUMNS (id FOR ORDINALITY, jpath VARCHAR(100) PATH '$.a', JEXST INT EXISTS PATH '$.b') ) AS dt; === Crash in error reporting === select * from json_table( '[{"a":"3"},{"a":2},{"b":1},{"a":0}]', "!@#$!@#$" columns (id for ordinality, jpath varchar(100) path '$.a', jexst int exists path '$.b') ) as tt; === Item print misses quotes === CREATE VIEW v2 AS SELECT * FROM JSON_TABLE('{}', '$' COLUMNS ( x VARCHAR(10) PATH '$.a' DEFAULT '"isn''t here"' ON EMPTY) ) t; SHOW CREATE VIEW v2; ERROR 1064 (42000): 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 't here"' ON EMPTY)) `t`' at line 1 === Error for outer reference == # JSON_TABLE referring outer scope CREATE TABLE t1(id int, jd JSON); INSERT INTO t1 values (1, '[1,3,5]'),(2,'[2,4,6]'); SELECT * FROM t1 WHERE id IN (SELECT * FROM JSON_TABLE(t1.jd, '$[*]' COLUMNS (id INT PATH '$')) AS jt); Produces: ERROR 1054 (42S22): Unknown column 't1.jd' in 'JSON_TABLE argument' Should not produce it (Note that for some other kinds of outer references it seemed to work) BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog

2 15

[Maria-developers] MDEV-17399: Name resolution and handling dependencies
by Sergey Petrunia 14 Feb '21

14 Feb '21

Hi Alexey, At the moment, name resolution of JSON_TABLE's first argument is done "like in the WHERE clause" - one can refer to any table that is defined in the WHERE clause. This allows one to write queries where JSON_TABLE tables have incorrect dependencies - circular - dependency that contradicts the dependency imposed by OUTER JOIN - dependency that contradicts STRAIGHT_JOIN (WRONG-DEPS) the patch checks for these cases but I've reported cases where it fails. I haven't been find any statement in the SQL Standard about this, but it makes a statement about similar constructs, table functions and LATERAL subqueries: https://jira.mariadb.org/browse/MDEV-17399?focusedCommentId=179327&page=com… Other databases seem to apply the same limitation to JSON_TABLE's argument. When they do it, this automatically fixes all the wrong-dependency issues - a query with (WRONG-DEPS) is rejected at the name resolution phase. I think we should follow this and modify name resolution process to work in the same way. The way MySQL did it is described here: https://dev.mysql.com/worklog/task/?id=8867, LLD, grep for end_lateral_table. I'm not fond of having implicit parameters (end_lateral_table) which change they way name resolution works, but I think it is an acceptable solution (and we already have other such parameters). (An alternative option would be to have items in JSON_TABLE's first argument to use their own Name_resolution_context object that would specify the correct first/last table they should look at. This seems to be much harder to do). What do you think? BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://petrunia.net

2 1

[Maria-developers] 4f9c5a8cf54: MDEV-24773: slave_compressed_protocol doesn't work properly with semi-sync replication
by sujatha 08 Feb '21

08 Feb '21

revision-id: 4f9c5a8cf545133c3d29c0de3f7e2e57564607f2 (mariadb-10.3.26-78-g4f9c5a8cf54) parent(s): 59eda73eff1a22ac0373d818bc802c05e82b5449 author: Sujatha committer: Sujatha timestamp: 2021-02-05 16:45:35 +0530 message: MDEV-24773: slave_compressed_protocol doesn't work properly with semi-sync replication Back port upstream fix commit 1800b015a1d487330f7b15f2020b887be348a66b Author: Venkatesh Duggirala <venkatesh.duggirala(a)oracle.com> Date: Fri Sep 8 20:29:22 2017 +0530 Bug#26027024 SLAVE_COMPRESSED_PROTOCOL DOESN'T WORK WITH SEMI-SYNC REPLICATION IN MYSQL-5.7 Analysis: In mysql-5.6, dump thread (the thread that is created on Master after Slave requested for a binlog dump) is also used to receive acknowledgements from the Slave and act on them accordingly. For performance reasons, a special thread called Ack Receiver thread is added in mysql-5.7 Semi synchronous replication plugin. This thread does not have special handling to receive acknowledgements if Slave has enabled compression in the protocol. Hence Master is unable to handle any slave if Slave_compressed_protocol is enabled on it. Fix: Enable compress flag on the communication channels if the Slave has Slave_compressed_protocol ON. --- .../rpl_semi_sync_slave_compressed_protocol.result | 19 ++++++++ ...l_semi_sync_slave_compressed_protocol-slave.opt | 1 + .../t/rpl_semi_sync_slave_compressed_protocol.test | 55 ++++++++++++++++++++++ sql/semisync_master_ack_receiver.cc | 5 ++ 4 files changed, 80 insertions(+) diff --git a/mysql-test/suite/rpl/r/rpl_semi_sync_slave_compressed_protocol.result b/mysql-test/suite/rpl/r/rpl_semi_sync_slave_compressed_protocol.result new file mode 100644 index 00000000000..86816652565 --- /dev/null +++ b/mysql-test/suite/rpl/r/rpl_semi_sync_slave_compressed_protocol.result @@ -0,0 +1,19 @@ +include/master-slave.inc +[connection master] +SET @@GLOBAL.rpl_semi_sync_master_enabled = 1; +connection slave; +include/stop_slave.inc +SET @@GLOBAL.rpl_semi_sync_slave_enabled = 1; +include/start_slave.inc +connection master; +CREATE TABLE t1 (i INT); +DROP TABLE t1; +include/rpl_sync.inc +include/assert_grep.inc [Check that there is no 'Read semi-sync reply magic number error' in error log.] +connection master; +SET @@GLOBAL. rpl_semi_sync_master_enabled = 0; +connection slave; +include/stop_slave.inc +SET @@GLOBAL. rpl_semi_sync_slave_enabled = 0; +include/start_slave.inc +include/rpl_end.inc diff --git a/mysql-test/suite/rpl/t/rpl_semi_sync_slave_compressed_protocol-slave.opt b/mysql-test/suite/rpl/t/rpl_semi_sync_slave_compressed_protocol-slave.opt new file mode 100644 index 00000000000..a1b687d691e --- /dev/null +++ b/mysql-test/suite/rpl/t/rpl_semi_sync_slave_compressed_protocol-slave.opt @@ -0,0 +1 @@ +--slave_compressed_protocol diff --git a/mysql-test/suite/rpl/t/rpl_semi_sync_slave_compressed_protocol.test b/mysql-test/suite/rpl/t/rpl_semi_sync_slave_compressed_protocol.test new file mode 100644 index 00000000000..6343fb29ffe --- /dev/null +++ b/mysql-test/suite/rpl/t/rpl_semi_sync_slave_compressed_protocol.test @@ -0,0 +1,55 @@ +################################################################################ +# Bug#26027024 SLAVE_COMPRESSED_PROTOCOL DOESN'T WORK WITH SEMI-SYNC +# REPLICATION IN MYSQL-5.7 +# +# Steps to reproduce: +# 1) Set slave_compressed_protocol ON on Slave. +# 2) Do some sample work on Master +# 3) After the work is synced on Slave, check that there is no error +# (Read semi-sync reply magic number error) on Slave. +# 4) Cleanup +################################################################################ +# Test is independent of Binlog format. One of the three formats is enough +# for testing. Choosing 'Row' format. +--source include/have_binlog_format_row.inc +--source include/master-slave.inc + +--let $sav_enabled_master=`SELECT @@GLOBAL.rpl_semi_sync_master_enabled ` +SET @@GLOBAL.rpl_semi_sync_master_enabled = 1; + +--connection slave +source include/stop_slave.inc; +--let $sav_enabled_slave=`SELECT @@GLOBAL.rpl_semi_sync_slave_enabled ` +SET @@GLOBAL.rpl_semi_sync_slave_enabled = 1; +source include/start_slave.inc; + +--connection master +# Do some sample work on Master with slave_compressed_protocol ON. +# (slave_compressed_protocol is set to ON in -slave.opt file of this test.) +CREATE TABLE t1 (i INT); +DROP TABLE t1; + +# Make sure sync is done, so that next 'assert' step can be executed without +# any issues. +--source include/rpl_sync.inc + +# Without the fix, the test would have generated few +# errors in the error log. With the fix, test will +# pass without any errors in the error log. +--let $assert_text= Check that there is no 'Read semi-sync reply magic number error' in error log. +--let $assert_select=Read semi-sync reply magic number error +--let $assert_file= $MYSQLTEST_VARDIR/log/mysqld.1.err +--let $assert_count= 0 +--let $assert_only_after = CURRENT_TEST:rpl.rpl_semi_sync_slave_compressed_protocol.test +--source include/assert_grep.inc + +--connection master +--eval SET @@GLOBAL. rpl_semi_sync_master_enabled = $sav_enabled_master + +--connection slave +source include/stop_slave.inc; +--eval SET @@GLOBAL. rpl_semi_sync_slave_enabled = $sav_enabled_slave +source include/start_slave.inc; + +# Cleanup +--source include/rpl_end.inc diff --git a/sql/semisync_master_ack_receiver.cc b/sql/semisync_master_ack_receiver.cc index 81f494c9d34..cc3a56a56b2 100644 --- a/sql/semisync_master_ack_receiver.cc +++ b/sql/semisync_master_ack_receiver.cc @@ -268,6 +268,11 @@ void Ack_receiver::run() net_clear(&net, 0); net.vio= &slave->vio; + /* + Set compress flag. This is needed to support + Slave_compress_protocol flag enabled Slaves + */ + net.compress= slave->thd->net.compress; len= my_net_read(&net); if (likely(len != packet_error))

1 0

[Maria-developers] RFC: incompatible changes to mtr debugging flags
by Sergei Golubchik 27 Jan '21

27 Jan '21

Hi, all, Here's a feature I want to discuss, as it's rather incompatibly removes a bunch of mtr command-line options. It's a unified handling of all debuggers. I wanted to do it for quite a while, but the actual trigger was me trying to add support for --rr (https://rr-project.org/) in embedded. It was supported only for non-embedded server, and making it work for embedded needed way more copy-pasting than I was comfortable with. So, in my branch all debuggers are not haphazardly added using whatever syntax one thought of at the moment and copy-pasted all over. They've handled by a common module, all support the same set of options (using gdb as an example): --gdb, --boot-gdb, --client-gdb, --manual-gdb. Both --gdb and --client-gdb work for embedded. All four of them accept an optional argument that can be a semicolon separated list of gdb commands, like in ./mtr 1st --gdb='b mysql_parse;r' but can start from command-line options too: ./mtr 1st --boot-gdb='-nh -q;b mysql_parse;r' A "debugger" is anything that wraps mysqld or mysqltest execution, the current list of debuggers is: gdb, ddd, dbx, lldb, valgrind, strace, ktrace, rr, devenv, windbg, vc_express, vc, vsjitdebugger. There's also "valgdb" that does what '--valgrind --gdb' was doing before. This removes the following mtr options: --rr-args --rr-dir --manual-debug --debugger --strace-option --stracer --valgrind-all --valgrind-mysqltest --valgrind-mysqld --valgrind-options --valgrind-option --valgrind-path --callgrind, and --valgrind only enables it for the server, not for everything. If pushed, it'll definitely break backward compatibility. But I don't think mtr backward compatibility is all that important, it might only affect a handful of developers who use scripts to start mtr with specific hard-coded settings, those scripts could be easily adjusted. It's much more important to have same mtr features in all branches, so that when switching between branches, one wouldn't need to guess what command line options mtr supports now. That is, if pushed, this should go into 10.2. Opinions? Missing features? Push / not push? for the reference, it's in bb-10.2-serg branch at the moment. Regards, Sergei

5 7

Re: [Maria-developers] 3e01da668d1: MDEV-21785: sequences used as default by other table not dumped in right order by mysqldump
by Sergei Golubchik 21 Jan '21

21 Jan '21

Hi, Oleksandr! On Jan 21, Oleksandr Byelkin wrote: > revision-id: 3e01da668d1 (mariadb-10.3.26-64-g3e01da668d1) > parent(s): 59998d3480f > author: Oleksandr Byelkin <sanja(a)mariadb.com> > committer: Oleksandr Byelkin <sanja(a)mariadb.com> > timestamp: 2021-01-14 10:45:51 +0100 > message: > > MDEV-21785: sequences used as default by other table not dumped in right order by mysqldump > > Dump sequences first. > > This atch made to keep it small and > to keep number of queries to the server the same. > > Order of tables in a dump can not be changed > (except sequences first) because mysql_list_tables > uses SHOW TABLES and I used SHOW FULL TABLES. it looks like a rather overengineered implementation. I'd suggest to simplify it as: > diff --git a/client/mysqldump.c b/client/mysqldump.c > index a964f96437d..17837713c01 100644 > --- a/client/mysqldump.c > +++ b/client/mysqldump.c > @@ -42,6 +42,11 @@ > /* on merge conflict, bump to a higher version again */ > #define DUMP_VERSION "10.19" > > +/** > + First mysql version supporting sequences. > +*/ > +#define FIRST_SEQUENCE_VERSION 100300 seems unnecessary, you implemented SHOW FULL TABLES in 5.0.2-alpha. Oct 2004. You can safely use SHOW FULL TABLES unconditionally. > + > #include <my_global.h> > #include <my_sys.h> > #include <my_user.h> > @@ -92,6 +97,13 @@ > /* Max length GTID position that we will output. */ > #define MAX_GTID_LENGTH 1024 > > + > +/* Dump sequence/tables control */ > +#define DUMP_TABLE_TABLE 1 > +#define DUMP_TABLE_SEQUENCE 2 > +#define DUMP_TABLE_ALL (DUMP_TABLE_TABLE | DUMP_TABLE_SEQUENCE) > + > + > static my_bool ignore_table_data(const uchar *hash_key, size_t len); > static void add_load_option(DYNAMIC_STRING *str, const char *option, > const char *option_value); > @@ -171,6 +183,7 @@ static DYNAMIC_STRING dynamic_where; > static MYSQL_RES *get_table_name_result= NULL; > static MEM_ROOT glob_root; > static MYSQL_RES *routine_res, *routine_list_res; > +static int get_table_name_result_short= 1; that'll be unnecessary > > > #include <sslopt-vars.h> > @@ -3853,13 +3866,16 @@ static char *alloc_query_str(size_t size) > ARGS > table - table name > db - db name > + type_ctrl - dump table or sequences or both > > RETURNS > void > */ > > > -static void dump_table(const char *table, const char *db, const uchar *hash_key, size_t len) > +static void dump_table(const char *table, const char *db, > + const uchar *hash_key, size_t len, > + const uint type_ctrl) > { > char ignore_flag; > char buf[200], table_buff[NAME_LEN+3]; > @@ -3881,9 +3897,11 @@ static void dump_table(const char *table, const char *db, const uchar *hash_key, > */ > if (check_if_ignore_table(table, table_type) & IGNORE_SEQUENCE_TABLE) > { > - get_sequence_structure(table, db); > + if (type_ctrl & DUMP_TABLE_SEQUENCE) > + get_sequence_structure(table, db); > + DBUG_VOID_RETURN; > + } else if (!(type_ctrl & DUMP_TABLE_TABLE)) > DBUG_VOID_RETURN; > - } here I'd remove the whole if(). Let's say the convention is that dump_table() should never be invoked for sequences, it's the caller's job to ensure it. > /* > Make sure you get the create table info before the following check for > --no-data flag below. Otherwise, the create table info won't be printed. > @@ -4368,18 +4386,49 @@ static void dump_table(const char *table, const char *db, const uchar *hash_key, > } /* dump_table */ > > > -static char *getTableName(int reset) > +static char *getTableName(int reset, uint table_control) > { > MYSQL_ROW row; > > if (!get_table_name_result) > { > - if (!(get_table_name_result= mysql_list_tables(mysql,NullS))) > - return(NULL); > + if (mysql_get_server_version(mysql) >= FIRST_SEQUENCE_VERSION) > + { > + const char *query= > + "SHOW FULL TABLES"; > + > + if (mysql_query_with_error_report(mysql, 0, query)) > + return (NULL); > + > + if (!(get_table_name_result= mysql_store_result(mysql))) > + return (NULL); > + > + get_table_name_result_short= 0; > + } > + else > + { > + if (!(get_table_name_result= mysql_list_tables(mysql,NullS))) > + return(NULL); > + get_table_name_result_short= 1; > + } this will be twice as small, if you won't try to support before-5.0 servers. > } > if ((row= mysql_fetch_row(get_table_name_result))) > - return((char*) row[0]); > - > + { > + int sequence; > + if (get_table_name_result_short || > + table_control == DUMP_TABLE_ALL) > + return((char*) row[0]); > + while (row && > + (((sequence= (strcmp(row[1], "SEQUENCE") == 0)) && > + (table_control & DUMP_TABLE_TABLE)) || > + ((!sequence) && > + (table_control & DUMP_TABLE_SEQUENCE)))) I'd rather just do static char *getTableName(int reset, uint want_sequence) and while (row && strcmp(row[1], "SEQUENCE") == want_sequence) row= mysql_fetch_row(get_table_name_result); > + { > + row= mysql_fetch_row(get_table_name_result); > + } > + if (row) > + return((char*) row[0]); > + } > if (reset) > mysql_data_seek(get_table_name_result,0); /* We want to read again */ > else > @@ -4785,17 +4834,17 @@ static int dump_all_stats() > /* EITS added in 10.0.1 */ > if (mysql_get_server_version(mysql) >= 100001) > { > - dump_table("column_stats", "mysql", NULL, 0); > - dump_table("index_stats", "mysql", NULL, 0); > - dump_table("table_stats", "mysql", NULL, 0); > + dump_table("column_stats", "mysql", NULL, 0, DUMP_TABLE_TABLE); > + dump_table("index_stats", "mysql", NULL, 0, DUMP_TABLE_TABLE); > + dump_table("table_stats", "mysql", NULL, 0, DUMP_TABLE_TABLE); this wouldn't be needed if the caller will only use dump_table for tables > } > /* Innodb may be disabled */ > if (!mysql_query(mysql, "show fields from innodb_index_stats")) > { > MYSQL_RES *tableres= mysql_store_result(mysql); > mysql_free_result(tableres); > - dump_table("innodb_index_stats", "mysql", NULL, 0); > - dump_table("innodb_table_stats", "mysql", NULL, 0); > + dump_table("innodb_index_stats", "mysql", NULL, 0, DUMP_TABLE_TABLE); > + dump_table("innodb_table_stats", "mysql", NULL, 0, DUMP_TABLE_TABLE); > } > opt_no_create_info= prev_no_create_info; > return 0; > @@ -4817,11 +4866,11 @@ static int dump_all_timezones() > opt_prev_no_create_info= opt_no_create_info; > opt_no_create_info= 1; > fprintf(md_result_file,"\nUSE mysql;\n"); > - dump_table("time_zone", "mysql", NULL, 0); > - dump_table("time_zone_name", "mysql", NULL, 0); > - dump_table("time_zone_leap_second", "mysql", NULL, 0); > - dump_table("time_zone_transition", "mysql", NULL, 0); > - dump_table("time_zone_transition_type", "mysql", NULL, 0); > + dump_table("time_zone", "mysql", NULL, 0, DUMP_TABLE_TABLE); > + dump_table("time_zone_name", "mysql", NULL, 0, DUMP_TABLE_TABLE); > + dump_table("time_zone_leap_second", "mysql", NULL, 0, DUMP_TABLE_TABLE); > + dump_table("time_zone_transition", "mysql", NULL, 0, DUMP_TABLE_TABLE); > + dump_table("time_zone_transition_type", "mysql", NULL, 0, DUMP_TABLE_TABLE); > opt_no_create_info= opt_prev_no_create_info; > return 0; > } > @@ -5346,12 +5396,29 @@ static int dump_all_tables_in_db(char *database) > DBUG_RETURN(1); > } > } > - while ((table= getTableName(0))) > + > + if (mysql_get_server_version(mysql) >= FIRST_SEQUENCE_VERSION && > + !opt_no_create_info) > + { > + // First process sequences > + while ((table= getTableName(1, DUMP_TABLE_SEQUENCE))) > + { > + char *end= strmov(afterdot, table); > + if (include_table((uchar*) hash_key, end - hash_key)) > + { > + dump_table(table, database, (uchar*) hash_key, end - hash_key, > + DUMP_TABLE_SEQUENCE); > + } > + } > + table_ctrl= DUMP_TABLE_TABLE; // next pass > + } and instead of all the above, you could do just for (int table_ctrl= 1; table_ctrl >= 0; table_ctrl--) > + while ((table= getTableName(0, table_ctrl))) > { > char *end= strmov(afterdot, table); > if (include_table((uchar*) hash_key, end - hash_key)) > { > - dump_table(table, database, (uchar*) hash_key, end - hash_key); > + dump_table(table, database, (uchar*) hash_key, end - hash_key, > + table_ctrl); > my_free(order_by); > order_by= 0; > if (opt_dump_triggers && mysql_get_server_version(mysql) >= 50009) > @@ -5745,11 +5813,24 @@ static int dump_selected_tables(char *db, char **table_names, int tables) > DBUG_RETURN(1); > } > } > + > + if (mysql_get_server_version(mysql) >= FIRST_SEQUENCE_VERSION && > + !opt_no_create_info) > + { > + /* Dump Sequence first */ > + for (pos= dump_tables; pos < end; pos++) > + { > + DBUG_PRINT("info",("Dumping sequence(?) %s", *pos)); > + dump_table(*pos, db, NULL, 0, DUMP_TABLE_SEQUENCE); > + } > + } > + else > + table_ctrl|= DUMP_TABLE_SEQUENCE; // dump all on next pass and here, in dump_selected_tables(), you'd need to do check_if_ignore_table() == IGNORE_SEQUENCE_TABLE. > /* Dump each selected table */ > for (pos= dump_tables; pos < end; pos++) > { > DBUG_PRINT("info",("Dumping table %s", *pos)); > - dump_table(*pos, db, NULL, 0); > + dump_table(*pos, db, NULL, 0, table_ctrl); > if (opt_dump_triggers && > mysql_get_server_version(mysql) >= 50009) > { Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0

[Maria-developers] eb75e8705d9: MDEV-8134: The relay-log is not flushed after the slave-relay-log.999999 showed
by sujatha 21 Jan '21

21 Jan '21

revision-id: eb75e8705d9a444e10057967eaebf947b1115ff8 (mariadb-10.2.31-695-geb75e8705d9) parent(s): 53acd1c1d88be82190c56af3e4cc11fb2770a169 author: Sujatha committer: Sujatha timestamp: 2021-01-21 13:00:02 +0530 message: MDEV-8134: The relay-log is not flushed after the slave-relay-log.999999 showed Problem: ======== Auto purge of relaylogs stops when relay-log-file is 'slave-relay-log.999999' and slave_parallel_threads is enabled. Analysis: ========= The problem is that in Relay_log_info::inc_group_relay_log_pos() function, when two log names are compared via strcmp() function, it gives correct result, when log name sequence numbers are of same digits(6 digits), But when the number goes to 7 digits, a 999999 compares greater than 1000000, which is wrong, hence the bug. Fix: ==== Extract the numeric extension part of the file name, convert it into unsigned long and compare. Thanks to David Zhao for the contribution. --- .../suite/rpl/r/rpl_relay_max_extension.result | 37 +++++++ .../suite/rpl/t/rpl_relay_max_extension.test | 109 +++++++++++++++++++++ sql/rpl_parallel.cc | 5 +- sql/rpl_rli.cc | 4 +- sql/sql_repl.cc | 17 ++++ sql/sql_repl.h | 1 + 6 files changed, 169 insertions(+), 4 deletions(-) diff --git a/mysql-test/suite/rpl/r/rpl_relay_max_extension.result b/mysql-test/suite/rpl/r/rpl_relay_max_extension.result new file mode 100644 index 00000000000..4444398203e --- /dev/null +++ b/mysql-test/suite/rpl/r/rpl_relay_max_extension.result @@ -0,0 +1,37 @@ +include/rpl_init.inc [topology=1->2] +connection server_2; +include/stop_slave.inc +RESET SLAVE; +include/start_slave.inc +include/stop_slave.inc +# +# Stop slave server +# +# +# Simulate file number get close to 999997 +# by renaming relay logs and modifying index/info files +# +# Restart slave server +# +SET @save_slave_parallel_threads= @@GLOBAL.slave_parallel_threads; +SET @save_max_relay_log_size= @@GLOBAL.max_relay_log_size; +SET GLOBAL slave_parallel_threads=1; +SET GLOBAL max_relay_log_size=100 * 1024; +include/start_slave.inc +connection server_1; +create table t1 (i int, c varchar(1024)); +# +# Insert some data to generate enough amount of binary logs +# +connection server_2; +# +# Assert that 'slave-relay-bin.999999' is purged. +# +NOT FOUND /slave-relay-bin.999999/ in slave-relay-bin.index +include/stop_slave.inc +SET GLOBAL slave_parallel_threads= @save_slave_parallel_threads; +SET GLOBAL max_relay_log_size= @save_max_relay_log_size; +include/start_slave.inc +connection server_1; +DROP TABLE t1; +include/rpl_end.inc diff --git a/mysql-test/suite/rpl/t/rpl_relay_max_extension.test b/mysql-test/suite/rpl/t/rpl_relay_max_extension.test new file mode 100644 index 00000000000..e1e087f2e0e --- /dev/null +++ b/mysql-test/suite/rpl/t/rpl_relay_max_extension.test @@ -0,0 +1,109 @@ +# ==== Purpose ==== +# +# Test verifies that auto purging mechanism of relay logs works fine when the +# file extension grows beyond 999999. +# +# ==== Implementation ==== +# +# Steps: +# 0 - In master-slave setup clear all the relay logs on the slave server. +# 1 - Start the slave so that new relay logs starting from +# 'slave-relay-bin.000001' are created. +# 2 - Get the active relay-log file name by using SHOW SLAVE STATUS. +# Shutdown the slave server. +# 3 - Rename active relay log to '999997' in both 'relay-log.info' and +# 'slave-relay-bin.index' files. +# 4 - Restart the slave server by configuring 'slave_parallel_threads=1' +# and 'max_relay_log_size=100K'. +# 5 - Generate load on master such that few relay logs are generated on +# slave. The relay log sequence number will change to 7 digits. +# 6 - Sync slave with master to ensure that relay logs are applied on +# slave. They should have been automatically purged. +# 7 - Assert that there is no 'slave-relay-bin.999999' file in +# 'relay-log.info'. +# +# ==== References ==== +# +# MDEV-8134: The relay-log is not flushed after the slave-relay-log.999999 +# showed +# + +--source include/have_innodb.inc +--source include/have_binlog_format_row.inc +--let $rpl_topology=1->2 +--source include/rpl_init.inc + +--connection server_2 +--source include/stop_slave.inc +RESET SLAVE; +--source include/start_slave.inc +--source include/stop_slave.inc +--let $relay_log=query_get_value(SHOW SLAVE STATUS, Relay_Log_File, 1) + +--echo # +--echo # Stop slave server +--echo # + +--let $datadir = `select @@datadir` +--exec echo "wait" > $MYSQLTEST_VARDIR/tmp/mysqld.2.expect +--shutdown_server 10 +--source include/wait_until_disconnected.inc + +--exec sed -i "s/$relay_log/slave-relay-bin.999997/g" $datadir/relay-log.info +--exec sed -i "s/$relay_log/slave-relay-bin.999997/g" $datadir/slave-relay-bin.index + +--echo # +--echo # Simulate file number get close to 999997 +--echo # by renaming relay logs and modifying index/info files + +--move_file $datadir/$relay_log $datadir/slave-relay-bin.999997 + +--echo # +--echo # Restart slave server +--echo # + +--exec echo "restart" > $MYSQLTEST_VARDIR/tmp/mysqld.2.expect +--enable_reconnect +--source include/wait_until_connected_again.inc +SET @save_slave_parallel_threads= @@GLOBAL.slave_parallel_threads; +SET @save_max_relay_log_size= @@GLOBAL.max_relay_log_size; + +SET GLOBAL slave_parallel_threads=1; +SET GLOBAL max_relay_log_size=100 * 1024; +--source include/start_slave.inc + +--connection server_1 +create table t1 (i int, c varchar(1024)); +--echo # +--echo # Insert some data to generate enough amount of binary logs +--echo # +--let $count = 1000 +--disable_query_log +while ($count) +{ + eval insert into t1 values (1001 - $count, repeat('a',1000)); + dec $count; +} +--enable_query_log +--save_master_pos + +--connection server_2 +--sync_with_master + +--let $relay_log=query_get_value(SHOW SLAVE STATUS, Relay_Log_File, 1) + +--echo # +--echo # Assert that 'slave-relay-bin.999999' is purged. +--echo # +let SEARCH_FILE=$datadir/slave-relay-bin.index; +let SEARCH_PATTERN=slave-relay-bin.999999; +source include/search_pattern_in_file.inc; + +--source include/stop_slave.inc +SET GLOBAL slave_parallel_threads= @save_slave_parallel_threads; +SET GLOBAL max_relay_log_size= @save_max_relay_log_size; +--source include/start_slave.inc + +--connection server_1 +DROP TABLE t1; +--source include/rpl_end.inc diff --git a/sql/rpl_parallel.cc b/sql/rpl_parallel.cc index 4cf87ba73b7..869640fd46f 100644 --- a/sql/rpl_parallel.cc +++ b/sql/rpl_parallel.cc @@ -4,6 +4,7 @@ #include "rpl_mi.h" #include "sql_parse.h" #include "debug_sync.h" +#include "sql_repl.h" /* Code for optional parallel execution of replicated events on the slave. @@ -82,7 +83,7 @@ handle_queued_pos_update(THD *thd, rpl_parallel_thread::queued_event *qev) return; mysql_mutex_lock(&rli->data_lock); - cmp= strcmp(rli->group_relay_log_name, qev->event_relay_log_name); + cmp= compare_log_name(rli->group_relay_log_name, qev->event_relay_log_name); if (cmp < 0) { rli->group_relay_log_pos= qev->future_event_relay_log_pos; @@ -91,7 +92,7 @@ handle_queued_pos_update(THD *thd, rpl_parallel_thread::queued_event *qev) rli->group_relay_log_pos < qev->future_event_relay_log_pos) rli->group_relay_log_pos= qev->future_event_relay_log_pos; - cmp= strcmp(rli->group_master_log_name, qev->future_event_master_log_name); + cmp= compare_log_name(rli->group_master_log_name, qev->future_event_master_log_name); if (cmp < 0) { strcpy(rli->group_master_log_name, qev->future_event_master_log_name); diff --git a/sql/rpl_rli.cc b/sql/rpl_rli.cc index 40ab375571a..5273b33c728 100644 --- a/sql/rpl_rli.cc +++ b/sql/rpl_rli.cc @@ -989,7 +989,7 @@ void Relay_log_info::inc_group_relay_log_pos(ulonglong log_pos, if (rgi->is_parallel_exec) { /* In case of parallel replication, do not update the position backwards. */ - int cmp= strcmp(group_relay_log_name, rgi->event_relay_log_name); + int cmp= compare_log_name(group_relay_log_name, rgi->event_relay_log_name); if (cmp < 0) { group_relay_log_pos= rgi->future_event_relay_log_pos; @@ -1001,7 +1001,7 @@ void Relay_log_info::inc_group_relay_log_pos(ulonglong log_pos, In the parallel case we need to update the master_log_name here, rather than in Rotate_log_event::do_update_pos(). */ - cmp= strcmp(group_master_log_name, rgi->future_event_master_log_name); + cmp= compare_log_name(group_master_log_name, rgi->future_event_master_log_name); if (cmp <= 0) { if (cmp < 0) diff --git a/sql/sql_repl.cc b/sql/sql_repl.cc index 4af8ebc2dd8..59a3f686e45 100644 --- a/sql/sql_repl.cc +++ b/sql/sql_repl.cc @@ -4541,5 +4541,22 @@ rpl_gtid_pos_update(THD *thd, char *str, size_t len) return false; } +int compare_log_name(const char *log_1, const char *log_2) { + int res= 1; + const char *ext1_str= strrchr(log_1, '.'); + const char *ext2_str= strrchr(log_2, '.'); + char file_name_1[255], file_name_2[255]; + strmake(file_name_1, log_1, (ext1_str - log_1)); + strmake(file_name_2, log_2, (ext2_str - log_2)); + char *endptr = NULL; + res= strcmp(file_name_1, file_name_2); + if (!res) + { + ulong ext1= strtoul(++ext1_str, &endptr, 10); + ulong ext2= strtoul(++ext2_str, &endptr, 10); + res= (ext1 > ext2 ? 1 : ((ext1 == ext2) ? 0 : -1)); + } + return res; +} #endif /* HAVE_REPLICATION */ diff --git a/sql/sql_repl.h b/sql/sql_repl.h index 8ddfa9239f6..9129aaeed5e 100644 --- a/sql/sql_repl.h +++ b/sql/sql_repl.h @@ -56,6 +56,7 @@ bool show_binlogs(THD* thd); extern int init_master_info(Master_info* mi); void kill_zombie_dump_threads(uint32 slave_server_id); int check_binlog_magic(IO_CACHE* log, const char** errmsg); +int compare_log_name(const char *log_1, const char *log_2); struct LOAD_FILE_IO_CACHE : public IO_CACHE {

1 0

[Maria-developers] How to change max key length for MyISAM?
by Witold Filipczyk 15 Jan '21

15 Jan '21

Sorry, for private question, but I did not know where to ask. mariadb-10.3.27. I changed in include/myisam.h: #define MI_MAX_KEY_LENGTH 3072 /* Max length in bytes */ but, it did not help, still this ERROR 1071 (42000): Specified key was too long; max key length is 1000 bytes. What more to change to get key length 3072 for MyISAM tables? And what can be negative impact of such a change? Witold Filipczyk

5 8