[MDEV-24823] Crash with invalid multi-table update of view in 2nd execution of SP Created: 2021-02-09  Updated: 2022-11-10  Resolved: 2021-04-25

Status: Closed
Project: MariaDB Server
Component/s: Data Manipulation - Update
Affects Version/s: 5.5, 10.0, 10.1, 10.2, 10.3, 10.4, 10.5
Fix Version/s: 10.2.38, 10.3.29, 10.4.19, 10.5.10, 10.6.1

Type: Bug Priority: Major
Reporter: Alice Sherepa Assignee: Igor Babaev
Resolution: Fixed Votes: 0
Labels: None

Issue Links:
Duplicate
is duplicated by MDEV-17435 Server crashes inst_select_lex::get_f... Closed
Relates
relates to MDEV-16940 Server crashes in unsafe_key_update u... Closed
relates to MDEV-21630 Server crashes in mysql_derived_prepa... Confirmed

 Description   

create table t1 (id int);
insert into t1 values (1),(2),(1); #not important for the test
 
create table t2 (pk int, c0 int)  ;
create view v2 as select * from t2;
insert into t2 values (1,-1); #not important for the test
 
create  procedure sp1() update (t1 join v2 on v2.c0 = t1.c1) set v2.pk = 1;  #--there is no column c1 in t1
 
--error 1054
call sp1;
call sp1;

crash on debug/non-debug builds, with MyIsam/InnoDB

10.2 afc5bac49d48b6fd13d

210209 15:29:54 [ERROR] mysqld got signal 11 ;
 
Server version: 10.2.37-MariaDB-debug-log
 
sql/signal_handler.cc:209(handle_fatal_signal)[0x55d2c693d568]
/lib/x86_64-linux-gnu/libpthread.so.0(+0x12730)[0x7f9a9d78b730]
sql/sql_update.cc:1201(unsafe_key_update(List<TABLE_LIST>, unsigned long long))[0x55d2c6578bd2]
sql/sql_update.cc:1420(Multiupdate_prelocking_strategy::handle_end(THD*))[0x55d2c657a4f5]
sql/sql_base.cc:4192(open_tables(THD*, DDL_options_st const&, TABLE_LIST**, unsigned int*, unsigned int, Prelocking_strategy*))[0x55d2c61b70d2]
sql/sql_base.h:250(open_tables(THD*, TABLE_LIST**, unsigned int*, unsigned int, Prelocking_strategy*))[0x55d2c656ed73]
sql/sql_update.cc:1547(mysql_multi_update_prepare(THD*))[0x55d2c657b3c3]
sql/sql_parse.cc:4071(mysql_execute_command(THD*))[0x55d2c62dd1d0]
sql/sp_head.cc:3332(sp_instr_stmt::exec_core(THD*, unsigned int*))[0x55d2c611af62]
sql/sp_head.cc:3095(sp_lex_keeper::reset_lex_and_exec_core(THD*, unsigned int*, bool, sp_instr*))[0x55d2c6119b1d]
sql/sp_head.cc:3248(sp_instr_stmt::execute(THD*, unsigned int*))[0x55d2c611a80d]
sql/sp_head.cc:1326(sp_head::execute(THD*, bool))[0x55d2c610e778]
sql/sp_head.cc:2202(sp_head::execute_procedure(THD*, List<Item>*))[0x55d2c611345f]
sql/sql_parse.cc:2981(do_execute_sp(THD*, sp_head*))[0x55d2c62d5b48]
sql/sql_parse.cc:5599(mysql_execute_command(THD*))[0x55d2c62e9660]
sql/sql_parse.cc:7763(mysql_parse(THD*, char*, unsigned int, Parser_state*, bool, bool))[0x55d2c62f81d8]
sql/sql_parse.cc:1830(dispatch_command(enum_server_command, THD*, char*, unsigned int, bool, bool))[0x55d2c62cef06]
sql/sql_parse.cc:1381(do_command(THD*))[0x55d2c62cb92b]
sql/sql_connect.cc:1336(do_handle_one_connection(CONNECT*))[0x55d2c66740f2]
sql/sql_connect.cc:1242(handle_one_connection)[0x55d2c66739b3]
perfschema/pfs.cc:1871(pfs_spawn_thread)[0x55d2c7b0d822]
nptl/pthread_create.c:487(start_thread)[0x7f9a9d780fa3]
x86_64/clone.S:97(clone)[0x7f9a9d1044cf]
 
Query (0x625000101990): update (t1 join v2 on v2.c0 = t1.c1) set v2.pk = 1



 Comments   
Comment by Igor Babaev [ 2021-03-26 ]

Here's how the server is brought to the reported crash.
When executing the first call of sp1() the function Multiupdate_prelocking_strategy::handle_end() comes to the code:

  if (select_lex->handle_derived(thd->lex, DT_MERGE))
    DBUG_RETURN(1);
  if (thd->lex->save_prep_leaf_tables())

that merges the specification of the view v2 into

update (t1 join v2 on v2.c0 = t1.c1) set v2.pk = 1;

with the function mysql_derived_merge().
When doing this the function checks the validity of on expression v2.c0 = t1.c1 after having merged the select of v2 into the join of the update statement. When the function discovers that t1.c1 refers to non-existing field it reports an error and returns to Multiupdate_prelocking_strategy::handle_end() that quits immediately without calling save_prep_leaf_tables().
After this we have:
v2->merged == true
thd->lex->prep_leaf_list_state != SAVED
<select_for_update>->first_cond_optimization == true.
When executing the second call of sp1() Multiupdate_prelocking_strategy::handle_end() comes to the code

  if (mysql_handle_derived(lex, DT_INIT) ||
      mysql_handle_derived(lex, DT_MERGE_FOR_INSERT) ||
      mysql_handle_derived(lex, DT_PREPARE))
    DBUG_RETURN(1);
  if (setup_tables_and_check_access(thd, &select_lex->context,
        &select_lex->top_join_list, table_list, select_lex->leaf_tables,
        FALSE, UPDATE_ACL, SELECT_ACL, FALSE))
    DBUG_RETURN(1);

