Details
-
Bug
-
Status: Closed (View Workflow)
-
Blocker
-
Resolution: Fixed
-
10.6, 10.10(EOL), 10.11, 11.0(EOL), 11.1(EOL)
Description
CREATE TABLE t1 (a INT); |
INSERT INTO t1 VALUES (1),(2); |
|
|
CREATE TABLE t2 (b INT); |
INSERT INTO t2 VALUES (3),(4); |
|
|
CREATE TABLE t3 (c DATETIME, d INT, KEY(c)); |
INSERT INTO t3 VALUES ('2012-11-11',5),('2012-12-12',6); |
|
|
--error ER_TRUNCATED_WRONG_VALUE
|
UPDATE t1, t2 SET t1.a = 26 WHERE t2.b IN (SELECT MIN(d) FROM t3 WHERE c >= '2012-01'); |
|
|
# Cleanup
|
DROP TABLE t1, t2, t3; |
|
10.6 a60462d9 |
mariadbd: /data/src/10.6/sql/opt_subselect.cc:6420: bool setup_jtbm_semi_joins(JOIN*, List<TABLE_LIST>*, List<Item>&): Assertion `subq_pred->test_set_strategy(8)' failed.
|
230822 17:41:58 [ERROR] mysqld got signal 6 ;
|
|
|
#8 0x00007ff689c45395 in __assert_fail_base (fmt=0x7ff689db9a70 "%s%s%s:%u: %s%sAssertion `%s' failed.\n%n", assertion=assertion@entry=0x55e7716e58a0 "subq_pred->test_set_strategy(8)", file=file@entry=0x55e7716e1c00 "/data/src/10.6/sql/opt_subselect.cc", line=line@entry=6420, function=function@entry=0x55e7716e5820 "bool setup_jtbm_semi_joins(JOIN*, List<TABLE_LIST>*, List<Item>&)") at ./assert/assert.c:92
|
#9 0x00007ff689c53df2 in __GI___assert_fail (assertion=0x55e7716e58a0 "subq_pred->test_set_strategy(8)", file=0x55e7716e1c00 "/data/src/10.6/sql/opt_subselect.cc", line=6420, function=0x55e7716e5820 "bool setup_jtbm_semi_joins(JOIN*, List<TABLE_LIST>*, List<Item>&)") at ./assert/assert.c:101
|
#10 0x000055e76f9fc69c in setup_jtbm_semi_joins (join=0x62b000088578, join_list=0x62b000082f98, eq_list=...) at /data/src/10.6/sql/opt_subselect.cc:6420
|
#11 0x000055e76f549629 in JOIN::optimize_inner (this=0x62b000088578) at /data/src/10.6/sql/sql_select.cc:2308
|
#12 0x000055e76f54477f in JOIN::optimize (this=0x62b000088578) at /data/src/10.6/sql/sql_select.cc:1868
|
#13 0x000055e76f5662f6 in mysql_select (thd=0x62b00007e218, tables=0x62b0000853b8, fields=..., conds=0x62b0000880f8, og_num=0, order=0x0, group=0x0, having=0x0, proc_param=0x0, select_options=2200096997504, result=0x62b000088438, unit=0x62b0000825c0, select_lex=0x62b000082de0) at /data/src/10.6/sql/sql_select.cc:5077
|
#14 0x000055e76f7bb6ef in mysql_multi_update (thd=0x62b00007e218, table_list=0x62b0000853b8, fields=0x62b000083098, values=0x62b0000834e8, conds=0x62b0000880f8, options=0, handle_duplicates=DUP_ERROR, ignore=false, unit=0x62b0000825c0, select_lex=0x62b000082de0, result=0x7ff682464f80) at /data/src/10.6/sql/sql_update.cc:1976
|
#15 0x000055e76f48c6fa in mysql_execute_command (thd=0x62b00007e218, is_called_from_prepared_stmt=false) at /data/src/10.6/sql/sql_parse.cc:4504
|
#16 0x000055e76f4a5827 in mysql_parse (thd=0x62b00007e218, rawbuf=0x62b000085238 "UPDATE t1, t2 SET t1.a = 26 WHERE t2.b IN (SELECT MIN(d) FROM t3 WHERE c >= '2012-01')", length=86, parser_state=0x7ff6824659f0) at /data/src/10.6/sql/sql_parse.cc:8053
|
#17 0x000055e76f47b4e8 in dispatch_command (command=COM_QUERY, thd=0x62b00007e218, packet=0x62900025d219 "UPDATE t1, t2 SET t1.a = 26 WHERE t2.b IN (SELECT MIN(d) FROM t3 WHERE c >= '2012-01')", packet_length=86, blocking=true) at /data/src/10.6/sql/sql_parse.cc:1896
|
#18 0x000055e76f47821c in do_command (thd=0x62b00007e218, blocking=true) at /data/src/10.6/sql/sql_parse.cc:1409
|
#19 0x000055e76f8e46ec in do_handle_one_connection (connect=0x608000002db8, put_in_cache=true) at /data/src/10.6/sql/sql_connect.cc:1416
|
#20 0x000055e76f8e40ad in handle_one_connection (arg=0x608000002d38) at /data/src/10.6/sql/sql_connect.cc:1318
|
#21 0x000055e770540b14 in pfs_spawn_thread (arg=0x617000005b98) at /data/src/10.6/storage/perfschema/pfs.cc:2201
|
#22 0x00007ff689ca7fd4 in start_thread (arg=<optimized out>) at ./nptl/pthread_create.c:442
|
#23 0x00007ff689d285bc in clone3 () at ../sysdeps/unix/sysv/linux/x86_64/clone3.S:81
|
|
10.6 non-debug ca5c122a |
#2 <signal handler called>
|
#3 0x000056307c3b6f96 in setup_table_map (tablenr=<optimized out>, table_list=0x7fe924015980, table=0x7fe924011cb8) at /data/src/10.6/sql/sql_base.h:361
|
#4 setup_jtbm_semi_joins (join=join@entry=0x7fe924013b78, join_list=<optimized out>, eq_list=...) at /data/src/10.6/sql/opt_subselect.cc:6428
|
#5 0x000056307c2c8c1a in JOIN::optimize_inner (this=this@entry=0x7fe924013b78) at /data/src/10.6/sql/sql_select.cc:2308
|
#6 0x000056307c2c989a in JOIN::optimize (this=this@entry=0x7fe924013b78) at /data/src/10.6/sql/sql_select.cc:1868
|
#7 0x000056307c2c9963 in mysql_select (thd=thd@entry=0x7fe924000c68, tables=tables@entry=0x7fe924010b50, fields=..., conds=conds@entry=0x7fe924013750, og_num=<optimized out>, order=<optimized out>, group=<optimized out>, having=<optimized out>, proc_param=<optimized out>, select_options=<optimized out>, result=<optimized out>, unit=<optimized out>, select_lex=<optimized out>) at /data/src/10.6/sql/sql_select.cc:5077
|
#8 0x000056307c32f55f in mysql_multi_update (thd=thd@entry=0x7fe924000c68, table_list=0x7fe924010b50, fields=fields@entry=0x7fe924005928, values=values@entry=0x7fe924005d78, conds=<optimized out>, options=<optimized out>, handle_duplicates=<optimized out>, ignore=<optimized out>, unit=<optimized out>, select_lex=<optimized out>, result=<optimized out>) at /data/src/10.6/sql/sql_update.cc:1976
|
#9 0x000056307c25d54d in mysql_execute_command (thd=thd@entry=0x7fe924000c68, is_called_from_prepared_stmt=is_called_from_prepared_stmt@entry=false) at /data/src/10.6/sql/sql_parse.cc:4504
|
#10 0x000056307c260366 in mysql_parse (thd=0x7fe924000c68, rawbuf=<optimized out>, length=<optimized out>, parser_state=<optimized out>) at /data/src/10.6/sql/sql_parse.cc:8053
|
#11 0x000056307c263065 in dispatch_command (command=command@entry=COM_QUERY, thd=thd@entry=0x7fe924000c68, packet=packet@entry=0x7fe9241004a9 "UPDATE t1, t2 SET t1.a = 26 WHERE t2.b IN (SELECT MIN(d) FROM t3 WHERE c >= '2012-01')", packet_length=packet_length@entry=86, blocking=blocking@entry=true) at /data/src/10.6/sql/sql_parse.cc:1993
|
#12 0x000056307c264874 in do_command (thd=0x7fe924000c68, blocking=blocking@entry=true) at /data/src/10.6/sql/sql_parse.cc:1409
|
#13 0x000056307c373a97 in do_handle_one_connection (connect=<optimized out>, connect@entry=0x56307f4aa258, put_in_cache=put_in_cache@entry=true) at /data/src/10.6/sql/sql_connect.cc:1416
|
#14 0x000056307c373e05 in handle_one_connection (arg=arg@entry=0x56307f4aa258) at /data/src/10.6/sql/sql_connect.cc:1318
|
#15 0x000056307c7071f0 in pfs_spawn_thread (arg=0x56307f45e688) at /data/src/10.6/storage/perfschema/pfs.cc:2201
|
#16 0x00007fe9416a7fd4 in start_thread (arg=<optimized out>) at ./nptl/pthread_create.c:442
|
#17 0x00007fe9417285bc in clone3 () at ../sysdeps/unix/sysv/linux/x86_64/clone3.S:81
|
The failure started happening after this commit in 10.6
commit ca5c122adcd39c34b1bd7059903668586496caf6
|
Author: Monty
|
Date: Fri Aug 11 17:59:40 2023 +0300
|
|
|
MDEV-9938 Prepared statement return wrong result (missing row)
|
Attachments
Issue Links
- causes
-
-
- Closed
-
Activity
Johnston , what I don't get is why does it result in a crash? If we've got a truncation warning in get_quick_record_count(), that may cause us to not generate a quick select. But query optimization should be able to proceed after that. Please elaborate...
wondering how the fix for MDEV-9938 could play a role... Did the entire subquery become constant (ie. degenerate)?
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.
It looks lioke Monty's patch for MDEV-9938 triggered this because of it has this part:
- Return an error if Item_cond::fix_field() generates an error
|
The old code did generate an error in some cases, but not in all
|
cases.
|
Note that convert_subq_to_jtbm is invoked (and finishes) before the JOIN::optimize is invoked for the sub-select, so we cannot easily fall-back to not using SJ-Materialization.
But we can easily make at least a coarse check: if thd->is_error()== true, JOIN::optimize() should return true and that should cause the invoker to stop attempts at further processing. Johnston can you try coding that please?
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));
To expand further on the commit that caused this error (ca5c122adcd39c34b1bd7059903668586496caf6), this alteration to Item_cond::fix_fields() is the root cause
if (fix_length_and_dec() || thd->is_error()) |
return TRUE; |
where before it was
if (fix_length_and_dec()) |
return TRUE; |
Note that thd->is_error() is set much earlier, as described above, but ignored until here
Item_cond::fix_fields () at item_cmpfunc.cc:4949
|
subselect_hash_sj_engine::make_semi_join_conds () at item_subselect.cc:5316
|
subselect_hash_sj_engine::init () at item_subselect.cc:5232
|
Item_in_subselect::setup_mat_engine () at item_subselect.cc:3606
|
now that fix_fields is returning an error, setup_mat_engine() is failing (returning TRUE), causing
JOIN::choose_subquery_plan to set it's strategy to SUBS_IN_TO_EXISTS. This error needed to be caught much earlier.
2 commits attached to this MDEV
1) bb-10.6-MDEV-31983, absolute minimal patch to stop the crash
2) bb-10.6-MDEV-31983-v2, catch the error ASAP after evaluating Item(s) with get_mm_tree() and making
SQL_SELECT::test_quick_select() and get_quick_record_count() return possible error conditions.
psergei can you check out these 2 commits when you get a chance? We need to either push the simple fix or work on v2.
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
func_call(... , &var)
|
makes it much more apparent that &var will be modified than this:
func_call(... , var);
|
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.
Hi Johnston,
- Generally the new patch is a step in the right direction.
- I think i have found one bug in it, described below.
- I've fixed it and added more error checking. Please check the last commit in bb-10.6-
MDEV-31983-v3, 4941ac9192394a1489f0bf01b6dd2ee5ec2906c9 (made on top of your commit). Do you see any issues? If not please add this into your patch.
Calling convention before the patch:
|
|
|
SQL_SELECT::test_quick_select()
|
-1 if error or impossible select (i.e. certainly no rows will be selected)
|
0 if can't use quick_select
|
1 if found usable ranges and quick select has been successfully created.
|
|
|
test_if_quick_select() - has the same return value.
|
test_if_use_dynamic_range_scan in the older version:
|
|
|
static
|
bool test_if_use_dynamic_range_scan(JOIN_TAB *join_tab)
|
{
|
return (join_tab->use_quick == 2 && test_if_quick_select(join_tab) > 0);
|
}
|
After the patch:
bool test_if_use_dynamic_range_scan(JOIN_TAB *join_tab)
|
{
|
return (join_tab->use_quick == 2 &&
|
test_if_quick_select(join_tab) == SQL_SELECT::OK);
|
}
|
this is not an equivalent check, we should also check if select has a quick.
one more question: before the patch, in make_join_statistics:
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; |
}
|
'select' is deleted if error condition happens.
while in the new code it is not:
if (get_quick_record_count(join->thd, select, s->table,
|
&s->const_keys, join->row_limit, &records))
|
goto error;
|
|
is this intentional? I would rather leave it as is...
Absolutely not intentional. Monty slipped that in there without me noticing.
The problem was caused by this scenario:
- Take an UPDATE query, unlike SELECT it will treat warnings as errors.
- IN subquery is converted into a non-mergeable semi-join (aka JTBM)
- The optimizer reaches the point where it runs join optimization
for the SELECT inside the semi-join - Range optimizer produces a warning for datetime_key_col >= '2012-01'
- It's an UPDATE so it is treated as error
- The optimizer doesn't catch this error condition, as
SQL_SELECT::test_quick_select() doesn't have a return code for error. - The optimizer continues to run, until we finally detect an error condition
inside Item_in_subselect::setup_mat_engine().
It is caught by accident by the patch forMDEV-9938in Item_cond::fix_fields
- The optimizer interprets this as that we're unable to run Materialization, but
we can't switch back to IN->EXISTs at this point so we fail an assertion.
Latest patch:
https://github.com/MariaDB/server/commit/39638a71adfd414d8616b013e78325c4983eac2d
Alternative commit message:
|
|
MDEV-31983 jointable materialization subquery optimization ignoring
|
|
|
...errors, then failing ASSERT.
|
UPDATE queries treat warnings as errors. In this case, an invalid
|
condition datetime_key_col >= '2012-01' caused warning-as-error inside
|
SQL_SELECT::test_quick_select.
|
|
|
The code that called test_quick_select() ignored this error and continued
|
join optimization. Then it eventually reached a thd->is_error() check
|
and failed to set up SJ-Materialization which failed an assert.
|
|
|
Fixed this by making SQL_SELECT::test_quick_select() return error in its
|
return value, and making any code that calls it to check for error condition
|
and abort the query if the error is returned.
|
in setting up the optimizing jointable materialization, make_join_statistics calls get_quick_record_count, then test_quick_select, in evaluating attached conditions will (eventually) construct a SEL_TREE out of the associated Item tree.
If constructing this SEL_TREE encounters any ignored errors, like in this case ER_TRUNCATED_WRONG_VALUE
setup_jtbm_semi_joins() continues after optimize()-ing it's subquery predicate with unpredictable results.