Fixed crashing bug in recursive SQL if write to tmp table would fail

This error was discovered while working on
MDEV-30540 Wrong result with IN list length reaching
           IN_PREDICATE_CONVERSION_THRESHOLD

If there is read error from handler::ha_rnd_next() during a recursive
query, st_select_lex_unit::exec_recursive() will crash as it will try to
get the error code from a structure that was deleted by the callee.
The code was using the construct:
   sl->join->exec();
   saved_error=sl->join->error;
This does not work as sl->join was freed by the exec() and sl->join would
be set to 0.
Fixed by having JOIN::exec() return the error code.
The included test case simulates the error in ha_rnd_next(), which causes
a crash without the patch.
scovered whle working on
MDEV-30540 Wrong result with IN list length reaching
           IN_PREDICATE_CONVERSION_THRESHOLD

If there is read error from handler::ha_rnd_next() during a recursive
query, st_select_lex_unit::exec_recursive() will crash as it will try to
get the error code from a structure that was deleted by the callee.
The code was using the construct:
   sl->join->exec();
   saved_error=sl->join->error;
This does not work as sl->join was freed by the exec() and sl->join was
set to 0.
Fixed by having JOIN::exec() return the error code.
The included test case simulates the error in ha_rnd_next(), which causes
a crash without the patch.

mysql-test/suite/maria/crash-recursive.result

@@ -0,0 +1,53 @@
+set @save_big_tables=@@big_tables;
+set big_tables=1;
+Warnings:
+Warning	1287	'@@big_tables' is deprecated and will be removed in a future release
+create table  folks(id int, name char(32), dob date, father int, mother int);
+insert into folks values
+(100, 'Me', '2000-01-01', 20, 30),
+(20, 'Dad', '1970-02-02', 10, 9),
+(30, 'Mom', '1975-03-03', 8, 7),
+(10, 'Grandpa Bill', '1940-04-05', null, null),
+(9, 'Grandma Ann', '1941-10-15', null, null),
+(25, 'Uncle Jim', '1968-11-18', 8, 7),
+(98, 'Sister Amy', '2001-06-20', 20, 30),
+(7, 'Grandma Sally', '1943-08-23', null, 6),
+(8, 'Grandpa Ben', '1940-10-21', null, null),
+(6, 'Grandgrandma Martha', '1923-05-17', null, null),
+(67, 'Cousin Eddie', '1992-02-28', 25, 27),
+(27, 'Auntie Melinda', '1971-03-29', null, null);
+call mtr.add_suppression(".*marked as crashed.*");
+SET @saved_dbug= @@SESSION.debug_dbug;
+SET SESSION debug_dbug="+d,ha_rnd_next_error";
+SET @ha_rnd_next_error_counter=110;
+with recursive
+ancestor_couples(h_id, h_name, h_dob, h_father, h_mother,
+w_id, w_name, w_dob, w_father, w_mother)
+as
+(
+select h.*, w.*
+from folks h, folks w,  coupled_ancestors a
+where a.father = h.id AND a.mother = w.id
+union
+select h.*, w.*
+from folks v, folks h, folks w
+where v.name = 'Me' and
+(v.father = h.id AND v.mother= w.id)
+),
+coupled_ancestors (id, name, dob, father, mother)
+as
+(
+select h_id, h_name, h_dob, h_father, h_mother
+from ancestor_couples
+union
+select w_id, w_name, w_dob, w_father, w_mother
+from ancestor_couples
+)
+select h_name, h_dob, w_name, w_dob
+from ancestor_couples;
+ERROR HY000: Table '(temporary)' is marked as crashed and should be repaired
+drop table folks;
+set big_tables=@save_big_tables;
+Warnings:
+Warning	1287	'@@big_tables' is deprecated and will be removed in a future release
+SET @@SESSION.debug_dbug=@saved_dbug;

mysql-test/suite/maria/crash-recursive.test

