February 2021 - developers - lists.mariadb.org

[Maria-developers] Re MDEV-7317: Ignored indexes
by Sergey Petrunia 24 Feb '21

24 Feb '21

Hi Varun, The code for the task is mostly fine. But I would like to question the user experience. == Ignoring an index causes InnoDB table rebuild == First (and foremost), making an index ignored (or not ignored) in an InnoDB table rebuilds the table: MariaDB [test]> set session alter_algorithm='instant'; alter table t1 alter index c ignore; Query OK, 0 rows affected (0.001 sec) ERROR 1845 (0A000): ALGORITHM=INSTANT is not supported for this operation. Try ALGORITHM=COPY This misses the point of the feature. If one needs to rebuild the table anyway, they can just drop (or recreate) the index. Curiously, it works with MyISAM. I think, the ideal solution would be to make ignoring/not ignoring the index an SQL layer change that doesn't concern the storage engine. If this is not possible, lets make InnoDB aware that it doesn't need to rebuild the table in this case. == Backward compatibility for SHOW CREATE TABLE == SHOW CREATE TABLE will show something like: KEY `a` (`a`) IGNORE, MySQL here is more backward-compatible and shows: KEY `a` (`a`) /*!80000 INVISIBLE */, Do we need to do this as well? I have no strong opinion here. == Use of Ignored index in hints == I can use Ignored index in FORCE/IGNORE index hints: MariaDB [test]> explain select * from t1 force index(a) where a<3 and b<3; +------+-------------+-------+------+---------------+------+---------+------+------+-------------+ | id | select_type | table | type | possible_keys | key | key_len | ref | rows | Extra | +------+-------------+-------+------+---------------+------+---------+------+------+-------------+ | 1 | SIMPLE | t1 | ALL | NULL | NULL | NULL | NULL | 1000 | Using where | +------+-------------+-------+------+---------------+------+---------+------+------+-------------+ This is allowed. I think, this misses the point of the feature. The point of this feature is to check "what will happen if one drops the index". For this particular query, the effect will be like this: MariaDB [test]> explain select * from t1 force index(no_such_index) where a<3 and b<3; ERROR 1176 (42000): Key 'no_such_index' doesn't exist in table 't1' and MySQL agrees with me here (below index a is present but is invisible) : mysql> explain select * from t1 force index(a) where a<3 and b<3; ERROR 1176 (42000): Key 'a' doesn't exist in table 't1' == optimizer_switch flag == Seeing "ignore_indexes=on" in optimizer_switch looks scary. I would say, we don't need an @@optimizer_switch flag. Maybe, Serg disagrees. If we have to have a switch flag, and its name should be short (my opinion), I would use the noun form: ignored_indexes=ON|OFF I'm not 100% happy with this, but ignoreD is much better here than ignore. (Before acting on this review point, lets check with others). BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://petrunia.net

2 1

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