MDEV-33218: Assertion `active_arena->is_stmt_prepare_or_first_stmt_execute()...

MDEV-33218: Assertion `active_arena->is_stmt_prepare_or_first_stmt_execute() || active_arena->state == Query_arena::STMT_SP_QUERY_ARGUMENTS' failed in st_select_lex::fix_prepare_information

In case there is a view that queried from a stored routine or
a prepared statement and this temporary table is dropped between
executions of SP/PS, then it leads to hitting an assertion
at the SELECT_LEX::fix_prepare_information. The fired assertion
 was added by the commit 85f2e4f8
(MDEV-32466: Potential memory leak on executing of create view statement).
Firing of this assertion means memory leaking on execution of SP/PS.
Moreover, if the added assert be commented out, different result sets
can be produced by the statement SELECT * FROM the hidden table.

Both hitting the assertion and different result sets have the same root
cause. This cause is usage of temporary table's metadata after the table
itself has been dropped. To fix the issue, reload the cache of stored
routines. To do it  cache of stored routines is reset at the end of
execution of the function dispatch_command(). Next time any stored routine
be called it will be loaded from the table mysql.proc. This happens inside
the method Sp_handler::sp_cache_routine where loading of a stored routine
is performed in case it missed in cache. Loading is performed unconditionally
while previously it was controlled by the parameter lookup_only. By that
reason the signature of the method Sroutine_hash_entry::sp_cache_routine
was changed by removing unused parameter lookup_only.

Clearing of sp caches affects the test main.lock_sync since it forces
opening and locking the table mysql.proc but the test assumes that each
statement locks its tables once during its execution. To keep this invariant
the debug sync points with names "before_lock_tables_takes_lock" and
"after_lock_tables_takes_lock" are not activated on handling the table
mysql.proc

mysql-test/main/ps.result

@@ -5982,6 +5982,19 @@ EXECUTE stmt USING DEFAULT;
 # Clean up
 DEALLOCATE PREPARE stmt;
 DROP TABLE t1, t2;
+# MDEV-33218: Assertion `active_arena->is_stmt_prepare_or_first_stmt_execute() || active_arena->state == Query_arena::STMT_SP_QUERY_ARGUMENTS' failed. in st_select_lex::fix_prepare_information
+CREATE TABLE t1 AS SELECT 1 f;
+PREPARE stmt FROM 'SHOW CREATE TABLE t1';
+DROP TABLE t1;
+EXECUTE stmt;
+ERROR 42S02: Table 'test.t1' doesn't exist
+CREATE VIEW t1 AS SELECT 1;
+EXECUTE stmt;
+View	Create View	character_set_client	collation_connection
+t1	CREATE ALGORITHM=UNDEFINED DEFINER=`root`@`localhost` SQL SECURITY DEFINER VIEW `t1` AS select 1 AS `1`	latin1	latin1_swedish_ci
+# Clean up
+DEALLOCATE PREPARE stmt;
+DROP VIEW t1;
 #
 # End of 10.4 tests
 #

mysql-test/main/ps.test

@@ -5416,6 +5416,18 @@ EXECUTE stmt USING DEFAULT;
 DEALLOCATE PREPARE stmt;
 DROP TABLE t1, t2;
 
+--echo # MDEV-33218: Assertion `active_arena->is_stmt_prepare_or_first_stmt_execute() || active_arena->state == Query_arena::STMT_SP_QUERY_ARGUMENTS' failed. in st_select_lex::fix_prepare_information
+CREATE TABLE t1 AS SELECT 1 f;
+PREPARE stmt FROM 'SHOW CREATE TABLE t1';
+DROP TABLE t1;
+--error ER_NO_SUCH_TABLE
+EXECUTE stmt;
+CREATE VIEW t1 AS SELECT 1;
+EXECUTE stmt;
+--echo # Clean up
+DEALLOCATE PREPARE stmt;
+DROP VIEW t1;
+
 --echo #
 --echo # End of 10.4 tests
 --echo #

