MDEV-5816: Stored programs: validation of stored program statementsbb-10.9-MDEV-5816-1

This is the prerequisite patch introducing the class sp_lex_instr
that encapsulates access to an instance of sp_lex_keeper. Every SP instruction
that does need access to a LEX object on its processing should inherit this class
and implement two abstract methods:
  is_invalid(),
  invalidate().

These method will be used in subsequent patches to implement recomplilation of
SP instructions on failure.

Currently, the following instructions are derived from the class sp_lex_instr:
  sp_instr_stmt,
  sp_instr_set,
  sp_instr_set_trigger_field,
  sp_instr_jump_if_not,
  sp_instr_freturn,
  sp_instr_cpush,
  sp_instr_cursor_copy_struct,
  sp_instr_set_case_expr

Additionally, this patch converts the class sp_instr_opt_meta
to the base abstract class (that is, not inhereted from
the class sp_instr).

Every jump SP instruction now must be inhereted directly from
the class sp_instr_opt_meta and additionally from either the class
sp_lex_instr or sp_instr depending on whether this SP instruction
does need access to LEX object or not.

Moreover, the class sp_cursor is no more owner of sp_lex_keeper.
Instead, the virtual method get_lex_keeper() has been added to the
class sp_cursor() that returns nullptr and this method is overriden
in the derived class sp_instr_cpush to provide a pointer to a real
instance of the class sp_lex_keeper.

3 files changed, 208 insertions, 79 deletions

diff --git a/sql/sp_instr.cc b/sql/sp_instr.cc
index 836d9af8927..ff4362db94e 100644
--- a/sql/sp_instr.cc
+++ b/sql/sp_instr.cc
@@ -755,15 +755,6 @@ sp_instr_set_trigger_field::print(String *str)
                                     QT_ITEM_ORIGINAL_FUNC_NULLIF));
 }
 
-/*
-  sp_instr_opt_meta
-*/
-
-uint sp_instr_opt_meta::get_cont_dest() const
-{
-  return m_cont_dest;
-}
-
 
 /*
  sp_instr_jump class functions
@@ -926,8 +917,15 @@ sp_instr_jump_if_not::opt_move(uint dst, List<sp_instr_opt_meta> *bp)
   }
   else if (m_cont_optdest)
     m_cont_dest= m_cont_optdest->m_ip; // Backward
-  /* This will take care of m_dest and m_ip */
-  sp_instr_jump::opt_move(dst, bp);
+
+  /*
+    Take care about m_dest and m_ip
+  */
+  if (m_dest > m_ip)
+    bp->push_back(this);      // Forward
+  else if (m_optdest)
+    m_dest= m_optdest->m_ip;  // Backward
+  m_ip= dst;
 }
 
 