@@ -0,0 +1,67 @@
+#
+# This test simulates an error in an aria file discovered during a recursive SQL call.
+# The error handling causes used join structures to be deleted, which caused crashes in
+# upper levels when trying to access structures that does not exist anymore
+#
+
+--source include/have_debug_sync.inc
+--source include/not_embedded.inc
+
+set @save_big_tables=@@big_tables;
+set big_tables=1;
+
+create table  folks(id int, name char(32), dob date, father int, mother int);
+
+insert into folks values
+(100, 'Me', '2000-01-01', 20, 30),
+(20, 'Dad', '1970-02-02', 10, 9),
+(30, 'Mom', '1975-03-03', 8, 7),
+(10, 'Grandpa Bill', '1940-04-05', null, null),
+(9, 'Grandma Ann', '1941-10-15', null, null),
+(25, 'Uncle Jim', '1968-11-18', 8, 7),
+(98, 'Sister Amy', '2001-06-20', 20, 30),
+(7, 'Grandma Sally', '1943-08-23', null, 6),
+(8, 'Grandpa Ben', '1940-10-21', null, null),
+(6, 'Grandgrandma Martha', '1923-05-17', null, null),
+(67, 'Cousin Eddie', '1992-02-28', 25, 27),
+(27, 'Auntie Melinda', '1971-03-29', null, null);
+
+
+call mtr.add_suppression(".*marked as crashed.*");
+SET @saved_dbug= @@SESSION.debug_dbug;
+SET SESSION debug_dbug="+d,ha_rnd_next_error";
+SET @ha_rnd_next_error_counter=110;
+
+let q=
+with recursive
+ancestor_couples(h_id, h_name, h_dob, h_father, h_mother,
+                 w_id, w_name, w_dob, w_father, w_mother)
+as
+(
+  select h.*, w.*
+    from folks h, folks w,  coupled_ancestors a
+      where a.father = h.id AND a.mother = w.id
+  union
+  select h.*, w.*
+    from folks v, folks h, folks w
+      where v.name = 'Me' and
+            (v.father = h.id AND v.mother= w.id)
+),
+coupled_ancestors (id, name, dob, father, mother)
+as
+(
+  select h_id, h_name, h_dob, h_father, h_mother
+    from ancestor_couples
+  union
+  select w_id, w_name, w_dob, w_father, w_mother
+    from ancestor_couples
+)
+select h_name, h_dob, w_name, w_dob
+  from ancestor_couples;
+
+--error ER_CRASHED_ON_USAGE
+eval $q;
+drop table folks;
+
+set big_tables=@save_big_tables;
+SET @@SESSION.debug_dbug=@saved_dbug;

sql/debug.cc

@@ -35,7 +35,7 @@ static const LEX_CSTRING debug_crash_counter=
 static const LEX_CSTRING debug_error_counter=
 { STRING_WITH_LEN("debug_error_counter") };
 
-static bool debug_decrement_counter(const LEX_CSTRING *name)
+bool debug_decrement_counter(const LEX_CSTRING *name)
 {
   THD *thd= current_thd;
   user_var_entry *entry= (user_var_entry*)

sql/debug.h

@@ -31,6 +31,7 @@
 #ifndef DBUG_OFF
 void debug_crash_here(const char *keyword);
 bool debug_simulate_error(const char *keyword, uint error);
+bool debug_decrement_counter(const LEX_CSTRING *name);
 #else
 #define debug_crash_here(A) do { } while(0)
 #define debug_simulate_error(A, B) 0

sql/handler.cc

@@ -42,6 +42,7 @@
 #include <pfs_transaction_provider.h>
 #include <mysql/psi/mysql_transaction.h>
 #include "debug_sync.h"         // DEBUG_SYNC
+#include "debug.h"              // debug_decrement_counter
 #include "sql_audit.h"
 #include "ha_sequence.h"
 #include "rowid_filter.h"
@@ -3573,6 +3574,15 @@ int handler::ha_rnd_next(uchar *buf)
               m_lock_type != F_UNLCK);
   DBUG_ASSERT(inited == RND);
 
+  DBUG_EXECUTE_IF("ha_rnd_next_error",
+  {
+    LEX_CSTRING user_var= { STRING_WITH_LEN("ha_rnd_next_error_counter") };
+    if (debug_decrement_counter(&user_var))
+    {
+      print_error(HA_ERR_WRONG_IN_RECORD,MYF(0));
+      DBUG_RETURN(HA_ERR_WRONG_IN_RECORD);
+    }
+  });
   do
   {
     TABLE_IO_WAIT(tracker, PSI_TABLE_FETCH_ROW, MAX_KEY, result,

sql/item_subselect.cc

@@ -4062,6 +4062,7 @@ int subselect_single_select_engine::exec()
   char const *save_where= thd->where;
   SELECT_LEX *save_select= thd->lex->current_select;
   thd->lex->current_select= select_lex;
+  bool exec_error= 0;
   DBUG_ENTER("subselect_single_select_engine::exec");
 
   if (join->optimization_state == JOIN::NOT_OPTIMIZED)
@@ -4153,7 +4154,7 @@ int subselect_single_select_engine::exec()
       }
     }
     
-    join->exec();
+    exec_error= join->exec();
 
     /* Enable the optimizations back */
     for (JOIN_TAB **ptab= changed_tabs; ptab != last_changed_tab; ptab++)
@@ -4171,7 +4172,7 @@ int subselect_single_select_engine::exec()
       item->make_const();
     thd->where= save_where;
     thd->lex->current_select= save_select;
-    DBUG_RETURN(join->error || thd->is_fatal_error || thd->is_error());
+    DBUG_RETURN(exec_error || thd->is_error());
   }
   thd->where= save_where;
   thd->lex->current_select= save_select;