mysql-test/main/sp.result

@@ -7184,15 +7184,14 @@ CREATE VIEW t1 AS SELECT 10 AS f1;
 CALL p1(1);
 ERROR HY000: The target table t1 of the INSERT is not insertable-into
 CREATE TEMPORARY TABLE t1 (f1 INT);
-# t1 still refers to the view since it was inlined
 CALL p1(2);
-ERROR HY000: The target table t1 of the INSERT is not insertable-into
 DROP VIEW t1;
 # t1 now refers to the temporary table
 CALL p1(3);
 # Check which values were inserted into the temp table.
 SELECT * FROM t1;
 f1
+2
 3
 DROP TEMPORARY TABLE t1;
 DROP PROCEDURE p1;

mysql-test/main/sp.test

@@ -8633,8 +8633,6 @@ CALL p1(1);
 
 CREATE TEMPORARY TABLE t1 (f1 INT);
 
---echo # t1 still refers to the view since it was inlined
---error ER_NON_INSERTABLE_TABLE
 CALL p1(2);
 
 DROP VIEW t1;

mysql-test/main/temp_table.result

@@ -616,5 +616,54 @@ Tables_in_test
 # in 11.2 and above here should be listed above used temporary tables
 DROP TEMPORARY TABLE t1, t2;
 #
+# MDEV-33218: Assertion `active_arena->is_stmt_prepare_or_first_stmt_execute() || active_arena->state == Query_arena::STMT_SP_QUERY_ARGUMENTS' failed. in st_select_lex::fix_prepare_information
+#
+CREATE VIEW v1 AS SELECT 5;
+CREATE PROCEDURE sp() SELECT * FROM v1;
+CREATE TEMPORARY TABLE v1 as SELECT 7;
+# sp() accesses the temporary table v1 that hides the view with the same name
+# Therefore expected output is the row (7)
+CALL sp();
+7
+7
+DROP TEMPORARY TABLE v1;
+# After the temporary table v1 has been dropped the next invocation of sp()
+# accesses the view v1. So, expected output is the row (5)
+CALL sp();
+5
+5
+# Clean up
+DROP VIEW v1;
+DROP PROCEDURE sp;
+# Another use case is when a temporary table hides a view is dropped
+# inside a stored routine being called.
+CREATE VIEW t1 AS SELECT 1;
+CREATE PROCEDURE p1()
+BEGIN
+DROP TEMPORARY TABLE t1;
+END
+|
+CREATE FUNCTION f1() RETURNS INT
+BEGIN
+CALL p1();
+RETURN 1;
+END
+|
+CREATE TEMPORARY TABLE t1 AS SELECT 1 AS a;
+PREPARE stmt FROM 'SELECT f1()';
+EXECUTE stmt;
+f1()
+1
+# The temporary table t1 has been dropped on first
+# execution of the prepared statement 'stmt',
+# next time this statement is run it results in issuing
+# the error ER_BAD_TABLE_ERROR
+EXECUTE stmt;
+ERROR 42S02: Unknown table 'test.t1'
+# Clean up
+DROP VIEW t1;
+DROP FUNCTION f1;
+DROP PROCEDURE p1;
+#
 # End of 10.4 tests
 #

mysql-test/main/temp_table.test

@@ -674,6 +674,60 @@ SHOW TABLES;
 
 DROP TEMPORARY TABLE t1, t2;
 