@@ -1203,7 +1201,7 @@ sp_instr_cpush::execute(THD *thd, uint *nextp)
 {
   DBUG_ENTER("sp_instr_cpush::execute");
 
-  sp_cursor::reset(thd, &m_lex_keeper);
+  sp_cursor::reset(thd);
   m_lex_keeper.disable_query_cache();
   thd->spcont->push_cursor(this);
 
@@ -1500,14 +1498,14 @@ sp_instr_cursor_copy_struct::exec_core(THD *thd, uint *nextp)
 
   /*
     Copy structure only once. If the cursor%ROWTYPE variable is declared
-    inside a LOOP block, it gets its structure on the first loop interation
+    inside a LOOP block, it gets its structure on the first loop iteration
     and remembers the structure for all consequent loop iterations.
     It we recreated the structure on every iteration, we would get
     potential memory leaks, and it would be less efficient.
   */
   if (!row->arguments())
   {
-    sp_cursor tmp(thd, &m_lex_keeper, true);
+    sp_cursor tmp(thd, true);
     // Open the cursor without copying data
     if (!(ret= tmp.open(thd)))
     {
diff --git a/sql/sp_instr.h b/sql/sp_instr.h
index e53f521b785..086f52ae5dd 100644
--- a/sql/sp_instr.h
+++ b/sql/sp_instr.h
@@ -254,19 +254,47 @@ private:
 
 
 /**
+  The base class for any stored program instruction that does need to
+  get access to the LEX object on execution.
+*/
+
+class sp_lex_instr : public sp_instr
+{
+public:
+  sp_lex_instr(uint ip, sp_pcontext *ctx, LEX *lex, bool is_lex_owner)
+  : sp_instr(ip, ctx), m_lex_keeper(lex, is_lex_owner)
+  {}
+
+  virtual bool is_invalid() const = 0;
+
+  virtual void invalidate() = 0;
+
+protected:
+  sp_lex_keeper m_lex_keeper;
+};
+
+
+/**
   Call out to some prepared SQL statement.
 */
-class sp_instr_stmt : public sp_instr
+class sp_instr_stmt : public sp_lex_instr
 {
   sp_instr_stmt(const sp_instr_stmt &); /**< Prevent use of these */
   void operator=(sp_instr_stmt &);
 
+  /**
+    Flag to tell whether a metadata this instruction depends on
+    has been changed and a LEX object should be reinitialized.
+  */
+  bool m_valid;
+
 public:
 
   LEX_STRING m_query;           ///< For thd->query
 
   sp_instr_stmt(uint ip, sp_pcontext *ctx, LEX *lex)
-    : sp_instr(ip, ctx), m_lex_keeper(lex, true)
+    : sp_lex_instr(ip, ctx, lex, true),
+      m_valid(true)
   {
     m_query.str= 0;
     m_query.length= 0;
@@ -278,18 +306,24 @@ public:
 
   void print(String *str) override;
 
-private:
+  bool is_invalid() const override
+  {
+    return !m_valid;
+  }
 
-  sp_lex_keeper m_lex_keeper;
+  void invalidate() override
+  {
+    m_valid= false;
+  }
 
 public:
   PSI_statement_info* get_psi_info() override { return & psi_info; }
   static PSI_statement_info psi_info;
 
-}; // class sp_instr_stmt : public sp_instr
+}; // class sp_instr_stmt : public sp_lex_instr
 
 
-class sp_instr_set : public sp_instr
+class sp_instr_set : public sp_lex_instr
 {
   sp_instr_set(const sp_instr_set &);   /**< Prevent use of these */
   void operator=(sp_instr_set &);
@@ -300,9 +334,8 @@ public:
                const Sp_rcontext_handler *rh,
                uint offset, Item *val,
                LEX *lex, bool lex_resp)
-    : sp_instr(ip, ctx),
-      m_rcontext_handler(rh), m_offset(offset), m_value(val),
-      m_lex_keeper(lex, lex_resp)
+    : sp_lex_instr(ip, ctx, lex, lex_resp),
+      m_rcontext_handler(rh), m_offset(offset), m_value(val)
   {}
 
   int execute(THD *thd, uint *nextp) override;
@@ -311,17 +344,26 @@ public:
 
   void print(String *str) override;
 
+  bool is_invalid() const override
+  {
+    return m_value == nullptr;
+  }
+
+  void invalidate() override
+  {
+    m_value= nullptr;
+  }
+
 protected:
   sp_rcontext *get_rcontext(THD *thd) const;
   const Sp_rcontext_handler *m_rcontext_handler;
   uint m_offset;                ///< Frame offset
   Item *m_value;
-  sp_lex_keeper m_lex_keeper;
 
 public:
   PSI_statement_info* get_psi_info() override { return & psi_info; }
   static PSI_statement_info psi_info;
-}; // class sp_instr_set : public sp_instr
+}; // class sp_instr_set : public sp_lex_instr
 
 
 /*
@@ -396,7 +438,7 @@ public:
 /**
   Set NEW/OLD row field value instruction. Used in triggers.
 */
-class sp_instr_set_trigger_field : public sp_instr
+class sp_instr_set_trigger_field : public sp_lex_instr
 {
   sp_instr_set_trigger_field(const sp_instr_set_trigger_field &);
   void operator=(sp_instr_set_trigger_field &);
@@ -406,9 +448,9 @@ public:
   sp_instr_set_trigger_field(uint ip, sp_pcontext *ctx,
                              Item_trigger_field *trg_fld,
                              Item *val, LEX *lex)
-    : sp_instr(ip, ctx),
+    : sp_lex_instr(ip, ctx, lex, true),
       trigger_field(trg_fld),
-      value(val), m_lex_keeper(lex, true)
+      value(val)
   {}
 
   int execute(THD *thd, uint *nextp) override;
@@ -417,15 +459,24 @@ public:
 
   void print(String *str) override;
 
+  bool is_invalid() const override
+  {
+    return value == nullptr;
+  }
+
+  void invalidate() override
+  {
+    value= nullptr;
+  }
+
 private:
   Item_trigger_field *trigger_field;
   Item *value;
-  sp_lex_keeper m_lex_keeper;
 
 public:
   PSI_statement_info* get_psi_info() override { return & psi_info; }
   static PSI_statement_info psi_info;
-}; // class sp_instr_trigger_field : public sp_instr
+}; // class sp_instr_trigger_field : public sp_lex_instr
 
 
 /**
@@ -435,36 +486,32 @@ public:
   Even if not all subclasses will use both the normal destination and
   the continuation destination, we put them both here for simplicity.
 */
