Skip to content

Commit

Permalink
MDEV-32164 Server crashes in JOIN::cleanup after erroneous query with…
Browse files Browse the repository at this point in the history
… 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().
  • Loading branch information
montywi committed Oct 3, 2023
1 parent d434717 commit 9ba8dc1
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 19 deletions.
12 changes: 12 additions & 0 deletions mysql-test/main/view.result
Expand Up @@ -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
#
16 changes: 16 additions & 0 deletions mysql-test/main/view.test
Expand Up @@ -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';

Expand Down Expand Up @@ -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 #
47 changes: 28 additions & 19 deletions sql/sql_select.cc
Expand Up @@ -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);
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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());
}
}
}
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -5861,7 +5861,12 @@ make_join_statistics(JOIN *join, List<TABLE_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.
Expand Down Expand Up @@ -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");

/*
Expand Down Expand Up @@ -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++)
{
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand All @@ -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);
}

/**
Expand Down Expand Up @@ -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 ;
Expand Down Expand Up @@ -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
}
}
Expand Down Expand Up @@ -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();
}


Expand Down Expand Up @@ -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())
{
Expand Down Expand Up @@ -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;
}
Expand Down

0 comments on commit 9ba8dc1

Please sign in to comment.