From 1ee1979ed687ea0343358da9b420810ebe3a9ce0 Mon Sep 17 00:00:00 2001 From: Dmitry Shulga Date: Wed, 19 Jul 2023 18:17:19 +0700 Subject: [PATCH] MDEV-5816: Stored programs: validation of stored program statements This patch is the second part of implementation for cursor's statement re-parsing. The patch does the following changes: - on re-parsing a failed SP instruction that does need to get access to LEX a new lex is instantiated for every SP instruction except cursor relating SP instructions. - items created on re-parsing a cursor relating statement are moved to the free_list of sp_lex_cursor. --- sql/sp_instr.cc | 126 +++++++++++++++++++++++++++++++++++------------- sql/sp_instr.h | 13 +++-- sql/sql_lex.h | 7 +++ 3 files changed, 108 insertions(+), 38 deletions(-) diff --git a/sql/sp_instr.cc b/sql/sp_instr.cc index c7fe786988353..a40e7d0866ade 100644 --- a/sql/sp_instr.cc +++ b/sql/sp_instr.cc @@ -363,34 +363,55 @@ sp_lex_keeper::reset_lex_and_exec_core(THD *thd, uint *nextp, void sp_lex_keeper::free_lex(THD *thd) { + /* + Currently, m_lex_resp == false for sp_instr_cursor_copy_struct instructions + and in some cases for sp_instr_set instructions. For these classes + free_lex() returns control flow immediately and doesn't change m_lex. + */ if (!m_lex_resp || !m_lex) return; /* Prevent endless recursion. */ m_lex->sphead= nullptr; lex_end(m_lex); - delete (st_lex_local *)m_lex; + sp_lex_cursor* cursor_lex= m_lex->get_lex_for_cursor(); + if (cursor_lex == nullptr) + { + delete (st_lex_local *)m_lex; + /* + In case it is not sp_lex_cursor set thd->lex to the null value + if it points to a LEX object we just deleted in order to avoid + dangling pointers problem. + */ + if (thd->lex == m_lex) + thd->lex= nullptr; - /* - Set thd->lex to the null value in case it points to a LEX object - we just deleted in order to avoid dangling pointer problem - */ - if (thd->lex == m_lex) - thd->lex= nullptr; + m_lex= nullptr; + m_lex_resp= false; + } + else + { + /* + sp_lex_cursor has references to items allocated on parsing a cursor + declaration statement. These items are deleted on re-parsing a failing + cursor declaration statement at the method + sp_lex_instr::cleanup_before_parsing. + Remove the reference to items that will be deleted from sp_lex_cursor + in order to avoid dangling pointers problem. + */ + cleanup_items(cursor_lex->free_list); + cursor_lex->free_list= nullptr; + } - m_lex= nullptr; - m_lex_resp= false; lex_query_tables_own_last= nullptr; } -void sp_lex_keeper::set_lex(LEX *lex, bool is_lex_owner) +void sp_lex_keeper::set_lex(LEX *lex) { m_lex= lex; - m_lex_resp= is_lex_owner; - - if (m_lex) - m_lex->sp_lex_in_use= true; + m_lex_resp= true; + m_lex->sp_lex_in_use= true; } @@ -406,11 +427,15 @@ int sp_lex_keeper::validate_lex_and_exec_core(THD *thd, uint *nextp, { thd->clear_error(); free_lex(thd); - LEX *lex= instr->parse_expr(thd, thd->spcont->m_sp); + LEX *lex= instr->parse_expr(thd, thd->spcont->m_sp, m_lex); if (!lex) return true; - set_lex(lex, true); + /* + m_lex != nullptr in case it points to sp_lex_cursor. + */ + if (m_lex == nullptr) + set_lex(lex); m_first_execution= true; } @@ -608,7 +633,7 @@ bool sp_lex_instr::setup_table_fields_for_trigger( return result; } -LEX* sp_lex_instr::parse_expr(THD *thd, sp_head *sp) +LEX* sp_lex_instr::parse_expr(THD *thd, sp_head *sp, LEX *sp_instr_lex) { String sql_query; @@ -637,17 +662,11 @@ LEX* sp_lex_instr::parse_expr(THD *thd, sp_head *sp) saved_ptr_to_next_trg_items_list= m_cur_trigger_stmt_items.first->next_trig_field_list; + /* + Clean up items owned by this SP instruction. + */ cleanup_before_parsing(sp->m_handler->type()); - Parser_state parser_state; - - if (parser_state.init(thd, sql_query.c_ptr(), sql_query.length())) - return nullptr; - - // Create a new LEX and initialize it. - - LEX *lex_saved= thd->lex; - DBUG_ASSERT(mem_root != thd->mem_root); /* Back up the current free_list pointer and reset it to nullptr. @@ -668,7 +687,40 @@ LEX* sp_lex_instr::parse_expr(THD *thd, sp_head *sp) thd->set_n_backup_active_arena(this, &backup); thd->free_list= nullptr; - thd->lex= new (thd->mem_root) st_lex_local; + Parser_state parser_state; + + if (parser_state.init(thd, sql_query.c_ptr(), sql_query.length())) + return nullptr; + + // Create a new LEX and initialize it. + + LEX *lex_saved= thd->lex; + Item **cursor_free_list= nullptr; + + /* + sp_instr_lex != nullptr for cursor relating SP instructions (sp_instr_cpush, + sp_instr_cursor_copy_struct) and in some cases for sp_instr_set. + */ + if (sp_instr_lex == nullptr) + thd->lex= new (thd->mem_root) st_lex_local; + else + { + sp_lex_cursor* cursor_lex= sp_instr_lex->get_lex_for_cursor(); + /* + In case sp_instr_cursor_copy_struct instruction being re-parsed + the items stored in free_list of sp_lex_cursor are not cleaned up + since the class sp_instr_cursor_copy_struct don't pass ownership of + lex object to sp_lex_keeper. So, clean up items stored in free_list of + sp_lex_cursor explicitly. For sp_instr_cpush instruction items stored + in free_list of sp_lex_cursor are cleaned up in the method free_lex() + since sp_instr_cpush owns a lex object stored in its sp_lex_keeper + data member. So, for the sp_instr_cpush instruction by the time we reach + this block cursor_lex->free_list is already empty. + */ + cleanup_items(cursor_lex->free_list); + cursor_free_list= &cursor_lex->free_list; + DBUG_ASSERT(thd->lex == sp_instr_lex); + } lex_start(thd); @@ -709,13 +761,21 @@ LEX* sp_lex_instr::parse_expr(THD *thd, sp_head *sp) setup_table_fields_for_trigger(thd, sp, saved_ptr_to_next_trg_items_list); - /* - Assign the list of items created on parsing to the current - stored routine instruction. - */ - free_list= thd->free_list; + if (cursor_free_list) + /* + Update sp_lex_cursor::free_list to point to a list of items + just created on re-parsing the cursor's statement. + */ + *cursor_free_list= thd->free_list; + else + /* + Assign the list of items created on re-parsing the statement to + the current stored routine's instruction. + */ + free_list= thd->free_list; + thd->free_list= nullptr; - } + } Query_arena old; thd->restore_active_arena(&old, &backup); diff --git a/sql/sp_instr.h b/sql/sp_instr.h index 27d1f917ceed0..404afcf9b1b29 100644 --- a/sql/sp_instr.h +++ b/sql/sp_instr.h @@ -76,6 +76,11 @@ class sp_lex_cursor: public sp_lex_local, public Query_arena return m_expr_str; } + sp_lex_cursor* get_lex_for_cursor() override + { + return this; + } + private: LEX_CSTRING m_expr_str; }; @@ -316,11 +321,8 @@ class sp_lex_keeper final Set LEX object. @param lex LEX-object - @param is_lex_owner this flag specifies whether this LEX object is owned - by the sp_lex_keeper and so should deleted when - needed. */ - void set_lex(LEX *lex, bool is_lex_owner); + void set_lex(LEX *lex); private: LEX *m_lex; @@ -386,10 +388,11 @@ class sp_lex_instr : public sp_instr @param thd Thread context. @param sp The stored program. + @param lex SP instruction's lex @return new LEX-object or NULL in case of failure. */ - LEX *parse_expr(THD *thd, sp_head *sp); + LEX *parse_expr(THD *thd, sp_head *sp, LEX *lex); SQL_I_List* get_instr_trig_field_list() override { diff --git a/sql/sql_lex.h b/sql/sql_lex.h index 6330bc779560e..43364ef214d97 100644 --- a/sql/sql_lex.h +++ b/sql/sql_lex.h @@ -3213,6 +3213,8 @@ class Lex_grant_privilege: public Grant_privilege, public Sql_alloc }; +class sp_lex_cursor; + struct LEX: public Query_tables_list { SELECT_LEX_UNIT unit; /* most upper unit */ @@ -4923,6 +4925,11 @@ struct LEX: public Query_tables_list { return query_tables != nullptr || sroutines.records > 0; } + + virtual sp_lex_cursor* get_lex_for_cursor() + { + return nullptr; + } };