-class sp_instr_opt_meta : public sp_instr
+class sp_instr_opt_meta
 {
 public:
 
   uint m_dest;                  ///< Where we will go
   uint m_cont_dest;             ///< Where continue handlers will go
 
-  sp_instr_opt_meta(uint ip, sp_pcontext *ctx)
-    : sp_instr(ip, ctx),
-      m_dest(0), m_cont_dest(0), m_optdest(0), m_cont_optdest(0)
+  explicit sp_instr_opt_meta(uint dest)
+    : m_dest(dest), m_cont_dest(0), m_optdest(0), m_cont_optdest(0)
   {}
 
-  sp_instr_opt_meta(uint ip, sp_pcontext *ctx, uint dest)
-    : sp_instr(ip, ctx),
-      m_dest(dest), m_cont_dest(0), m_optdest(0), m_cont_optdest(0)
+  virtual ~sp_instr_opt_meta()
   {}
 
   virtual void set_destination(uint old_dest, uint new_dest)
     = 0;
 
-  uint get_cont_dest() const override;
-
 protected:
 
   sp_instr *m_optdest;          ///< Used during optimization
   sp_instr *m_cont_optdest;     ///< Used during optimization
 
-}; // class sp_instr_opt_meta : public sp_instr
+}; // class sp_instr_opt_meta
 
-class sp_instr_jump : public sp_instr_opt_meta
+
+class sp_instr_jump : public sp_instr, public sp_instr_opt_meta
 {
   sp_instr_jump(const sp_instr_jump &); /**< Prevent use of these */
   void operator=(sp_instr_jump &);
@@ -472,11 +519,11 @@ class sp_instr_jump : public sp_instr_opt_meta
 public:
 
   sp_instr_jump(uint ip, sp_pcontext *ctx)
-    : sp_instr_opt_meta(ip, ctx)
+    : sp_instr(ip, ctx), sp_instr_opt_meta(0)
   {}
 
   sp_instr_jump(uint ip, sp_pcontext *ctx, uint dest)
-    : sp_instr_opt_meta(ip, ctx, dest)
+    : sp_instr(ip, ctx), sp_instr_opt_meta(dest)
   {}
 
   int execute(THD *thd, uint *nextp) override;
@@ -496,6 +543,11 @@ public:
     m_dest= dest;
   }
 
+  uint get_cont_dest() const override
+  {
+    return m_cont_dest;
+  }
+
   /**
     Update the destination; used by the optimizer.
   */