The call of mysql_handle_derived(lex, DT_PREPARE) does not assign v2->table anything because v2 has been merged. The following call of setup_tables_and_check_access() sees that
<select_for_update>->first_cond_optimization == true
<select_for_update>->prep_leaf_list_state != SELECT_LEX::SAVED
and build the list <select_for_update>->leaf_tables as containing t1,v2.
The call select_for_update->handle_derived(thd->lex, DT_MERGE) does not do anything because v2 is merged.
Now we come to the call of unsafe_key_update() with the list of t1,v2 as the the first parameter and the valueof v2->table == NULL. The attempt to get v2->table->map causes a crash.

Comment by Igor Babaev [ 2021-04-03 ]

A similar test case with multi-table delete demonstrates just a minor promlem:

MariaDB [test]> create  procedure sp1() delete t1 from t1 join v2 on v2.c0 = t1.c1;
Query OK, 0 rows affected (0.00 sec)
MariaDB [test]> call sp1();
ERROR 1054 (42S22): Unknown column 't1.c1' in 'from clause'
MariaDB [test]> call sp1();
ERROR 1054 (42S22): Unknown column 't1.c1' in 'on clause'

We see that the first and the second call of sp1 report errors with slightly different error messages.

Comment by Igor Babaev [ 2021-04-07 ]

The following change fixes the reported problem:

diff --git a/sql/sql_derived.cc b/sql/sql_derived.cc
index be5905d..85d67a1 100644
--- a/sql/sql_derived.cc
+++ b/sql/sql_derived.cc
@@ -435,6 +435,7 @@ bool mysql_derived_merge(THD *thd, LEX *lex, TABLE_LIST *derived)
       derived->on_expr= expr;
       derived->prep_on_expr= expr->copy_andor_structure(thd);
     }
+    thd->where= "on clause";
     if (derived->on_expr &&
         ((!derived->on_expr->fixed &&
           derived->on_expr->fix_fields(thd, &derived->on_expr)) ||
diff --git a/sql/sql_update.cc b/sql/sql_update.cc
index 01743a6..fe2421a 100644
--- a/sql/sql_update.cc
+++ b/sql/sql_update.cc
@@ -1395,10 +1395,8 @@ bool Multiupdate_prelocking_strategy::handle_end(THD *thd)
         FALSE, UPDATE_ACL, SELECT_ACL, FALSE))
     DBUG_RETURN(1);
 
-  if (select_lex->handle_derived(thd->lex, DT_MERGE))
-    DBUG_RETURN(1);
-
-  if (thd->lex->save_prep_leaf_tables())
+  bool rc= select_lex->handle_derived(thd->lex, DT_MERGE);
+  if (thd->lex->save_prep_leaf_tables() || rc)
     DBUG_RETURN(1);
 
   List<Item> *fields= &lex->select_lex.item_list;

After having applied this diff we have:

MariaDB [test]> create table t1 (id int) engine=myisam;
Query OK, 0 rows affected (0.01 sec)
MariaDB [test]> insert into t1 values (1),(2),(1);
Query OK, 3 rows affected (0.00 sec)
Records: 3  Duplicates: 0  Warnings: 0
MariaDB [test]> create table t2 (pk int, c0 int) engine=myisam;
Query OK, 0 rows affected (0.00 sec)
MariaDB [test]> insert into t2 values (1,1), (2,3);
Query OK, 2 rows affected (0.00 sec)
Records: 2  Duplicates: 0  Warnings: 0
MariaDB [test]> create view v2 as select * from t2;
Query OK, 0 rows affected (0.01 sec)
MariaDB [test]> create  procedure sp1() update (t1 join v2 on v2.c0 = t1.c1) set v2.pk = 1;
Query OK, 0 rows affected (0.00 sec)
MariaDB [test]> call sp1();
ERROR 1054 (42S22): Unknown column 't1.c1' in 'on clause'
MariaDB [test]> call sp1();
ERROR 1054 (42S22): Unknown column 't1.c1' in 'on clause'

However in debugger we see that for the first call of sp1 the bug is reported after the call of fix_fields() for the ON expression in the function mysql_derived_merge() while for the second call of sp1 it happens when after fix_fields() for the ON expression is called in setup_on_expr().
If we check where this error message is reported for the procedure with SELECT instead of UPDATE

create  procedure sp0() select * from t1 join v2 on v2.c0 = t1.c1;

we see that for both calls of sp0() it happens when fix_fields() for ON expression is called in setup_on_expr().

We also see that for the following SP

create  procedure sp2() update ((t1 join v2 on v2.c0 = t1.c1) join v2 as v on v.c0 = t1.c2) set v2.pk = 1;

the first and the second calls of sp2 report different error messages

MariaDB [test]> call sp2();
ERROR 1054 (42S22): Unknown column 't1.c1' in 'on clause'
MariaDB [test]> call sp2();
ERROR 1054 (42S22): Unknown column 't1.c2' in 'on clause'

So the suggested fix cannot be considered as satisfactory.

Comment by Oleksandr Byelkin [ 2021-04-22 ]

OK to push

Generated at Thu Feb 08 09:32:56 UTC 2024 using Jira 8.20.16#820016-sha1:9d11dbea5f4be3d4cc21f03a88dd11d8c8687422.