From 9ba8dc1413ff0fac018b5e22cdb5f5a8ff912ab2 Mon Sep 17 00:00:00 2001 From: Michael Widenius Date: Wed, 27 Sep 2023 17:26:24 +0300 Subject: [PATCH] MDEV-32164 Server crashes in JOIN::cleanup after erroneous query with view The problem was that we did not handle errors properly in JOIN::get_best_combination. In case an early error, JOIN->join_tab would contain unintialized values, which would cause errors on cleanup(). The error in question was reported earlier, but not noticed until later. One cause of this is that most of the sql_select.cc code just checks thd->fatal_error and not thd->is_error(). Fixed by changing of checks of fatal_error to is_error(). --- mysql-test/main/view.result | 12 ++++++++++ mysql-test/main/view.test | 16 +++++++++++++ sql/sql_select.cc | 47 ++++++++++++++++++++++--------------- 3 files changed, 56 insertions(+), 19 deletions(-) diff --git a/mysql-test/main/view.result b/mysql-test/main/view.result index 75cfd55f47c..0f47c47bc32 100644 --- a/mysql-test/main/view.result +++ b/mysql-test/main/view.result @@ -7011,5 +7011,17 @@ ERROR HY000: View 'test.v' references invalid table(s) or column(s) or function( DROP VIEW v; DROP FUNCTION f; # +# MDEV-32164 Server crashes in JOIN::cleanup after erroneous query with +# view +# +CREATE TABLE t1 (a INT, b INT, KEY (a,b)) ENGINE=InnoDB; +CREATE VIEW v1 AS SELECT a FROM t1 WHERE a != '' GROUP BY a; +INSERT INTO t1 VALUES (1,NULL),(2,0),(3,NULL); +CREATE TABLE t2 (c INT) ENGINE=InnoDB; +CREATE TEMPORARY TABLE tmp SELECT v1.a FROM v1 JOIN t2 ON (v1.a = t2.c); +ERROR 22007: Truncated incorrect DECIMAL value: '' +DROP VIEW v1; +DROP TABLE t1, t2; +# # End of 10.6 tests # diff --git a/mysql-test/main/view.test b/mysql-test/main/view.test index d6c600c82b1..a4fe17a86f7 100644 --- a/mysql-test/main/view.test +++ b/mysql-test/main/view.test @@ -2,6 +2,7 @@ # Save the initial number of concurrent sessions. --source include/count_sessions.inc --source include/default_optimizer_switch.inc +--source include/have_innodb.inc SET optimizer_switch='outer_join_with_cache=off'; @@ -6773,6 +6774,21 @@ SELECT * FROM v; DROP VIEW v; DROP FUNCTION f; +--echo # +--echo # MDEV-32164 Server crashes in JOIN::cleanup after erroneous query with +--echo # view +--echo # + +CREATE TABLE t1 (a INT, b INT, KEY (a,b)) ENGINE=InnoDB; +CREATE VIEW v1 AS SELECT a FROM t1 WHERE a != '' GROUP BY a; +INSERT INTO t1 VALUES (1,NULL),(2,0),(3,NULL); +CREATE TABLE t2 (c INT) ENGINE=InnoDB; +--error ER_TRUNCATED_WRONG_VALUE +CREATE TEMPORARY TABLE tmp SELECT v1.a FROM v1 JOIN t2 ON (v1.a = t2.c); +# Cleanup +DROP VIEW v1; +DROP TABLE t1, t2; + --echo # --echo # End of 10.6 tests --echo # diff --git a/sql/sql_select.cc b/sql/sql_select.cc index ed3b61a3594..14d6994ccc5 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -2530,7 +2530,7 @@ JOIN::optimize_inner() result->prepare_to_read_rows(); if (unlikely(make_join_statistics(this, select_lex->leaf_tables, &keyuse)) || - unlikely(thd->is_fatal_error)) + unlikely(thd->is_error())) { DBUG_PRINT("error",("Error: make_join_statistics() failed")); DBUG_RETURN(1); @@ -2763,7 +2763,7 @@ int JOIN::optimize_stage2() { ref_item= substitute_for_best_equal_field(thd, tab, ref_item, equals, map2table, true); - if (unlikely(thd->is_fatal_error)) + if (unlikely(thd->is_error())) DBUG_RETURN(1); if (first_inner) @@ -2998,7 +2998,7 @@ int JOIN::optimize_stage2() else group_list= 0; } - else if (thd->is_fatal_error) // End of memory + else if (thd->is_error()) // End of memory DBUG_RETURN(1); } simple_group= rollup.state == ROLLUP::STATE_NONE; @@ -3629,7 +3629,7 @@ bool JOIN::make_aggr_tables_info() curr_tab->all_fields= &tmp_all_fields1; curr_tab->fields= &tmp_fields_list1; - DBUG_RETURN(thd->is_fatal_error); + DBUG_RETURN(thd->is_error()); } } } @@ -3950,7 +3950,7 @@ bool JOIN::make_aggr_tables_info() !join_tab || !join_tab-> is_using_agg_loose_index_scan())) DBUG_RETURN(true); - if (unlikely(setup_sum_funcs(thd, sum_funcs) || thd->is_fatal_error)) + if (unlikely(setup_sum_funcs(thd, sum_funcs) || thd->is_error())) DBUG_RETURN(true); } if (group_list || order) @@ -5861,7 +5861,12 @@ make_join_statistics(JOIN *join, List &tables_list, goto error; records= get_quick_record_count(join->thd, select, s->table, &s->const_keys, join->row_limit); - + if (join->thd->is_error()) + { + /* get_quick_record_count generated an error */ + delete select; + goto error; + } /* Range analyzer might have modified the condition. Put it the new condition to where we got it from. @@ -11194,6 +11199,8 @@ bool JOIN::get_best_combination() table_map used_tables; JOIN_TAB *j; KEYUSE *keyuse; + JOIN_TAB *sjm_nest_end= NULL; + JOIN_TAB *sjm_nest_root= NULL; DBUG_ENTER("get_best_combination"); /* @@ -11248,20 +11255,17 @@ bool JOIN::get_best_combination() DBUG_RETURN(TRUE); if (inject_splitting_cond_for_all_tables_with_split_opt()) - DBUG_RETURN(TRUE); + goto error; JOIN_TAB_RANGE *root_range; if (!(root_range= new (thd->mem_root) JOIN_TAB_RANGE)) - DBUG_RETURN(TRUE); + goto error; root_range->start= join_tab; /* root_range->end will be set later */ join_tab_ranges.empty(); if (join_tab_ranges.push_back(root_range, thd->mem_root)) - DBUG_RETURN(TRUE); - - JOIN_TAB *sjm_nest_end= NULL; - JOIN_TAB *sjm_nest_root= NULL; + goto error; for (j=join_tab, tablenr=0 ; tablenr < table_count ; tablenr++,j++) { @@ -11295,7 +11299,7 @@ bool JOIN::get_best_combination() JOIN_TAB_RANGE *jt_range; if (!(jt= (JOIN_TAB*) thd->alloc(sizeof(JOIN_TAB)*sjm->tables)) || !(jt_range= new JOIN_TAB_RANGE)) - DBUG_RETURN(TRUE); + goto error; jt_range->start= jt; jt_range->end= jt + sjm->tables; join_tab_ranges.push_back(jt_range, thd->mem_root); @@ -11375,7 +11379,7 @@ bool JOIN::get_best_combination() { if ((keyuse= best_positions[tablenr].key) && create_ref_for_key(this, j, keyuse, TRUE, used_tables)) - DBUG_RETURN(TRUE); // Something went wrong + goto error; // Something went wrong } if (j->last_leaf_in_bush) j= j->bush_root_tab; @@ -11389,6 +11393,11 @@ bool JOIN::get_best_combination() update_depend_map(this); DBUG_RETURN(0); + +error: + /* join_tab was not correctly setup. Don't use it */ + join_tab= 0; + DBUG_RETURN(1); } /** @@ -11708,7 +11717,7 @@ static bool create_ref_for_key(JOIN *join, JOIN_TAB *j, keyinfo->key_part[i].length, keyuse->val, FALSE); - if (unlikely(thd->is_fatal_error)) + if (unlikely(thd->is_error())) DBUG_RETURN(TRUE); tmp.copy(thd); j->ref.const_ref_part_map |= key_part_map(1) << i ; @@ -25077,7 +25086,7 @@ create_sort_index(THD *thd, JOIN *join, JOIN_TAB *tab, Filesort *fsort) DBUG_ASSERT(tab->type == JT_REF || tab->type == JT_EQ_REF); // Update ref value if (unlikely(cp_buffer_from_ref(thd, table, &tab->ref) && - thd->is_fatal_error)) + thd->is_error())) goto err; // out of memory } } @@ -26938,7 +26947,7 @@ change_refs_to_tmp_fields(THD *thd, Ref_ptr_array ref_pointer_array, itr++; itr.sublist(res_selected_fields, elements); - return thd->is_fatal_error; + return thd->is_error(); } @@ -27116,7 +27125,7 @@ static bool add_ref_to_table_cond(THD *thd, JOIN_TAB *join_tab) value), thd->mem_root); } - if (unlikely(thd->is_fatal_error)) + if (unlikely(thd->is_error())) DBUG_RETURN(TRUE); if (!cond->fixed()) { @@ -27727,7 +27736,7 @@ int print_explain_message_line(select_result_sink *result, else item_list.push_back(item_null, mem_root); - if (unlikely(thd->is_fatal_error) || unlikely(result->send_data(item_list))) + if (unlikely(thd->is_error()) || unlikely(result->send_data(item_list))) return 1; return 0; }