@@ -508,10 +560,10 @@ public:
 public:
   PSI_statement_info* get_psi_info() override { return & psi_info; }
   static PSI_statement_info psi_info;
-}; // class sp_instr_jump : public sp_instr_opt_meta
+}; // class sp_instr_jump : public sp_instr, public sp_instr_opt_meta
 
 
-class sp_instr_jump_if_not : public sp_instr_jump
+class sp_instr_jump_if_not : public sp_lex_instr, public sp_instr_opt_meta
 {
   sp_instr_jump_if_not(const sp_instr_jump_if_not &); /**< Prevent use of these */
   void operator=(sp_instr_jump_if_not &);
@@ -519,13 +571,15 @@ class sp_instr_jump_if_not : public sp_instr_jump
 public:
 
   sp_instr_jump_if_not(uint ip, sp_pcontext *ctx, Item *i, LEX *lex)
-    : sp_instr_jump(ip, ctx), m_expr(i),
-      m_lex_keeper(lex, true)
+    : sp_lex_instr(ip, ctx, lex, true),
+      sp_instr_opt_meta(0),
+      m_expr(i)
   {}
 
   sp_instr_jump_if_not(uint ip, sp_pcontext *ctx, Item *i, uint dest, LEX *lex)
-    : sp_instr_jump(ip, ctx, dest), m_expr(i),
-      m_lex_keeper(lex, true)
+    : sp_lex_instr(ip, ctx, lex, true),
+      sp_instr_opt_meta(dest),
+      m_expr(i)
   {}
 
   int execute(THD *thd, uint *nextp) override;
@@ -544,22 +598,44 @@ public:
 
   void opt_move(uint dst, List<sp_instr_opt_meta> *ibp) override;
 
+  uint get_cont_dest() const override
+  {
+    return m_cont_dest;
+  }
+
   void set_destination(uint old_dest, uint new_dest) override
   {
-    sp_instr_jump::set_destination(old_dest, new_dest);
+    if (m_dest == old_dest)
+      m_dest= new_dest;
     if (m_cont_dest == old_dest)
       m_cont_dest= new_dest;
   }
 
+  void backpatch(uint dest, sp_pcontext *dst_ctx) override
+  {
+    /* Calling backpatch twice is a logic flaw in jump resolution. */
+    DBUG_ASSERT(m_dest == 0);
+    m_dest= dest;
+  }
+
+  bool is_invalid() const override
+  {
+    return m_expr == nullptr;
+  }
+
+  void invalidate() override
+  {
+    m_expr= nullptr;
+  }
+
 private:
 
   Item *m_expr;                 ///< The condition
-  sp_lex_keeper m_lex_keeper;
 
 public:
   PSI_statement_info* get_psi_info() override { return & psi_info; }
   static PSI_statement_info psi_info;
-}; // class sp_instr_jump_if_not : public sp_instr_jump
+}; // class sp_instr_jump_if_not : public sp_lex_instr, public sp_instr_opt_meta
 
 
 class sp_instr_preturn : public sp_instr
@@ -589,7 +665,7 @@ public:
 }; // class sp_instr_preturn : public sp_instr
 
 
-class sp_instr_freturn : public sp_instr
+class sp_instr_freturn : public sp_lex_instr
 {
   sp_instr_freturn(const sp_instr_freturn &);   /**< Prevent use of these */
   void operator=(sp_instr_freturn &);
@@ -598,8 +674,8 @@ public:
 
   sp_instr_freturn(uint ip, sp_pcontext *ctx,
                    Item *val, const Type_handler *handler, LEX *lex)
-    : sp_instr(ip, ctx), m_value(val), m_type_handler(handler),
-      m_lex_keeper(lex, true)
+    : sp_lex_instr(ip, ctx, lex, true),
+      m_value(val), m_type_handler(handler)
   {}
 
   int execute(THD *thd, uint *nextp) override;
@@ -618,12 +694,22 @@ protected:
 
   Item *m_value;
   const Type_handler *m_type_handler;
-  sp_lex_keeper m_lex_keeper;
 
+  bool is_invalid() const override
+  {
+    return m_value == nullptr;
+  }
+
+  void invalidate() override
+  {
+    /* TODO: be careful and check that the object referenced by m_value
+       is not leaked */
+    m_value= nullptr;
+  }
 public:
   PSI_statement_info* get_psi_info() override { return & psi_info; }
   static PSI_statement_info psi_info;
-}; // class sp_instr_freturn : public sp_instr
+}; // class sp_instr_freturn : public sp_lex_instr
 
 
 class sp_instr_hpush_jump : public sp_instr_jump
