|
Note that if one changes UPDATE to be a SELECT, it doesn't crash:
MariaDB [j4]> explain select * from t1,t2 WHERE t2.b IN (SELECT MIN(d) FROM t3 WHERE c >= '2012-01');
|
+------+--------------+-------------+------+---------------+------+---------+------+------+--------------------------------------------------------+
|
| id | select_type | table | type | possible_keys | key | key_len | ref | rows | Extra |
|
+------+--------------+-------------+------+---------------+------+---------+------+------+--------------------------------------------------------+
|
| 1 | PRIMARY | <subquery2> | ALL | distinct_key | NULL | NULL | NULL | 1 | |
|
| 1 | PRIMARY | t1 | ALL | NULL | NULL | NULL | NULL | 2 | Using join buffer (flat, BNL join) |
|
| 1 | PRIMARY | t2 | ALL | NULL | NULL | NULL | NULL | 2 | Using where; Using join buffer (incremental, BNL join) |
|
| 2 | MATERIALIZED | t3 | ALL | c | NULL | NULL | NULL | 2 | Using where |
|
+------+--------------+-------------+------+---------------+------+---------+------+------+--------------------------------------------------------+
|
.
as for the UPDATE query, indeed it does hit this in SQL_SELECT::test_quick_select.
else if (thd->is_error())
|
{
|
thd->no_errors=0;
|
thd->mem_root= param.old_root;
|
free_root(&alloc, MYF(0));
|
=> DBUG_RETURN(-1);
|
.
I don't immediately see a reason why UPDATE would work one way while SELECT work the other way? Johnston could you please check this? Looks like a bug...
Another bug is the behavior after we've got the error.
We continue to optimize the subquery as if the error didn't happen but range optimizer returned "Impossible condition".
We only hit the error accidentally here in Item_cond::fix_fields
=> if (fix_length_and_dec() || thd->is_error())
|
return TRUE;
|
when invoked from here:
#0 Item_cond::fix_fields (this=0x7fff68079780, thd=0x7fff68000d78, ref=0x7fff68079508) at /home/psergey/dev-git2/10.6-dbg/sql/item_cmpfunc.cc:4972
|
#1 0x000055555634e491 in subselect_hash_sj_engine::make_semi_join_conds (this=0x7fff680794b8) at /home/psergey/dev-git2/10.6-dbg/sql/item_subselect.cc:5316
|
#2 0x000055555634df4a in subselect_hash_sj_engine::init (this=0x7fff680794b8, tmp_columns=0x7fff68017e80, subquery_id=2) at /home/psergey/dev-git2/10.6-dbg/sql/item_subselect.cc:5232
|
#3 0x0000555556349d42 in Item_in_subselect::setup_mat_engine (this=0x7fff68019660) at /home/psergey/dev-git2/10.6-dbg/sql/item_subselect.cc:3606
|
#4 0x00005555560e67bc in JOIN::choose_subquery_plan (this=0x7fff6801a320, join_tables=0) at /home/psergey/dev-git2/10.6-dbg/sql/opt_subselect.cc:6701
|
#5 0x0000555555f0e334 in make_join_statistics (join=0x7fff6801a320, tables_list=@0x7fff68017de0: {<base_list> = {<Sql_alloc> = {<No data fields>}, first = 0x7fff6801a8d0, last = 0x7fff6801a8d0, elements = 1}, <N
|
#6 0x0000555555f01473 in JOIN::optimize_inner (this=0x7fff6801a320) at /home/psergey/dev-git2/10.6-dbg/sql/sql_select.cc:2531
|
#7 0x0000555555efecae in JOIN::optimize (this=0x7fff6801a320) at /home/psergey/dev-git2/10.6-dbg/sql/sql_select.cc:1868
|
#8 0x000055555633e287 in Item_in_subselect::optimize (this=0x7fff68019660, out_rows=0x7ffff42b17f8, cost=0x7ffff42b1800) at /home/psergey/dev-git2/10.6-dbg/sql/item_subselect.cc:849
|
#9 0x00005555560e5a70 in setup_jtbm_semi_joins (join=0x7fff68019ad8, join_list=0x7fff68005af8, eq_list=@0x7ffff42b1940: {<base_list> = {<Sql_alloc> = {<No data fields>}, first = 0x555557a57c60 <end_of_list>, las
|
#10 0x0000555555f007e4 in JOIN::optimize_inner (this=0x7fff68019ad8) at /home/psergey/dev-git2/10.6-dbg/sql/sql_select.cc:2308
|
#11 0x0000555555efecae in JOIN::optimize (this=0x7fff68019ad8) at /home/psergey/dev-git2/10.6-dbg/sql/sql_select.cc:1868
|
#12 0x0000555555f0aa8d in mysql_select (thd=0x7fff68000d78, tables=0x7fff68016a60, fields=@0x7ffff42b1b90: {<base_list> = {<Sql_alloc> = {<No data fields>}, first = 0x555557a57c60 <end_of_list>, last = 0x7ffff42b
|
#13 0x0000555555ff1dc0 in mysql_multi_update (thd=0x7fff68000d78, table_list=0x7fff68016a60, fields=0x7fff68005bf8, values=0x7fff68006048, conds=0x7fff68019660, options=4, handle_duplicates=DUP_ERROR, ignore=fals
|
#14 0x0000555555eb1c83 in mysql_execute_command (thd=0x7fff68000d78, is_called_from_prepared_stmt=false) at /home/psergey/dev-git2/10.6-dbg/sql/sql_parse.cc:4504
|
Based on that, the code in JOIN::choose_subquery_plan decides to make materialization non-applicable:
if (in_subs->test_strategy(SUBS_MATERIALIZATION) &&
|
in_subs->setup_mat_engine())
|
{
|
/*
|
If materialization was the cheaper or the only user-selected strategy,
|
but it is not possible to execute it due to limitations in the
|
implementation, fall back to IN-TO-EXISTS.
|
*/
|
=> in_subs->set_strategy(SUBS_IN_TO_EXISTS);
|
}
|
and then we fail an assert in setup_jtbm_semi_joins(), because it requires that Materialization is applicable while it should not be.
|
|
sorry, I should have documented this better. The error is flagged in a call from Field_datetime::store_TIME_with_warning().
In the attached update example...
#0 Field_datetime::store_TIME_with_warning (this=0x7f334005a190, dt=0x7f3384191fb0, str=0x7f3384191fe0, was_cut=1) at /home/rex/src/mariadb/server.10.6/sql/field.cc:5833
|
#1 0x000055747233a202 in Field_datetime::store (this=0x7f334005a190, from=0x7f334005e150 "2012-01", len=7, cs=0x557473b319e0 <my_charset_utf8mb3_general_ci>)
|
at /home/rex/src/mariadb/server.10.6/sql/field.cc:5851
|
#2 0x000055747238bf5e in Item::save_str_value_in_field (this=0x7f33400449b0, field=0x7f334005a190, result=0x7f33400449d8) at /home/rex/src/mariadb/server.10.6/sql/item.cc:408
|
#3 0x00005574723a1636 in Item_string::save_in_field (this=0x7f33400449b0, field=0x7f334005a190, no_conversions=true) at /home/rex/src/mariadb/server.10.6/sql/item.cc:6830
|
#4 0x000055747238f949 in Item::save_in_field_no_warnings (this=0x7f33400449b0, field=0x7f334005a190, no_conversions=true) at /home/rex/src/mariadb/server.10.6/sql/item.cc:1514
|
#5 0x000055747253097d in Field_temporal::get_mm_leaf (this=0x7f334005a190, prm=0x7f3384192920, key_part=0x7f334005b550, cond=0x7f334007aeb0, op=SCALAR_CMP_GE, value=0x7f33400449b0)
|
at /home/rex/src/mariadb/server.10.6/sql/opt_range.cc:8894
|
#6 0x00005574725303e6 in Item_bool_func::get_mm_leaf (this=0x7f334007aeb0, param=0x7f3384192920, field=0x7f334005a190, key_part=0x7f334005b550, functype=Item_func::GE_FUNC,
|
value=0x7f33400449b0) at /home/rex/src/mariadb/server.10.6/sql/opt_range.cc:8818
|
#7 0x000055747252f7b7 in Item_bool_func::get_mm_parts (this=0x7f334007aeb0, param=0x7f3384192920, field=0x7f334005a190, type=Item_func::GE_FUNC, value=0x7f33400449b0)
|
at /home/rex/src/mariadb/server.10.6/sql/opt_range.cc:8653
|
#8 0x0000557471f001d2 in Item_bool_func2_with_rev::get_func_mm_tree (this=0x7f334007aeb0, param=0x7f3384192920, field=0x7f334005a190, value=0x7f33400449b0)
|
at /home/rex/src/mariadb/server.10.6/sql/item_cmpfunc.h:497
|
#9 0x000055747252e53b in Item_bool_func::get_full_func_mm_tree (this=0x7f334007aeb0, param=0x7f3384192920, field_item=0x7f334007efa0, value=0x7f33400449b0)
|
at /home/rex/src/mariadb/server.10.6/sql/opt_range.cc:8312
|
#10 0x0000557471effe04 in Item_bool_func::get_full_func_mm_tree_for_args (this=0x7f334007aeb0, param=0x7f3384192920, item=0x7f334007efa0, value=0x7f33400449b0)
|
at /home/rex/src/mariadb/server.10.6/sql/item_cmpfunc.h:208
|
#11 0x0000557471f003d5 in Item_bool_func2_with_rev::get_mm_tree (this=0x7f334007aeb0, param=0x7f3384192920, cond_ptr=0x7f3340045388)
|
at /home/rex/src/mariadb/server.10.6/sql/item_cmpfunc.h:525
|
#12 0x000055747251ff78 in SQL_SELECT::test_quick_select (this=0x7f3340045380, thd=0x7f334000f7f8, keys_to_use=..., prev_tables=0, limit=18446744073709551615, force_quick_range=false,
|
ordered_output=false, remove_false_parts_of_where=true, only_single_index_range_scan=false) at /home/rex/src/mariadb/server.10.6/sql/opt_range.cc:2892
|
#13 0x0000557471ffdcc1 in get_quick_record_count (thd=0x7f334000f7f8, select=0x7f3340045380, table=0x7f334005e5c8, keys=0x7f334007d400, limit=18446744073709551615)
|
at /home/rex/src/mariadb/server.10.6/sql/sql_select.cc:5135
|
#14 0x0000557472000612 in make_join_statistics (join=0x7f3340079720, tables_list=..., keyuse_array=0x7f3340079a48) at /home/rex/src/mariadb/server.10.6/sql/sql_select.cc:5862
|
#15 0x0000557471ff4157 in JOIN::optimize_inner (this=0x7f3340079720) at /home/rex/src/mariadb/server.10.6/sql/sql_select.cc:2531
|
#16 0x0000557471ff18cc in JOIN::optimize (this=0x7f3340079720) at /home/rex/src/mariadb/server.10.6/sql/sql_select.cc:1868
|
#17 0x0000557472462ade in Item_in_subselect::optimize (this=0x7f334007b970, out_rows=0x7f3384193798, cost=0x7f33841937a0) at /home/rex/src/mariadb/server.10.6/sql/item_subselect.cc:849
|
#18 0x00005574721f252f in setup_jtbm_semi_joins (join=0x7f3340079130, join_list=0x7f3340014578, eq_list=...) at /home/rex/src/mariadb/server.10.6/sql/opt_subselect.cc:6404
|
#19 0x0000557471ff3426 in JOIN::optimize_inner (this=0x7f3340079130) at /home/rex/src/mariadb/server.10.6/sql/sql_select.cc:2308
|
#20 0x0000557471ff18cc in JOIN::optimize (this=0x7f3340079130) at /home/rex/src/mariadb/server.10.6/sql/sql_select.cc:1868
|
#21 0x0000557471ffd995 in mysql_select (thd=0x7f334000f7f8, tables=0x7f3340044b50, fields=..., conds=0x7f334007b970, og_num=0, order=0x0, group=0x0, having=0x0, proc_param=0x0,
|
select_options=2200096997504, result=0x7f334005b3c0, unit=0x7f3340013ba0, select_lex=0x7f33400143c0) at /home/rex/src/mariadb/server.10.6/sql/sql_select.cc:5077
|
#22 0x00005574720f0077 in mysql_multi_update (thd=0x7f334000f7f8, table_list=0x7f3340044b50, fields=0x7f3340014678, values=0x7f3340014ac8, conds=0x7f334007b970, options=0,
|
handle_duplicates=DUP_ERROR, ignore=false, unit=0x7f3340013ba0, select_lex=0x7f33400143c0, result=0x7f3384193eb0) at /home/rex/src/mariadb/server.10.6/sql/sql_update.cc:1976
|
#23 0x0000557471fa1c18 in mysql_execute_command (thd=0x7f334000f7f8, is_called_from_prepared_stmt=false) at /home/rex/src/mariadb/server.10.6/sql/sql_parse.cc:4504
|
we need to catch this error ASAP, certainly before any memory is allocated by for example the creation of a temporary table
0x17E4A51: my_malloc (my_malloc.c:91)
|
0x17D4211: alloc_root (my_alloc.c:198)
|
0x17D444D: multi_alloc_root (my_alloc.c:332)
|
0xB283A9: Create_tmp_table::start(THD*, TMP_TABLE_PARAM*, st_mysql_const_lex_string const*) (sql_select.cc:19378)
|
0xB2C503: create_tmp_table(THD*, TMP_TABLE_PARAM*, List<Item>&, st_order*, bool, bool, unsigned long long, unsigned long long, st_mysql_const_lex_string const*, bool, bool) (sql_select.cc:20234)
|
0xA24A10: select_materialize_with_stats::create_result_table(THD*, List<Item>*, bool, unsigned long long, st_mysql_const_lex_string const*, bool, bool, bool, unsigned int) (sql_class.cc:4336)
|
0xF763F4: subselect_hash_sj_engine::init(List<Item>*, unsigned int) (item_subselect.cc:5188)
|
0xF72410: Item_in_subselect::setup_mat_engine() (item_subselect.cc:3606)
|
0xCF62D1: JOIN::choose_subquery_plan(unsigned long long) (opt_subselect.cc:6701)
|
0xB04331: make_join_statistics(JOIN*, List<TABLE_LIST>&, st_dynamic_array*) (sql_select.cc:6019)
|
0xAF7156: JOIN::optimize_inner() (sql_select.cc:2531)
|
Looking at get_quick_record_count(), it is defined to return either an estimate of the number of rows, zero or HA_POS_ERROR
Underneath this, we have SQL_SELECT::test_quick_select() in which we call cond->get_mm_tree() setting the thread error flag and returning a null pointer, catching the error, freeing some memory and returning -1.
This is then caught in get_quick_record_count(), sets table->reginfo.impossible_range and returns 0;
Here is where the return error code is discarded (unable to return -1 as the return code is unsigned, impossible_range doesn't necessarily mean there is an error). Right after this call is the logical place to re-acquire this error condition, or we could modify the return code of get_quick_record_count().
As to the reason the equivalent select doesn't crash...
Field::set_datetime_warning() checks to see if thd->abort_on_warning is true.
In the case of a select, it defaults to false, so thd->is_error() is also false after this SEL_TREE (failed) construction.
(as an aside, a lot of un-needed code is executed because of this).
compare this with the beginning of mysql_multi_update(), where
thd->abort_on_warning set to !ignore && thd->is_strict_mode()
ignore is false from lex->ignore and thd->is_strict_mode() is true from global_system_variables & MODE_STRICT_TRANS_TABLES.
So here thd->abort_on_warning is true, causing thd->is_error() to be true.
Because thd->is_error() is true, in_subs->setup_mat_engine() fails (attempted as SUBS_MATERIALIZATION is a strategy option), this fails, it removes this option from Item_in_subselect::in_strategy, leaving only SUBS_IN_TO_EXISTS in the bitmap.
setup_jtbm_semi_joins() then is called in JOIN::optimize_inner() which expects to be able to use the failed materialization engine so is protected by the failing DBUG_ASSERT(subq_pred->test_set_strategy(SUBS_MATERIALIZATION));
|
|
Hi @Rex.
Thanks for the clarification about different behavior between SELECT and UPDATE.
I that the variant in bb-10.6-MDEV-31983 is error-prone.
For example, there is a call to select->test_quick_select() in JOIN::make_range_rowid_filters() and it does check for thd->is_error(), but then
just skips the rowid filter for this JOIN_TAB and continues execution...
As for test_if_skip_sort_order(), it doesn't seem to check anything after test_quick_select() call.
The second variant is better but I would argue for three return values:
1. OK, which combines QUICK_SELECT_CANT_USE and QUICK_SELECT_SUCCESS. The caller can check SQL_SELECT::quick to see if quick select was constructed.
2. ERROR, which means "Unrecoverable error, stop processing now". (This is important as InnoDB for example does not tolerate a storage
engine call after it has returned certain kinds of errors, like lock wait timeout. And the code in SQL_SELECT::test_quick_select() can hit that)
3. IMPOSSIBLE, which means "Impossible condition" and so can require special handling.
If we define the enum inside SQL_SELECT, we don't need QUICK_SELECT_ prefix, it makes the condition repetitive:
> + if (error == SQL_SELECT::QUICK_SELECT_IMPOSSIBLE_RANGE)
|
A comment about get_quick_record_count():
> -static ha_rows get_quick_record_count(THD *thd, SQL_SELECT *select,
|
> +static bool get_quick_record_count(THD *thd, SQL_SELECT *select,
|
> TABLE *table,
|
> - const key_map *keys,ha_rows limit)
|
> + const key_map *keys,ha_rows limit,
|
> + ha_rows &quick_count)
|
Please change it to be 'ha_rows *quick_count'.
It is a convention to avoid using references (especially for OUT parameters).
The rationale is that this
makes it much more apparent that &var will be modified than this:
After the patch, there is still code in sql_select.cc with checks like:
if (sel->test_quick_select(...) < 0)
|
...
|
Please go through all calls to test_quick_select and change accordingly.
I'd like to make another review pass after all this is done.
|