revision-id: 451c3b36edb8f63fa66599bf5deefa9e4494979b (mariadb-10.2.31-561-g451c3b3) parent(s): c7902186129cae888c45a87150d33059528a7033 author: Igor Babaev committer: Igor Babaev timestamp: 2020-11-10 16:02:30 -0800 message: MDEV-23619 MariaDB crash on WITH RECURSIVE UNION ALL (CTE) query Due to a premature cleanup of the unit that specified a recursive CTE used in the second operand of union the server fell into an infinite loop in the reported test case. In other cases this premature cleanup could cause other problems. The bug is the result of a not quite correct fix for MDEV-17024. The unit that specifies a recursive CTE has to be cleaned only after the cleanup of the last external reference to this CTE. It means that cleanups of the unit triggered not by the cleanup of a external reference to the CTE must be blocked. Usage of local table chains in selects to get external references to recursive CTEs was not correct either because of possible merges of some selects. Also fixed a minor bug in st_select_lex::set_explain_type() that caused typing 'RECURSIVE UNION' instead of 'UNION' in EXPLAIN output for external references to a recursive CTE. --- mysql-test/r/cte_recursive.result | 229 +++++++++++++++++++++++++++++++++++++- mysql-test/t/cte_recursive.test | 50 +++++++++ sql/sql_lex.cc | 8 +- sql/sql_select.cc | 5 +- sql/sql_union.cc | 42 +++---- 5 files changed, 309 insertions(+), 25 deletions(-) diff --git a/mysql-test/r/cte_recursive.result b/mysql-test/r/cte_recursive.result index 6404931..85883d3 100644 --- a/mysql-test/r/cte_recursive.result +++ b/mysql-test/r/cte_recursive.result @@ -1301,7 +1301,7 @@ select ancestors.name, ancestors.dob from ancestors; id select_type table type possible_keys key key_len ref rows Extra 1 PRIMARY <derived4> ALL NULL NULL NULL NULL 24 4 DERIVED folks ALL NULL NULL NULL NULL 12 Using where -6 RECURSIVE UNION <derived3> ALL NULL NULL NULL NULL 12 +6 UNION <derived3> ALL NULL NULL NULL NULL 12 5 RECURSIVE UNION <derived4> ALL NULL NULL NULL NULL 24 NULL UNION RESULT <union4,6,5> ALL NULL NULL NULL NULL NULL 3 DERIVED folks ALL NULL NULL NULL NULL 12 Using where @@ -3964,5 +3964,232 @@ YEAR d1 d2 DROP PROCEDURE p; DROP TABLE t1,t2,t3,t4; # +# MDEV-23619: recursive CTE used only in the second operand of UNION +# +create table t1 ( +a bigint(10) not null auto_increment, +b int(5) not null, +c bigint(10) default null, +primary key (a) +) engine myisam; +insert into t1 values +(1,3,12), (2,7,15), (3,1,3), (4,3,1); +explain with recursive r_cte as +( select * from t1 as s +union +select t1.* from t1, r_cte as r where t1.c = r.a ) +select 0 as b FROM dual union all select b FROM r_cte as t; +id select_type table type possible_keys key key_len ref rows Extra +1 PRIMARY NULL NULL NULL NULL NULL NULL NULL No tables used +2 DERIVED s ALL NULL NULL NULL NULL 4 +3 RECURSIVE UNION t1 ALL NULL NULL NULL NULL 4 Using where +3 RECURSIVE UNION <derived2> ref key0 key0 8 test.t1.c 2 +NULL UNION RESULT <union2,3> ALL NULL NULL NULL NULL NULL +4 UNION <derived2> ALL NULL NULL NULL NULL 4 +with recursive r_cte as +( select * from t1 as s +union +select t1.* from t1, r_cte as r where t1.c = r.a ) +select 0 as b FROM dual union all select b FROM r_cte as t; +b +0 +3 +7 +1 +3 +analyze format=json with recursive r_cte as +( select * from t1 as s +union +select t1.* from t1, r_cte as r where t1.c = r.a ) +select 0 as b FROM dual union all select b FROM r_cte as t; +ANALYZE +{ + "query_block": { + "union_result": { + "table_name": "<union1,4>", + "access_type": "ALL", + "r_loops": 0, + "r_rows": null, + "query_specifications": [ + { + "query_block": { + "select_id": 1, + "table": { + "message": "No tables used" + } + } + }, + { + "query_block": { + "select_id": 4, + "r_loops": 1, + "r_total_time_ms": "REPLACED", + "table": { + "table_name": "<derived2>", + "access_type": "ALL", + "r_loops": 1, + "rows": 4, + "r_rows": 4, + "r_total_time_ms": "REPLACED", + "filtered": 100, + "r_filtered": 100, + "materialized": { + "query_block": { + "recursive_union": { + "table_name": "<union2,3>", + "access_type": "ALL", + "r_loops": 0, + "r_rows": null, + "query_specifications": [ + { + "query_block": { + "select_id": 2, + "r_loops": 1, + "r_total_time_ms": "REPLACED", + "table": { + "table_name": "s", + "access_type": "ALL", + "r_loops": 1, + "rows": 4, + "r_rows": 4, + "r_total_time_ms": "REPLACED", + "filtered": 100, + "r_filtered": 100 + } + } + }, + { + "query_block": { + "select_id": 3, + "r_loops": 1, + "r_total_time_ms": "REPLACED", + "table": { + "table_name": "t1", + "access_type": "ALL", + "r_loops": 1, + "rows": 4, + "r_rows": 4, + "r_total_time_ms": "REPLACED", + "filtered": 100, + "r_filtered": 100, + "attached_condition": "t1.c is not null" + }, + "table": { + "table_name": "<derived2>", + "access_type": "ref", + "possible_keys": ["key0"], + "key": "key0", + "key_length": "8", + "used_key_parts": ["a"], + "ref": ["test.t1.c"], + "r_loops": 4, + "rows": 2, + "r_rows": 0.5, + "r_total_time_ms": "REPLACED", + "filtered": 100, + "r_filtered": 100 + } + } + } + ] + } + } + } + } + } + } + ] + } + } +} +prepare stmt from "with recursive r_cte as +( select * from t1 as s +union +select t1.* from t1, r_cte as r where t1.c = r.a ) +select 0 as b FROM dual union all select b FROM r_cte as t"; +execute stmt; +b +0 +3 +7 +1 +3 +execute stmt; +b +0 +3 +7 +1 +3 +deallocate prepare stmt; +#checking hanging cte that uses a recursive cte +explain with h_cte as +( with recursive r_cte as +( select * from t1 as s +union +select t1.* from t1, r_cte as r where t1.c = r.a ) +select 0 as b FROM dual union all select b FROM r_cte as t) +select * from t1 as tt; +id select_type table type possible_keys key key_len ref rows Extra +1 PRIMARY tt ALL NULL NULL NULL NULL 4 +with h_cte as +( with recursive r_cte as +( select * from t1 as s +union +select t1.* from t1, r_cte as r where t1.c = r.a ) +select 0 as b FROM dual union all select b FROM r_cte as t) +select * from t1 as tt; +a b c +1 3 12 +2 7 15 +3 1 3 +4 3 1 +analyze format=json with h_cte as +( with recursive r_cte as +( select * from t1 as s +union +select t1.* from t1, r_cte as r where t1.c = r.a ) +select 0 as b FROM dual union all select b FROM r_cte as t) +select * from t1 as tt; +ANALYZE +{ + "query_block": { + "select_id": 1, + "r_loops": 1, + "r_total_time_ms": "REPLACED", + "table": { + "table_name": "tt", + "access_type": "ALL", + "r_loops": 1, + "rows": 4, + "r_rows": 4, + "r_total_time_ms": "REPLACED", + "filtered": 100, + "r_filtered": 100 + } + } +} +prepare stmt from "with h_cte as +( with recursive r_cte as +( select * from t1 as s +union +select t1.* from t1, r_cte as r where t1.c = r.a ) +select 0 as b FROM dual union all select b FROM r_cte as t) +select * from t1 as tt"; +execute stmt; +a b c +1 3 12 +2 7 15 +3 1 3 +4 3 1 +execute stmt; +a b c +1 3 12 +2 7 15 +3 1 3 +4 3 1 +deallocate prepare stmt; +drop table t1; +# # End of 10.2 tests # diff --git a/mysql-test/t/cte_recursive.test b/mysql-test/t/cte_recursive.test index d190458..082e9be 100644 --- a/mysql-test/t/cte_recursive.test +++ b/mysql-test/t/cte_recursive.test @@ -2641,5 +2641,55 @@ DROP PROCEDURE p; DROP TABLE t1,t2,t3,t4; --echo # +--echo # MDEV-23619: recursive CTE used only in the second operand of UNION +--echo # + +create table t1 ( + a bigint(10) not null auto_increment, + b int(5) not null, + c bigint(10) default null, + primary key (a) +) engine myisam; +insert into t1 values + (1,3,12), (2,7,15), (3,1,3), (4,3,1); + +let $q= +with recursive r_cte as +( select * from t1 as s + union + select t1.* from t1, r_cte as r where t1.c = r.a ) +select 0 as b FROM dual union all select b FROM r_cte as t; + +eval explain $q; +eval $q; +--source include/analyze-format.inc +eval analyze format=json $q; +eval prepare stmt from "$q"; +execute stmt; +execute stmt; +deallocate prepare stmt; + +--echo #checking hanging cte that uses a recursive cte +let $q1= +with h_cte as +( with recursive r_cte as + ( select * from t1 as s + union + select t1.* from t1, r_cte as r where t1.c = r.a ) + select 0 as b FROM dual union all select b FROM r_cte as t) +select * from t1 as tt; + +eval explain $q1; +eval $q1; +--source include/analyze-format.inc +eval analyze format=json $q1; +eval prepare stmt from "$q1"; +execute stmt; +execute stmt; +deallocate prepare stmt; + +drop table t1; + +--echo # --echo # End of 10.2 tests --echo # diff --git a/sql/sql_lex.cc b/sql/sql_lex.cc index e1e3073..77e6b2b 100644 --- a/sql/sql_lex.cc +++ b/sql/sql_lex.cc @@ -4450,9 +4450,11 @@ void st_select_lex::set_explain_type(bool on_the_fly) /* pos_in_table_list=NULL for e.g. post-join aggregation JOIN_TABs. */ - if (tab->table && tab->table->pos_in_table_list && - tab->table->pos_in_table_list->with && - tab->table->pos_in_table_list->with->is_recursive) + if (!(tab->table && tab->table->pos_in_table_list)) + continue; + TABLE_LIST *tbl= tab->table->pos_in_table_list; + if (tbl->with && tbl->with->is_recursive && + tbl->is_with_table_recursive_reference()) { uses_cte= true; break; diff --git a/sql/sql_select.cc b/sql/sql_select.cc index 3b09009..82ff4d3 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -12275,6 +12275,9 @@ void JOIN::join_free() for (tmp_unit= select_lex->first_inner_unit(); tmp_unit; tmp_unit= tmp_unit->next_unit()) + { + if (tmp_unit->with_element && tmp_unit->with_element->is_recursive) + continue; for (sl= tmp_unit->first_select(); sl; sl= sl->next_select()) { Item_subselect *subselect= sl->master_unit()->item; @@ -12292,7 +12295,7 @@ void JOIN::join_free() /* Can't unlock if at least one JOIN is still needed */ can_unlock= can_unlock && full_local; } - + } /* We are not using tables anymore Unlock all tables. We may be in an INSERT .... SELECT statement. diff --git a/sql/sql_union.cc b/sql/sql_union.cc index 8b1719c..9a16237 100644 --- a/sql/sql_union.cc +++ b/sql/sql_union.cc @@ -1370,13 +1370,7 @@ bool st_select_lex_unit::cleanup() { DBUG_RETURN(FALSE); } - /* - When processing a PS/SP or an EXPLAIN command cleanup of a unit can - be performed immediately when the unit is reached in the cleanup - traversal initiated by the cleanup of the main unit. - */ - if (!thd->stmt_arena->is_stmt_prepare() && !thd->lex->describe && - with_element && with_element->is_recursive && union_result) + if (with_element && with_element->is_recursive && union_result) { select_union_recursive *result= with_element->rec_result; if (++result->cleanup_count == with_element->rec_outer_references) @@ -1554,27 +1548,31 @@ bool st_select_lex::cleanup() if (join) { + List_iterator<TABLE_LIST> ti(leaf_tables); + TABLE_LIST *tbl; + while ((tbl= ti++)) + { + if (tbl->is_recursive_with_table() && + !tbl->is_with_table_recursive_reference()) + { + /* + If query is killed before open_and_process_table() for tbl + is called then 'with' is already set, but 'derived' is not. + */ + st_select_lex_unit *unit= tbl->with->spec; + error|= (bool) error | (uint) unit->cleanup(); + } + } DBUG_ASSERT((st_select_lex*)join->select_lex == this); error= join->destroy(); delete join; join= 0; } - for (TABLE_LIST *tbl= get_table_list(); tbl; tbl= tbl->next_local) - { - if (tbl->is_recursive_with_table() && - !tbl->is_with_table_recursive_reference()) - { - /* - If query is killed before open_and_process_table() for tbl - is called then 'with' is already set, but 'derived' is not. - */ - st_select_lex_unit *unit= tbl->with->spec; - error|= (bool) error | (uint) unit->cleanup(); - } - } for (SELECT_LEX_UNIT *lex_unit= first_inner_unit(); lex_unit ; lex_unit= lex_unit->next_unit()) { + if (lex_unit->with_element && lex_unit->with_element->is_recursive) + continue; error= (bool) ((uint) error | (uint) lex_unit->cleanup()); } inner_refs_list.empty(); @@ -1594,8 +1592,12 @@ void st_select_lex::cleanup_all_joins(bool full) join->cleanup(full); for (unit= first_inner_unit(); unit; unit= unit->next_unit()) + { + if (unit->with_element && unit->with_element->is_recursive) + continue; for (sl= unit->first_select(); sl; sl= sl->next_select()) sl->cleanup_all_joins(full); + } DBUG_VOID_RETURN; }