@@ -759,7 +845,7 @@ public:
 
 
 /** This is DECLARE CURSOR */
-class sp_instr_cpush : public sp_instr, public sp_cursor
+class sp_instr_cpush : public sp_lex_instr, public sp_cursor
 {
   sp_instr_cpush(const sp_instr_cpush &); /**< Prevent use of these */
   void operator=(sp_instr_cpush &);
@@ -767,7 +853,8 @@ class sp_instr_cpush : public sp_instr, public sp_cursor
 public:
 
   sp_instr_cpush(uint ip, sp_pcontext *ctx, LEX *lex, uint offset)
-    : sp_instr(ip, ctx), m_lex_keeper(lex, true), m_cursor(offset)
+    : sp_lex_instr(ip, ctx, lex, true), m_cursor(offset),
+      m_metadata_changed(false)
   {}
 
   int execute(THD *thd, uint *nextp) override;
@@ -781,15 +868,35 @@ public:
   */
   bool cleanup_stmt(bool /*restore_set_statement_vars*/) override
   { return false; }
+
+  bool is_invalid() const override
+  {
+    return m_metadata_changed;
+  }
+
+  void invalidate() override
+  {
+    m_metadata_changed= true;
+  }
+
+  sp_lex_keeper *get_lex_keeper() override
+  {
+    return &m_lex_keeper;
+  }
 private:
 
-  sp_lex_keeper m_lex_keeper;
   uint m_cursor;                /**< Frame offset (for debugging) */
 
+  /**
+    Flag if a statement's metadata has been changed in result of running DDL
+    on depending database objects used in the statement.
+  */
+  bool m_metadata_changed;
+
 public:
   PSI_statement_info* get_psi_info() override { return & psi_info; }
   static PSI_statement_info psi_info;
-}; // class sp_instr_cpush : public sp_instr
+}; // class sp_instr_cpush : public sp_lex_instr, public sp_cursor
 
 
 class sp_instr_cpop : public sp_instr
@@ -846,32 +953,47 @@ private:
 public:
   PSI_statement_info* get_psi_info() override { return & psi_info; }
   static PSI_statement_info psi_info;
-}; // class sp_instr_copen : public sp_instr_stmt
+}; // class sp_instr_copen : public sp_instr
 
 
 /**
   Initialize the structure of a cursor%ROWTYPE variable
   from the LEX containing the cursor SELECT statement.
 */
-class sp_instr_cursor_copy_struct: public sp_instr
+class sp_instr_cursor_copy_struct: public sp_lex_instr
 {
   /**< Prevent use of these */
   sp_instr_cursor_copy_struct(const sp_instr_cursor_copy_struct &);
   void operator=(sp_instr_cursor_copy_struct &);
-  sp_lex_keeper m_lex_keeper;
   uint m_cursor;
   uint m_var;
+  /**
+    Flag to tell whether metadata has been changed and the LEX object should
+    be reinitialized.
+  */
+  bool m_valid;
 public:
   sp_instr_cursor_copy_struct(uint ip, sp_pcontext *ctx, uint coffs,
                               sp_lex_cursor *lex, uint voffs)
-    : sp_instr(ip, ctx), m_lex_keeper(lex, false),
+    : sp_lex_instr(ip, ctx, lex, false),
       m_cursor(coffs),
-      m_var(voffs)
+      m_var(voffs),
+      m_valid(true)
   {}
   int execute(THD *thd, uint *nextp) override;
   int exec_core(THD *thd, uint *nextp) override;
   void print(String *str) override;
 
+  bool is_invalid() const override
+  {
+    return !m_valid;
+  }
+
+  void invalidate() override
+  {
+    m_valid= true;
+  }
+
 public:
   PSI_statement_info* get_psi_info() override { return & psi_info; }
   static PSI_statement_info psi_info;
@@ -993,15 +1115,14 @@ public:
 }; // class sp_instr_error : public sp_instr
 
 