@@ -5718,9 +5719,8 @@ int subselect_hash_sj_engine::exec()
   /* The subquery should be optimized, and materialized only once. */
   DBUG_ASSERT(materialize_join->optimization_state == JOIN::OPTIMIZATION_DONE &&
               !is_materialized);
-  materialize_join->exec();
+  res= materialize_join->exec();
-  if (unlikely((res= MY_TEST(materialize_join->error || thd->is_fatal_error ||
+  if (unlikely((res= (res || thd->is_error()))))
-                             thd->is_error()))))
     goto err;
 
   /*

sql/opt_subselect.cc

@@ -5869,10 +5869,10 @@ enum_nested_loop_state join_tab_execution_startup(JOIN_TAB *tab)
       ((subselect_hash_sj_engine*)in_subs->engine);
     if (!hash_sj_engine->is_materialized)
     {
-      hash_sj_engine->materialize_join->exec();
+      int error= hash_sj_engine->materialize_join->exec();
       hash_sj_engine->is_materialized= TRUE; 
 
-      if (unlikely(hash_sj_engine->materialize_join->error) ||
+      if (unlikely(error) ||
           unlikely(tab->join->thd->is_fatal_error))
         DBUG_RETURN(NESTED_LOOP_ERROR);
     }

sql/sql_select.cc

@@ -4657,8 +4657,9 @@ bool JOIN::save_explain_data(Explain_query *output, bool can_overwrite,
 }
 
 
-void JOIN::exec()
+int JOIN::exec()
 {
+  int res;
   DBUG_EXECUTE_IF("show_explain_probe_join_exec_start", 
                   if (dbug_user_var_equals_int(thd, 
                                                "show_explain_probe_select_id", 
@@ -4666,7 +4667,7 @@ void JOIN::exec()
                         dbug_serve_apcs(thd, 1);
                  );
   ANALYZE_START_TRACKING(thd, &explain->time_tracker);
-  exec_inner();
+  res= exec_inner();
   ANALYZE_STOP_TRACKING(thd, &explain->time_tracker);
 
   DBUG_EXECUTE_IF("show_explain_probe_join_exec_end", 
@@ -4675,10 +4676,11 @@ void JOIN::exec()
                                                select_lex->select_number))
                         dbug_serve_apcs(thd, 1);
                  );
+  return res;
 }
 
 
-void JOIN::exec_inner()
+int JOIN::exec_inner()
 {
   List<Item> *columns_list= &fields_list;
   DBUG_ENTER("JOIN::exec_inner");
@@ -4714,12 +4716,12 @@ void JOIN::exec_inner()
     {
       thd->set_examined_row_count(0);
       thd->limit_found_rows= 0;
-      DBUG_VOID_RETURN;
+      DBUG_RETURN(0);
     }
     columns_list= &procedure_fields_list;
   }
   if (result->prepare2(this))
-    DBUG_VOID_RETURN;
+    DBUG_RETURN(error);
 
   if (!tables_list && (table_count || !select_lex->with_sum_func) &&
       !select_lex->have_window_funcs())
@@ -4733,7 +4735,7 @@ void JOIN::exec_inner()
                                            Protocol::SEND_NUM_ROWS |
                                            Protocol::SEND_EOF))
       {
-        DBUG_VOID_RETURN;
+        DBUG_RETURN(error);
       }
 
       /*
@@ -4771,7 +4773,7 @@ void JOIN::exec_inner()
     /* Single select (without union) always returns 0 or 1 row */
     thd->limit_found_rows= send_records;
     thd->set_examined_row_count(0);
-    DBUG_VOID_RETURN;
+    DBUG_RETURN(error);
   }
 
   /*
@@ -4791,7 +4793,7 @@ void JOIN::exec_inner()
   if (unlikely(thd->is_error()))
   {
     error= thd->is_error();
-    DBUG_VOID_RETURN;
+    DBUG_RETURN(error);
   }
 
   if (zero_result_cause)
@@ -4815,7 +4817,7 @@ void JOIN::exec_inner()
 			      select_options,
 			      zero_result_cause,
 			      having ? having : tmp_having, all_fields);
-      DBUG_VOID_RETURN;
+      DBUG_RETURN(0);
     }
   }
   
@@ -4839,14 +4841,14 @@ void JOIN::exec_inner()
       if (unlikely(thd->is_error()))
       {
         error= thd->is_error();
-        DBUG_VOID_RETURN;
+        DBUG_RETURN(error);
       }
     }
   }
 
   if ((this->select_lex->options & OPTION_SCHEMA_TABLE) &&
       get_schema_tables_result(this, PROCESSED_BY_JOIN_EXEC))
-    DBUG_VOID_RETURN;
+    DBUG_RETURN(0);
 
   if (select_options & SELECT_DESCRIBE)
   {
@@ -4854,13 +4856,13 @@ void JOIN::exec_inner()
 		    order != 0 && !skip_sort_order,
 		    select_distinct,
                     !table_count ? "No tables used" : NullS);
-    DBUG_VOID_RETURN;
+    DBUG_RETURN(0);
   }
   else if (select_lex->pushdown_select)
   {
     /* Execute the query pushed into a foreign engine */
     error= select_lex->pushdown_select->execute();