+--echo #
+--echo # MDEV-33218: Assertion `active_arena->is_stmt_prepare_or_first_stmt_execute() || active_arena->state == Query_arena::STMT_SP_QUERY_ARGUMENTS' failed. in st_select_lex::fix_prepare_information
+--echo #
+CREATE VIEW v1 AS SELECT 5;
+CREATE PROCEDURE sp() SELECT * FROM v1;
+CREATE TEMPORARY TABLE v1 as SELECT 7;
+--echo # sp() accesses the temporary table v1 that hides the view with the same name
+--echo # Therefore expected output is the row (7)
+CALL sp();
+DROP TEMPORARY TABLE v1;
+--echo # After the temporary table v1 has been dropped the next invocation of sp()
+--echo # accesses the view v1. So, expected output is the row (5)
+CALL sp();
+
+--echo # Clean up
+DROP VIEW v1;
+DROP PROCEDURE sp;
+
+--echo # Another use case is when a temporary table hides a view is dropped
+--echo # inside a stored routine being called.
+
+CREATE VIEW t1 AS SELECT 1;
+
+--delimiter |
+CREATE PROCEDURE p1()
+BEGIN
+  DROP TEMPORARY TABLE t1;
+END
+|
+
+CREATE FUNCTION f1() RETURNS INT
+BEGIN
+ CALL p1();
+ RETURN 1;
+END
+|
+
+--delimiter ;
+
+CREATE TEMPORARY TABLE t1 AS SELECT 1 AS a;
+PREPARE stmt FROM 'SELECT f1()';
+EXECUTE stmt;
+--echo # The temporary table t1 has been dropped on first
+--echo # execution of the prepared statement 'stmt',
+--echo # next time this statement is run it results in issuing
+--echo # the error ER_BAD_TABLE_ERROR
+--error ER_BAD_TABLE_ERROR
+EXECUTE stmt;
+
+--echo # Clean up
+DROP VIEW t1;
+DROP FUNCTION f1;
+DROP PROCEDURE p1;
+
 --echo #
 --echo # End of 10.4 tests
 --echo #

sql/sp.cc