-class sp_instr_set_case_expr : public sp_instr_opt_meta
+class sp_instr_set_case_expr : public sp_lex_instr, public sp_instr_opt_meta
 {
 public:
 
   sp_instr_set_case_expr(uint ip, sp_pcontext *ctx, uint case_expr_id,
                          Item *case_expr, LEX *lex)
-    : sp_instr_opt_meta(ip, ctx),
-      m_case_expr_id(case_expr_id), m_case_expr(case_expr),
-      m_lex_keeper(lex, true)
+    : sp_lex_instr(ip, ctx, lex, true), sp_instr_opt_meta(0),
+      m_case_expr_id(case_expr_id), m_case_expr(case_expr)
   {}
 
   int execute(THD *thd, uint *nextp) override;
@@ -1014,21 +1135,35 @@ public:
 
   void opt_move(uint dst, List<sp_instr_opt_meta> *ibp) override;
 
+  uint get_cont_dest() const override
+  {
+    return m_cont_dest;
+  }
+
   void set_destination(uint old_dest, uint new_dest) override
   {
     if (m_cont_dest == old_dest)
       m_cont_dest= new_dest;
   }
 
+  bool is_invalid() const override
+  {
+    return m_case_expr == nullptr;
+  }
+
+  void invalidate() override
+  {
+    m_case_expr= nullptr;
+  }
+
 private:
 
   uint m_case_expr_id;
   Item *m_case_expr;
-  sp_lex_keeper m_lex_keeper;
 
 public:
   PSI_statement_info* get_psi_info() override { return & psi_info; }
   static PSI_statement_info psi_info;
-}; // class sp_instr_set_case_expr : public sp_instr_opt_meta
+}; // public sp_lex_instr, public sp_instr_opt_meta
 
 #endif
diff --git a/sql/sql_class.h b/sql/sql_class.h
index 806f77c1ba2..53e59549912 100644
--- a/sql/sql_class.h
+++ b/sql/sql_class.h
@@ -5952,19 +5952,17 @@ private:
 public:
   sp_cursor()
    :result(NULL, false),
-    m_lex_keeper(NULL),
     server_side_cursor(NULL)
   { }
-  sp_cursor(THD *thd_arg, sp_lex_keeper *lex_keeper, bool view_structure_only)
+  sp_cursor(THD *thd_arg, bool view_structure_only)
    :result(thd_arg, view_structure_only),
-    m_lex_keeper(lex_keeper),
     server_side_cursor(NULL)
   {}
 
   virtual ~sp_cursor()
   { destroy(); }
 
-  sp_lex_keeper *get_lex_keeper() { return m_lex_keeper; }
+  virtual sp_lex_keeper *get_lex_keeper() { return nullptr; }
 
   int open(THD *thd);
 
@@ -5977,17 +5975,15 @@ public:
 
   bool export_structure(THD *thd, Row_definition_list *list);
 
-  void reset(THD *thd_arg, sp_lex_keeper *lex_keeper)
+  void reset(THD *thd_arg)
   {
     sp_cursor_statistics::reset();
     result.reset(thd_arg);
-    m_lex_keeper= lex_keeper;
     server_side_cursor= NULL;
   }
 
 private:
   Select_fetch_into_spvars result;
-  sp_lex_keeper *m_lex_keeper;
   Server_side_cursor *server_side_cursor;
   void destroy();
 };