-    DBUG_VOID_RETURN;
+    DBUG_RETURN(error);
   }
   else
   {
@@ -4879,7 +4881,7 @@ void JOIN::exec_inner()
   if (unlikely(thd->is_error()))
   {
     error= thd->is_error();
-    DBUG_VOID_RETURN;
+    DBUG_RETURN(error);
   }
 
   THD_STAGE_INFO(thd, stage_sending_data);
@@ -4894,7 +4896,7 @@ void JOIN::exec_inner()
   DBUG_PRINT("counts", ("thd->examined_row_count: %lu",
                         (ulong) thd->get_examined_row_count()));
 
-  DBUG_VOID_RETURN;
+  DBUG_RETURN(error);
 }
 
 
@@ -5067,7 +5069,7 @@ mysql_select(THD *thd, TABLE_LIST *tables, List<Item> &fields, COND *conds,
              SELECT_LEX_UNIT *unit, SELECT_LEX *select_lex)
 {
   int err= 0;
-  bool free_join= 1;
+  bool free_join= 1, exec_error= 0;
   DBUG_ENTER("mysql_select");
 
   if (!fields.is_empty())
@@ -5146,7 +5148,7 @@ mysql_select(THD *thd, TABLE_LIST *tables, List<Item> &fields, COND *conds,
   if (unlikely(thd->is_error()))
     goto err;
 
-  join->exec();
+  exec_error= join->exec();
 
   if (thd->lex->describe & DESCRIBE_EXTENDED)
   {
@@ -5166,9 +5168,9 @@ err:
   {
     THD_STAGE_INFO(thd, stage_end);
     err|= (int)(select_lex->cleanup());
-    DBUG_RETURN(err || thd->is_error());
+    DBUG_RETURN(exec_error || err || thd->is_error());
   }
-  DBUG_RETURN(join->error ? join->error: err);
+  DBUG_RETURN(exec_error || err);
 }
 
 

sql/sql_select.h

@@ -1717,9 +1717,8 @@ public:
   bool build_explain();
   int reinit();
   int init_execution();
-  void exec();
+  int exec() __attribute__((warn_unused_result));
-
+  int exec_inner();
-  void exec_inner();
   bool prepare_result(List<Item> **columns_list);
   int destroy();
   void restore_tmp();

sql/sql_union.cc

@@ -2155,8 +2155,8 @@ bool st_select_lex_unit::exec()
   ulonglong add_rows=0;
   ha_rows examined_rows= 0;
   bool first_execution= !executed;
-  DBUG_ENTER("st_select_lex_unit::exec");
   bool was_executed= executed;
+  DBUG_ENTER("st_select_lex_unit::exec");
 
   if (executed && !uncacheable && !describe)
     DBUG_RETURN(FALSE);
@@ -2243,7 +2243,7 @@ bool st_select_lex_unit::exec()
 	if (sl->tvc)
 	  sl->tvc->exec(sl);
 	else
-	  sl->join->exec();
+	  saved_error= sl->join->exec();
         if (sl == union_distinct && !have_except_all_or_intersect_all &&
             !(with_element && with_element->is_recursive))
 	{
@@ -2253,8 +2253,6 @@ bool st_select_lex_unit::exec()
 	    DBUG_RETURN(TRUE);
 	  table->no_keyread=1;
 	}
-	if (!sl->tvc)
-	  saved_error= sl->join->error;
 	if (likely(!saved_error))
 	{
 	  examined_rows+= thd->get_examined_row_count();
@@ -2401,7 +2399,8 @@ bool st_select_lex_unit::exec()
         {
           join->join_examined_rows= 0;
           saved_error= join->reinit();
-          join->exec();
+          if (join->exec())
+            saved_error= 1;
         }
       }
 
@@ -2507,8 +2506,7 @@ bool st_select_lex_unit::exec_recursive()
       sl->tvc->exec(sl);
     else
     {
-      sl->join->exec();
+      saved_error= sl->join->exec();
-      saved_error= sl->join->error;
     }
     if (likely(!saved_error))
     {
@@ -2520,11 +2518,10 @@ bool st_select_lex_unit::exec_recursive()
 	 DBUG_RETURN(1);
        }
     }
-    if (unlikely(saved_error))
+    else
     {
       thd->lex->current_select= lex_select_save;
       goto err;
-      
     }
   }