From ae0509771462c02149e294f8f8593d6ff10755f7 Mon Sep 17 00:00:00 2001 From: Monty Date: Wed, 1 Mar 2023 19:59:42 +0200 Subject: Fixed crashing bug in recursive SQL if write to tmp table would fail This error was discovered while working on MDEV-30540 Wrong result with IN list length reaching IN_PREDICATE_CONVERSION_THRESHOLD If there is read error from handler::ha_rnd_next() during a recursive query, st_select_lex_unit::exec_recursive() will crash as it will try to get the error code from a structure that was deleted by the callee. The code was using the construct: sl->join->exec(); saved_error=sl->join->error; This does not work as sl->join was freed by the exec() and sl->join would be set to 0. Fixed by having JOIN::exec() return the error code. The included test case simulates the error in ha_rnd_next(), which causes a crash without the patch. scovered whle working on MDEV-30540 Wrong result with IN list length reaching IN_PREDICATE_CONVERSION_THRESHOLD If there is read error from handler::ha_rnd_next() during a recursive query, st_select_lex_unit::exec_recursive() will crash as it will try to get the error code from a structure that was deleted by the callee. The code was using the construct: sl->join->exec(); saved_error=sl->join->error; This does not work as sl->join was freed by the exec() and sl->join was set to 0. Fixed by having JOIN::exec() return the error code. The included test case simulates the error in ha_rnd_next(), which causes a crash without the patch. --- mysql-test/suite/maria/crash-recursive.result | 53 +++++++++++++++++++++ mysql-test/suite/maria/crash-recursive.test | 67 +++++++++++++++++++++++++++ sql/debug.cc | 2 +- sql/debug.h | 1 + sql/handler.cc | 10 ++++ sql/item_subselect.cc | 10 ++-- sql/opt_subselect.cc | 4 +- sql/sql_select.cc | 46 +++++++++--------- sql/sql_select.h | 5 +- sql/sql_union.cc | 15 +++--- 10 files changed, 171 insertions(+), 42 deletions(-) create mode 100644 mysql-test/suite/maria/crash-recursive.result create mode 100644 mysql-test/suite/maria/crash-recursive.test diff --git a/mysql-test/suite/maria/crash-recursive.result b/mysql-test/suite/maria/crash-recursive.result new file mode 100644 index 00000000000..998bb9e501b --- /dev/null +++ b/mysql-test/suite/maria/crash-recursive.result @@ -0,0 +1,53 @@ +set @save_big_tables=@@big_tables; +set big_tables=1; +Warnings: +Warning 1287 '@@big_tables' is deprecated and will be removed in a future release +create table folks(id int, name char(32), dob date, father int, mother int); +insert into folks values +(100, 'Me', '2000-01-01', 20, 30), +(20, 'Dad', '1970-02-02', 10, 9), +(30, 'Mom', '1975-03-03', 8, 7), +(10, 'Grandpa Bill', '1940-04-05', null, null), +(9, 'Grandma Ann', '1941-10-15', null, null), +(25, 'Uncle Jim', '1968-11-18', 8, 7), +(98, 'Sister Amy', '2001-06-20', 20, 30), +(7, 'Grandma Sally', '1943-08-23', null, 6), +(8, 'Grandpa Ben', '1940-10-21', null, null), +(6, 'Grandgrandma Martha', '1923-05-17', null, null), +(67, 'Cousin Eddie', '1992-02-28', 25, 27), +(27, 'Auntie Melinda', '1971-03-29', null, null); +call mtr.add_suppression(".*marked as crashed.*"); +SET @saved_dbug= @@SESSION.debug_dbug; +SET SESSION debug_dbug="+d,ha_rnd_next_error"; +SET @ha_rnd_next_error_counter=110; +with recursive +ancestor_couples(h_id, h_name, h_dob, h_father, h_mother, +w_id, w_name, w_dob, w_father, w_mother) +as +( +select h.*, w.* +from folks h, folks w, coupled_ancestors a +where a.father = h.id AND a.mother = w.id +union +select h.*, w.* +from folks v, folks h, folks w +where v.name = 'Me' and +(v.father = h.id AND v.mother= w.id) +), +coupled_ancestors (id, name, dob, father, mother) +as +( +select h_id, h_name, h_dob, h_father, h_mother +from ancestor_couples +union +select w_id, w_name, w_dob, w_father, w_mother +from ancestor_couples +) +select h_name, h_dob, w_name, w_dob +from ancestor_couples; +ERROR HY000: Table '(temporary)' is marked as crashed and should be repaired +drop table folks; +set big_tables=@save_big_tables; +Warnings: +Warning 1287 '@@big_tables' is deprecated and will be removed in a future release +SET @@SESSION.debug_dbug=@saved_dbug; diff --git a/mysql-test/suite/maria/crash-recursive.test b/mysql-test/suite/maria/crash-recursive.test new file mode 100644 index 00000000000..1fa18ce85c0 --- /dev/null +++ b/mysql-test/suite/maria/crash-recursive.test @@ -0,0 +1,67 @@ +# +# This test simulates an error in an aria file discovered during a recursive SQL call. +# The error handling causes used join structures to be deleted, which caused crashes in +# upper levels when trying to access structures that does not exist anymore +# + +--source include/have_debug_sync.inc +--source include/not_embedded.inc + +set @save_big_tables=@@big_tables; +set big_tables=1; + +create table folks(id int, name char(32), dob date, father int, mother int); + +insert into folks values +(100, 'Me', '2000-01-01', 20, 30), +(20, 'Dad', '1970-02-02', 10, 9), +(30, 'Mom', '1975-03-03', 8, 7), +(10, 'Grandpa Bill', '1940-04-05', null, null), +(9, 'Grandma Ann', '1941-10-15', null, null), +(25, 'Uncle Jim', '1968-11-18', 8, 7), +(98, 'Sister Amy', '2001-06-20', 20, 30), +(7, 'Grandma Sally', '1943-08-23', null, 6), +(8, 'Grandpa Ben', '1940-10-21', null, null), +(6, 'Grandgrandma Martha', '1923-05-17', null, null), +(67, 'Cousin Eddie', '1992-02-28', 25, 27), +(27, 'Auntie Melinda', '1971-03-29', null, null); + + +call mtr.add_suppression(".*marked as crashed.*"); +SET @saved_dbug= @@SESSION.debug_dbug; +SET SESSION debug_dbug="+d,ha_rnd_next_error"; +SET @ha_rnd_next_error_counter=110; + +let q= +with recursive +ancestor_couples(h_id, h_name, h_dob, h_father, h_mother, + w_id, w_name, w_dob, w_father, w_mother) +as +( + select h.*, w.* + from folks h, folks w, coupled_ancestors a + where a.father = h.id AND a.mother = w.id + union + select h.*, w.* + from folks v, folks h, folks w + where v.name = 'Me' and + (v.father = h.id AND v.mother= w.id) +), +coupled_ancestors (id, name, dob, father, mother) +as +( + select h_id, h_name, h_dob, h_father, h_mother + from ancestor_couples + union + select w_id, w_name, w_dob, w_father, w_mother + from ancestor_couples +) +select h_name, h_dob, w_name, w_dob + from ancestor_couples; + +--error ER_CRASHED_ON_USAGE +eval $q; +drop table folks; + +set big_tables=@save_big_tables; +SET @@SESSION.debug_dbug=@saved_dbug; \ No newline at end of file diff --git a/sql/debug.cc b/sql/debug.cc index a0e2340e254..a24cc4e407e 100644 --- a/sql/debug.cc +++ b/sql/debug.cc @@ -35,7 +35,7 @@ static const LEX_CSTRING debug_crash_counter= static const LEX_CSTRING debug_error_counter= { STRING_WITH_LEN("debug_error_counter") }; -static bool debug_decrement_counter(const LEX_CSTRING *name) +bool debug_decrement_counter(const LEX_CSTRING *name) { THD *thd= current_thd; user_var_entry *entry= (user_var_entry*) diff --git a/sql/debug.h b/sql/debug.h index 48bae774625..f0eaa79e3c7 100644 --- a/sql/debug.h +++ b/sql/debug.h @@ -31,6 +31,7 @@ #ifndef DBUG_OFF void debug_crash_here(const char *keyword); bool debug_simulate_error(const char *keyword, uint error); +bool debug_decrement_counter(const LEX_CSTRING *name); #else #define debug_crash_here(A) do { } while(0) #define debug_simulate_error(A, B) 0 diff --git a/sql/handler.cc b/sql/handler.cc index 1a4cc630a37..595e76c708b 100644 --- a/sql/handler.cc +++ b/sql/handler.cc @@ -42,6 +42,7 @@ #include #include #include "debug_sync.h" // DEBUG_SYNC +#include "debug.h" // debug_decrement_counter #include "sql_audit.h" #include "ha_sequence.h" #include "rowid_filter.h" @@ -3573,6 +3574,15 @@ int handler::ha_rnd_next(uchar *buf) m_lock_type != F_UNLCK); DBUG_ASSERT(inited == RND); + DBUG_EXECUTE_IF("ha_rnd_next_error", + { + LEX_CSTRING user_var= { STRING_WITH_LEN("ha_rnd_next_error_counter") }; + if (debug_decrement_counter(&user_var)) + { + print_error(HA_ERR_WRONG_IN_RECORD,MYF(0)); + DBUG_RETURN(HA_ERR_WRONG_IN_RECORD); + } + }); do { TABLE_IO_WAIT(tracker, PSI_TABLE_FETCH_ROW, MAX_KEY, result, diff --git a/sql/item_subselect.cc b/sql/item_subselect.cc index 30562b5c12c..80228917210 100644 --- a/sql/item_subselect.cc +++ b/sql/item_subselect.cc @@ -4062,6 +4062,7 @@ int subselect_single_select_engine::exec() char const *save_where= thd->where; SELECT_LEX *save_select= thd->lex->current_select; thd->lex->current_select= select_lex; + bool exec_error= 0; DBUG_ENTER("subselect_single_select_engine::exec"); if (join->optimization_state == JOIN::NOT_OPTIMIZED) @@ -4153,7 +4154,7 @@ int subselect_single_select_engine::exec() } } - join->exec(); + exec_error= join->exec(); /* Enable the optimizations back */ for (JOIN_TAB **ptab= changed_tabs; ptab != last_changed_tab; ptab++) @@ -4171,7 +4172,7 @@ int subselect_single_select_engine::exec() item->make_const(); thd->where= save_where; thd->lex->current_select= save_select; - DBUG_RETURN(join->error || thd->is_fatal_error || thd->is_error()); + DBUG_RETURN(exec_error || thd->is_error()); } thd->where= save_where; thd->lex->current_select= save_select; @@ -5718,9 +5719,8 @@ int subselect_hash_sj_engine::exec() /* The subquery should be optimized, and materialized only once. */ DBUG_ASSERT(materialize_join->optimization_state == JOIN::OPTIMIZATION_DONE && !is_materialized); - materialize_join->exec(); - if (unlikely((res= MY_TEST(materialize_join->error || thd->is_fatal_error || - thd->is_error())))) + res= materialize_join->exec(); + if (unlikely((res= (res || thd->is_error())))) goto err; /* diff --git a/sql/opt_subselect.cc b/sql/opt_subselect.cc index 655ffebab65..c6bbf611da9 100644 --- a/sql/opt_subselect.cc +++ b/sql/opt_subselect.cc @@ -5869,10 +5869,10 @@ enum_nested_loop_state join_tab_execution_startup(JOIN_TAB *tab) ((subselect_hash_sj_engine*)in_subs->engine); if (!hash_sj_engine->is_materialized) { - hash_sj_engine->materialize_join->exec(); + int error= hash_sj_engine->materialize_join->exec(); hash_sj_engine->is_materialized= TRUE; - if (unlikely(hash_sj_engine->materialize_join->error) || + if (unlikely(error) || unlikely(tab->join->thd->is_fatal_error)) DBUG_RETURN(NESTED_LOOP_ERROR); } diff --git a/sql/sql_select.cc b/sql/sql_select.cc index 3054681987e..d4e399975f6 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -382,8 +382,8 @@ void dbug_serve_apcs(THD *thd, int n_calls) Intended usage: DBUG_EXECUTE_IF("show_explain_probe_2", - if (dbug_user_var_equals_int(thd, "select_id", select_id)) - dbug_serve_apcs(thd, 1); + if (dbug_user_var_equals_int(thd, "select_id", select_id)) + dbug_serve_apcs(thd, 1); ); */ @@ -751,7 +751,7 @@ fix_inner_refs(THD *thd, List &all_fields, SELECT_LEX *select, else { for (sum_func= ref->in_sum_func; sum_func && - sum_func->aggr_level >= select->nest_level; + sum_func->aggr_level >= select->nest_level; sum_func= sum_func->in_sum_func) { if (sum_func->aggr_level == select->nest_level) @@ -4657,8 +4657,9 @@ bool JOIN::save_explain_data(Explain_query *output, bool can_overwrite, } -void JOIN::exec() +int JOIN::exec() { + int res; DBUG_EXECUTE_IF("show_explain_probe_join_exec_start", if (dbug_user_var_equals_int(thd, "show_explain_probe_select_id", @@ -4666,7 +4667,7 @@ void JOIN::exec() dbug_serve_apcs(thd, 1); ); ANALYZE_START_TRACKING(thd, &explain->time_tracker); - exec_inner(); + res= exec_inner(); ANALYZE_STOP_TRACKING(thd, &explain->time_tracker); DBUG_EXECUTE_IF("show_explain_probe_join_exec_end", @@ -4675,10 +4676,11 @@ void JOIN::exec() select_lex->select_number)) dbug_serve_apcs(thd, 1); ); + return res; } -void JOIN::exec_inner() +int JOIN::exec_inner() { List *columns_list= &fields_list; DBUG_ENTER("JOIN::exec_inner"); @@ -4714,12 +4716,12 @@ void JOIN::exec_inner() { thd->set_examined_row_count(0); thd->limit_found_rows= 0; - DBUG_VOID_RETURN; + DBUG_RETURN(0); } columns_list= &procedure_fields_list; } if (result->prepare2(this)) - DBUG_VOID_RETURN; + DBUG_RETURN(error); if (!tables_list && (table_count || !select_lex->with_sum_func) && !select_lex->have_window_funcs()) @@ -4733,7 +4735,7 @@ void JOIN::exec_inner() Protocol::SEND_NUM_ROWS | Protocol::SEND_EOF)) { - DBUG_VOID_RETURN; + DBUG_RETURN(error); } /* @@ -4771,7 +4773,7 @@ void JOIN::exec_inner() /* Single select (without union) always returns 0 or 1 row */ thd->limit_found_rows= send_records; thd->set_examined_row_count(0); - DBUG_VOID_RETURN; + DBUG_RETURN(error); } /* @@ -4791,7 +4793,7 @@ void JOIN::exec_inner() if (unlikely(thd->is_error())) { error= thd->is_error(); - DBUG_VOID_RETURN; + DBUG_RETURN(error); } if (zero_result_cause) @@ -4815,7 +4817,7 @@ void JOIN::exec_inner() select_options, zero_result_cause, having ? having : tmp_having, all_fields); - DBUG_VOID_RETURN; + DBUG_RETURN(0); } } @@ -4839,14 +4841,14 @@ void JOIN::exec_inner() if (unlikely(thd->is_error())) { error= thd->is_error(); - DBUG_VOID_RETURN; + DBUG_RETURN(error); } } } if ((this->select_lex->options & OPTION_SCHEMA_TABLE) && get_schema_tables_result(this, PROCESSED_BY_JOIN_EXEC)) - DBUG_VOID_RETURN; + DBUG_RETURN(0); if (select_options & SELECT_DESCRIBE) { @@ -4854,13 +4856,13 @@ void JOIN::exec_inner() order != 0 && !skip_sort_order, select_distinct, !table_count ? "No tables used" : NullS); - DBUG_VOID_RETURN; + DBUG_RETURN(0); } else if (select_lex->pushdown_select) { /* Execute the query pushed into a foreign engine */ error= select_lex->pushdown_select->execute(); - DBUG_VOID_RETURN; + DBUG_RETURN(error); } else { @@ -4879,7 +4881,7 @@ void JOIN::exec_inner() if (unlikely(thd->is_error())) { error= thd->is_error(); - DBUG_VOID_RETURN; + DBUG_RETURN(error); } THD_STAGE_INFO(thd, stage_sending_data); @@ -4894,7 +4896,7 @@ void JOIN::exec_inner() DBUG_PRINT("counts", ("thd->examined_row_count: %lu", (ulong) thd->get_examined_row_count())); - DBUG_VOID_RETURN; + DBUG_RETURN(error); } @@ -5067,7 +5069,7 @@ mysql_select(THD *thd, TABLE_LIST *tables, List &fields, COND *conds, SELECT_LEX_UNIT *unit, SELECT_LEX *select_lex) { int err= 0; - bool free_join= 1; + bool free_join= 1, exec_error= 0; DBUG_ENTER("mysql_select"); if (!fields.is_empty()) @@ -5146,7 +5148,7 @@ mysql_select(THD *thd, TABLE_LIST *tables, List &fields, COND *conds, if (unlikely(thd->is_error())) goto err; - join->exec(); + exec_error= join->exec(); if (thd->lex->describe & DESCRIBE_EXTENDED) { @@ -5166,9 +5168,9 @@ err: { THD_STAGE_INFO(thd, stage_end); err|= (int)(select_lex->cleanup()); - DBUG_RETURN(err || thd->is_error()); + DBUG_RETURN(exec_error || err || thd->is_error()); } - DBUG_RETURN(join->error ? join->error: err); + DBUG_RETURN(exec_error || err); } diff --git a/sql/sql_select.h b/sql/sql_select.h index 5113d1d36aa..452303367e1 100644 --- a/sql/sql_select.h +++ b/sql/sql_select.h @@ -1717,9 +1717,8 @@ public: bool build_explain(); int reinit(); int init_execution(); - void exec(); - - void exec_inner(); + int exec() __attribute__((warn_unused_result)); + int exec_inner(); bool prepare_result(List **columns_list); int destroy(); void restore_tmp(); diff --git a/sql/sql_union.cc b/sql/sql_union.cc index e77023f1fd3..75b7790f557 100644 --- a/sql/sql_union.cc +++ b/sql/sql_union.cc @@ -2155,8 +2155,8 @@ bool st_select_lex_unit::exec() ulonglong add_rows=0; ha_rows examined_rows= 0; bool first_execution= !executed; - DBUG_ENTER("st_select_lex_unit::exec"); bool was_executed= executed; + DBUG_ENTER("st_select_lex_unit::exec"); if (executed && !uncacheable && !describe) DBUG_RETURN(FALSE); @@ -2243,7 +2243,7 @@ bool st_select_lex_unit::exec() if (sl->tvc) sl->tvc->exec(sl); else - sl->join->exec(); + saved_error= sl->join->exec(); if (sl == union_distinct && !have_except_all_or_intersect_all && !(with_element && with_element->is_recursive)) { @@ -2253,8 +2253,6 @@ bool st_select_lex_unit::exec() DBUG_RETURN(TRUE); table->no_keyread=1; } - if (!sl->tvc) - saved_error= sl->join->error; if (likely(!saved_error)) { examined_rows+= thd->get_examined_row_count(); @@ -2401,7 +2399,8 @@ bool st_select_lex_unit::exec() { join->join_examined_rows= 0; saved_error= join->reinit(); - join->exec(); + if (join->exec()) + saved_error= 1; } } @@ -2507,8 +2506,7 @@ bool st_select_lex_unit::exec_recursive() sl->tvc->exec(sl); else { - sl->join->exec(); - saved_error= sl->join->error; + saved_error= sl->join->exec(); } if (likely(!saved_error)) { @@ -2520,11 +2518,10 @@ bool st_select_lex_unit::exec_recursive() DBUG_RETURN(1); } } - if (unlikely(saved_error)) + else { thd->lex->current_select= lex_select_save; goto err; - } } -- cgit v1.2.1