@@ -1907,7 +1907,7 @@ Sp_handler::sp_show_create_routine(THD *thd,
 
   DBUG_EXECUTE_IF("cache_sp_in_show_create",
     /* Some tests need just need a way to cache SP without other side-effects.*/
-    sp_cache_routine(thd, name, false, &sp);
+    sp_cache_routine(thd, name, &sp);
     sp->show_create_routine(thd, this);
     DBUG_RETURN(false);
   );
@@ -2331,7 +2331,7 @@ Sp_handler::sp_cache_routine_reentrant(THD *thd,
   int ret;
   Parser_state *oldps= thd->m_parser_state;
   thd->m_parser_state= NULL;
-  ret= sp_cache_routine(thd, name, false, sp);
+  ret= sp_cache_routine(thd, name, sp);
   thd->m_parser_state= oldps;
   return ret;
 }
@@ -2738,7 +2738,6 @@ void sp_update_stmt_used_routines(THD *thd, Query_tables_list *prelocking_ctx,
 */
 
 int Sroutine_hash_entry::sp_cache_routine(THD *thd,
-                                          bool lookup_only,
                                           sp_head **sp) const
 {
   char qname_buff[NAME_LEN*2+1+1];
@@ -2751,7 +2750,7 @@ int Sroutine_hash_entry::sp_cache_routine(THD *thd,
   */
   DBUG_ASSERT(mdl_request.ticket || this == thd->lex->sroutines_list.first);
 
-  return m_handler->sp_cache_routine(thd, &name, lookup_only, sp);
+  return m_handler->sp_cache_routine(thd, &name, sp);
 }
 
 
@@ -2763,9 +2762,6 @@ int Sroutine_hash_entry::sp_cache_routine(THD *thd,
 
   @param[in]  thd   Thread context.
   @param[in]  name  Name of routine.
-  @param[in]  lookup_only Only check that the routine is in the cache.
-                    If it's not, don't try to load. If it is present,
-                    but old, don't try to reload.
   @param[out] sp    Pointer to sp_head object for routine, NULL if routine was
                     not found.
 
@@ -2776,7 +2772,6 @@ int Sroutine_hash_entry::sp_cache_routine(THD *thd,
 
 int Sp_handler::sp_cache_routine(THD *thd,
                                  const Database_qualified_name *name,
-                                 bool lookup_only,
                                  sp_head **sp) const
 {
   int ret= 0;
@@ -2788,9 +2783,6 @@ int Sp_handler::sp_cache_routine(THD *thd,
 
   *sp= sp_cache_lookup(spc, name);
 
-  if (lookup_only)
-    DBUG_RETURN(SP_OK);
-
   if (*sp)
   {
     sp_cache_flush_obsolete(spc, sp);
@@ -2842,7 +2834,6 @@ int Sp_handler::sp_cache_routine(THD *thd,
                        * name->m_db is a database name, e.g. "dbname"
                        * name->m_name is a package-qualified name,
                          e.g. "pkgname.spname"
-  @param lookup_only - don't load mysql.proc if not cached
   @param [OUT] sp    - the result is returned here.
   @retval false      - loaded or does not exists
   @retval true       - error while loading mysql.proc
@@ -2852,14 +2843,13 @@ int
 Sp_handler::sp_cache_package_routine(THD *thd,
                                      const LEX_CSTRING &pkgname_cstr,
                                      const Database_qualified_name *name,
-                                     bool lookup_only, sp_head **sp) const
+                                     sp_head **sp) const
 {
   DBUG_ENTER("sp_cache_package_routine");
   DBUG_ASSERT(type() == TYPE_ENUM_FUNCTION || type() == TYPE_ENUM_PROCEDURE);
   sp_name pkgname(&name->m_db, &pkgname_cstr, false);
   sp_head *ph= NULL;
   int ret= sp_handler_package_body.sp_cache_routine(thd, &pkgname,
-                                                    lookup_only,
                                                     &ph);
   if (!ret)
   {
@@ -2894,12 +2884,12 @@ Sp_handler::sp_cache_package_routine(THD *thd,
 
 int Sp_handler::sp_cache_package_routine(THD *thd,
                                          const Database_qualified_name *name,
-                                         bool lookup_only, sp_head **sp) const
+                                         sp_head **sp) const
 {
   DBUG_ENTER("Sp_handler::sp_cache_package_routine");
   Prefix_name_buf pkgname(thd, name->m_name);
   DBUG_ASSERT(pkgname.length);
-  DBUG_RETURN(sp_cache_package_routine(thd, pkgname, name, lookup_only, sp));
+  DBUG_RETURN(sp_cache_package_routine(thd, pkgname, name, sp));
 }
 
 

sql/sp.h

@@ -101,10 +101,10 @@ class Sp_handler
   int sp_cache_package_routine(THD *thd,
                                const LEX_CSTRING &pkgname_cstr,
                                const Database_qualified_name *name,
-                               bool lookup_only, sp_head **sp) const;
+                               sp_head **sp) const;
   int sp_cache_package_routine(THD *thd,
                                const Database_qualified_name *name,
-                               bool lookup_only, sp_head **sp) const;
+                               sp_head **sp) const;
   sp_head *sp_find_package_routine(THD *thd,
                                    const LEX_CSTRING pkgname_str,
                                    const Database_qualified_name *name,
@@ -202,7 +202,7 @@ class Sp_handler
                                    const Database_qualified_name *name,
                                    bool cache_only) const;
   virtual int sp_cache_routine(THD *thd, const Database_qualified_name *name,
-                               bool lookup_only, sp_head **sp) const;
+                               sp_head **sp) const;
 
   int sp_cache_routine_reentrant(THD *thd,
                                  const Database_qualified_name *nm,
@@ -283,9 +283,9 @@ class Sp_handler_package_procedure: public Sp_handler_procedure
 {
 public:
   int sp_cache_routine(THD *thd, const Database_qualified_name *name,
-                       bool lookup_only, sp_head **sp) const
+                       sp_head **sp) const
   {
-    return sp_cache_package_routine(thd, name, lookup_only, sp);
+    return sp_cache_package_routine(thd, name, sp);
   }
   sp_head *sp_find_routine(THD *thd,
                            const Database_qualified_name *name,
@@ -332,9 +332,9 @@ class Sp_handler_package_function: public Sp_handler_function
 {
 public:
   int sp_cache_routine(THD *thd, const Database_qualified_name *name,
-                       bool lookup_only, sp_head **sp) const
+                       sp_head **sp) const
   {
-    return sp_cache_package_routine(thd, name, lookup_only, sp);
+    return sp_cache_package_routine(thd, name, sp);
   }
   sp_head *sp_find_routine(THD *thd,
                            const Database_qualified_name *name,
@@ -632,7 +632,7 @@ class Sroutine_hash_entry
 
   const Sp_handler *m_handler;
 
-  int sp_cache_routine(THD *thd, bool lookup_only, sp_head **sp) const;
+  int sp_cache_routine(THD *thd, sp_head **sp) const;
 };
 
 

sql/sp_cache.cc

@@ -78,6 +78,8 @@ class sp_cache
 
   /* All routines in this cache */
   HASH m_hashtable;
+public:
+  void clear();
 }; // class sp_cache
 
 #ifdef HAVE_PSI_INTERFACE
@@ -313,6 +315,10 @@ sp_cache::cleanup()
   my_hash_free(&m_hashtable);
 }
 
+void sp_cache::clear()
+{
+  my_hash_reset(&m_hashtable);
+}
 
 void Sp_caches::sp_caches_clear()
 {
@@ -321,3 +327,15 @@ void Sp_caches::sp_caches_clear()
   sp_cache_clear(&sp_package_spec_cache);
   sp_cache_clear(&sp_package_body_cache);
 }
+
+void Sp_caches::sp_caches_empty()
+{
+  if (sp_proc_cache)
+    sp_proc_cache->clear();
+  if (sp_func_cache)
+    sp_func_cache->clear();
+  if (sp_package_spec_cache)
+    sp_package_spec_cache->clear();
+  if (sp_package_body_cache)
+    sp_package_body_cache->clear();
+}

sql/sql_base.cc

@@ -3563,7 +3563,7 @@ open_and_process_routine(THD *thd, Query_tables_list *prelocking_ctx,
           DBUG_RETURN(TRUE);
 
         /* Ensures the routine is up-to-date and cached, if exists. */
-        if (rt->sp_cache_routine(thd, has_prelocking_list, &sp))
+        if (rt->sp_cache_routine(thd, &sp))
           DBUG_RETURN(TRUE);
 
         /* Remember the version of the routine in the parse tree. */
@@ -3604,7 +3604,7 @@ open_and_process_routine(THD *thd, Query_tables_list *prelocking_ctx,
           Validating routine version is unnecessary, since CALL
           does not affect the prepared statement prelocked list.
         */
-        if (rt->sp_cache_routine(thd, false, &sp))
+        if (rt->sp_cache_routine(thd, &sp))
           DBUG_RETURN(TRUE);
       }
     }
@@ -5566,13 +5566,23 @@ bool lock_tables(THD *thd, TABLE_LIST *tables, uint count, uint flags)
 	*(ptr++)= table->table;
     }
 
-    DEBUG_SYNC(thd, "before_lock_tables_takes_lock");
+#ifdef ENABLED_DEBUG_SYNC
+    if (!tables ||
+        !(strcmp(tables->db.str, "mysql") == 0 &&
+          strcmp(tables->table_name.str, "proc") == 0))
+      DEBUG_SYNC(thd, "before_lock_tables_takes_lock");
+#endif
 
     if (! (thd->lock= mysql_lock_tables(thd, start, (uint) (ptr - start),
                                         flags)))
       DBUG_RETURN(TRUE);
 
-    DEBUG_SYNC(thd, "after_lock_tables_takes_lock");
+#ifdef ENABLED_DEBUG_SYNC
+    if (!tables ||
+        !(strcmp(tables->db.str, "mysql") == 0 &&
+          strcmp(tables->table_name.str, "proc") == 0))
+      DEBUG_SYNC(thd, "after_lock_tables_takes_lock");
+#endif
 
     if (thd->lex->requires_prelocking() &&
         thd->lex->sql_command != SQLCOM_LOCK_TABLES)

sql/sql_class.cc

@@ -848,6 +848,7 @@ THD::THD(my_thread_id id, bool is_wsrep_applier)
   prepare_derived_at_open= FALSE;
   create_tmp_table_for_derived= FALSE;
   save_prep_leaf_list= FALSE;
+  reset_sp_cache= false;
   org_charset= 0;
   /* Restore THR_THD */
   set_current_thd(old_THR_THD);

sql/sql_class.h

 /*
-   Copyright (c) 2000, 2016, Oracle and/or its affiliates.
    Copyright (c) 2009, 2022, MariaDB Corporation.
 
    This program is free software; you can redistribute it and/or modify
@@ -2268,6 +2267,11 @@ class Sp_caches
     swap_variables(sp_cache*, sp_package_body_cache, rhs.sp_package_body_cache);
   }
   void sp_caches_clear();
+  /**
+    Clear content of sp related caches.
+    Don't delete cache objects itself.
+  */
+  void sp_caches_empty();
 };
 
 
@@ -2569,6 +2573,12 @@ class THD: public THD_count, /* this must be first */
 
   bool save_prep_leaf_list;
 
+  /**
+    The data member reset_sp_cache is to signal that content of sp_cache
+    must be reset (all items be removed from it).
+  */
+  bool reset_sp_cache;
+
   /* container for handler's private per-connection data */
   Ha_data ha_data[MAX_HA];
 

sql/sql_parse.cc

@@ -2402,6 +2402,7 @@ bool dispatch_command(enum enum_server_command command, THD *thd,
   {
     WSREP_DEBUG("THD is killed at dispatch_end");
   }
+
   wsrep_after_command_before_result(thd);
   if (wsrep_current_error(thd) && !wsrep_command_no_result(command))
   {
@@ -2429,6 +2430,12 @@ bool dispatch_command(enum enum_server_command command, THD *thd,
 
 #endif /* WITH_WSREP */
 
+  if (thd->reset_sp_cache)
+  {
+    thd->sp_caches_empty();
+    thd->reset_sp_cache= false;
+  }
+
   if (do_end_of_statement)
   {
     DBUG_ASSERT(thd->derived_tables == NULL &&
@@ -6071,7 +6078,7 @@ mysql_execute_command(THD *thd)
       if (sph->sp_resolve_package_routine(thd, thd->lex->sphead,
                                           lex->spname, &sph, &pkgname))
         return true;
-      if (sph->sp_cache_routine(thd, lex->spname, false, &sp))
+      if (sph->sp_cache_routine(thd, lex->spname, &sp))
         goto error;
       if (!sp || sp->show_routine_code(thd))
       {

sql/sql_table.cc

@@ -2393,6 +2393,7 @@ int mysql_rm_table_no_locks(THD *thd, TABLE_LIST *tables, bool if_exists,
         goto err;
       }
       table->table= 0;
+      thd->reset_sp_cache= true;
     }
 
     if ((drop_temporary && if_exists) || !real_table)
@@ -2629,8 +2630,11 @@ int mysql_rm_table_no_locks(THD *thd, TABLE_LIST *tables, bool if_exists,
     }
     DBUG_PRINT("table", ("table: %p  s: %p", table->table,
                          table->table ?  table->table->s :  NULL));
+    if (is_temporary_table(table))
+      thd->reset_sp_cache= true;
   }
   DEBUG_SYNC(thd, "rm_table_no_locks_before_binlog");
+
   thd->thread_specific_used= TRUE;
   error= 0;
 err:
@@ -5236,6 +5240,7 @@ int create_table_impl(THD *thd, const LEX_CSTRING &orig_db,
     if (is_trans != NULL)
       *is_trans= table->file->has_transactions();
 
+    thd->reset_sp_cache= true;
     thd->thread_specific_used= TRUE;
     create_info->table= table;                  // Store pointer to table
   }