MDEV-39022 Add LOCAL spvar syntax for prepared statements and SYS_R…

[sanja-byelkin]

…EFCURSORs

This patch adds the following syntax:

OPEN c0 FOR LOCAL spvar_with_ps_name;
PREPARE LOCAL spvar_with_ps_name FROM 'dynamic sql'; EXECUTE LOCAL spvar_with_ps_name;
DEALLOCATE PREPARE LOCAL spvar_with_ps_name;

OPEN c0 FOR PREPARE stmt;

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[gemini-code-assist]

Code Review

This pull request implements support for opening cursors using prepared statements and local variables (e.g., 'OPEN c FOR PREPARE stmt' and 'OPEN c FOR LOCAL spvar'). The feedback identifies critical security and stability issues, including potential null pointer dereferences when executing prepared statements with local variables outside of stored procedures, and a potential out-of-bounds read due to non-null-terminated LEX_CSTRING variables. Additionally, it is recommended to introduce an 'is_ps' boolean flag to prevent incorrect fallbacks when statement names evaluate to empty, and to add clarifying comments for temporary string buffers.

[gemini-code-assist]

If PREPARE LOCAL spvar FROM ... is executed as a top-level statement outside of a stored procedure, lex->spcont will be nullptr. Accessing lex->spcont->find_variable_or_error without a null check will cause a server crash. We must add a check to ensure lex->spcont is not null before attempting to look up the local variable.

  if (name.local_var_name().length)
  {
    if (!lex->spcont)
    {
      my_error(ER_SP_UNDECLARED_VAR, MYF(0), name.local_var_name().str);
      return true;
    }
    sp_variable *spvar= lex->spcont->find_variable_or_error(
                                                name.local_var_name(), false);

[gemini-code-assist]

We should evaluate whether the cursor is a prepared-statement-based cursor (either a simple PS or a LOCAL variable) and store this in a boolean variable is_ps. This prevents us from incorrectly falling back to a regular cursor open or dynamic string open when the evaluated statement name is empty (e.g., when spvar is ''). Also, since value_buffer and converter_buffer are String objects initialized with temporary buffers whose charsets are expected to be immediately overwritten, we should add a comment indicating this.

  StringBuffer<64> value_buffer, converter_buffer; // charset will be overwritten
  Lex_ident_sys ps_name= lps.evaluate_name(thd, &value_buffer,
                                           &converter_buffer, "OPEN");
  bool is_ps= lps.var().rcontext_handler() != nullptr || lps.name().str != nullptr;
  if (ps_name.str == nullptr && lps.var().rcontext_handler()/*FOR LOCAL*/)
  {
    /*
      We're here if this is an "OPEN c0 FOR LOCAL spvar" statement
      and the SP variable evaluates to SQL NULL:
        SET spvar= NULL;
        OPEN c0 FOR LOCAL spvar;
      evaluate_name() should have raised the ER_UNKNOWN_STMT_HANDLER error.
    */
    DBUG_ASSERT(thd->is_error());
    DBUG_RETURN(-1);
  }

References

1. When a String object is initialized with a temporary buffer and its charset is expected to be immediately overwritten (e.g., by a subsequent call to field->val_str()), add a comment such as '// charset will be overwritten' to prevent confusion for future developers.

[gemini-code-assist]

Use the is_ps boolean to determine whether to open the cursor from a prepared statement, rather than checking ps_name.length. This ensures that if the evaluated statement name is empty, we still attempt to open it as a prepared statement (which will correctly fail with an unknown statement handler error) instead of falling back to opening it as a regular cursor or dynamic string.

    int rc= is_ps ? cursor->open_from_ps(thd, ps_name) :
            !lps.code() ? cursor->open(thd, m_cursor_name, return_type) :
                          cursor->open_from_dynamic_string(thd,
                                                set_placeholder_first_ip,
                                                m_set_ps_placeholder_count);

[gemini-code-assist]

Use the is_ps boolean here as well to ensure consistent behavior when reopening the cursor.

  DBUG_RETURN(is_ps ? cursor->open_from_ps(thd, ps_name) :
              !lps.code() ? cursor->open(thd, m_cursor_name, return_type,
                                       false/*don't check max_open_cursors*/) :
              cursor->open_from_dynamic_string(thd, set_placeholder_first_ip,
                                               m_set_ps_placeholder_count));

[gemini-code-assist]

For defensive programming and to prevent potential crashes in release builds (where DBUG_ASSERT(lex->spcont) is compiled out), we should also check if lex->spcont is null before calling find_variable_or_error in this overload.

  if (open_for.local_var_name().length)
  {
    if (!lex->spcont)
    {
      my_error(ER_SP_UNDECLARED_VAR, MYF(0), open_for.local_var_name().str);
      return true;
    }
    sp_variable *spvar= lex->spcont->find_variable_or_error(
                                             open_for.local_var_name(), false);

[gemini-code-assist]

LEX_CSTRING is not guaranteed to be null-terminated (it is often a slice of the query buffer). Passing name.str directly to my_error with the %s format specifier can result in printing garbage or reading out of bounds. We should ensure the name is safely null-terminated before passing it to my_error.

  sp_variable *find_variable_or_error(const LEX_CSTRING &name,
                                      bool current_scope_only) const
  {
    sp_variable *res= find_variable(&name, current_scope_only);
    if (!res)
    {
      char buf[128];
      size_t len= name.length < sizeof(buf) ? name.length : sizeof(buf) - 1;
      memcpy(buf, name.str, len);
      buf[len]= '\0';
      my_error(ER_SP_UNDECLARED_VAR, MYF(0), buf);
    }
    return res;
  }

[Copilot]

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds support for using LOCAL SP variables as prepared statement names, and for opening cursor variables from prepared statements (including via LOCAL variables), aligning behavior with ORACLE-mode SYS_REFCURSOR usage.

Changes:

• Extend the SQL grammar to accept PREPARE/EXECUTE/DEALLOCATE PREPARE LOCAL spvar and OPEN c FOR PREPARE stmt / OPEN c FOR LOCAL spvar.
• Add runtime support to resolve prepared-statement names from SP local variables (including erroring when the variable evaluates to SQL NULL).
• Add mysql-test coverage for the new syntax and cursor open behaviors (including disallowed cases for strict ref cursors and stored functions).

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
sql/sql_yacc.yy	Extends parser rules/types to accept LOCAL statement names and enhanced OPEN ... FOR ... forms.
sql/sql_prepare.cc	Implements variable-based statement-name resolution and wires it into PREPARE/EXECUTE/DEALLOCATE execution paths.
sql/sql_lex.h	Introduces Lex_sql_statement_name_st and Lex_open_for_st, and extends Lex_prepared_stmt to store SP-variable address for LOCAL.
sql/sql_lex.cc	Updates statement builders and OPEN cursor handling to support PREPARE and LOCAL sources, plus validation/error paths.
sql/sp_pcontext.h	Adds find_variable_or_error() helper to consolidate SP-variable lookup + error reporting.
sql/sp_instr.cc	Updates cursor-open instruction execution to resolve prepared statement name at runtime (including LOCAL variable evaluation).
mysql-test/main/sp-cursor-for-ps.test	New tests for OPEN ... FOR PREPARE and OPEN ... FOR LOCAL, including error cases.
mysql-test/main/sp-cursor-for-ps.result	Expected output for the new sp-cursor-for-ps test.
mysql-test/main/ps-local.test	New tests for PREPARE/EXECUTE/DEALLOCATE ... LOCAL and mixed simple/extended usages.
mysql-test/main/ps-local.result	Expected output for the new ps-local test.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[sanja-byelkin]

Generally all is good, but fix small things.

Missing coverage:

• EXECUTE LOCAL spvar USING — the USING parameter path is never exercised with the LOCAL form. The test only covers the "not all parameters bound" error case.
• PREPARE LOCAL spvar where spvar holds NULL (error path) — covered for OPEN but not for standalone PREPARE/EXECUTE/DEALLOCATE.
• OPEN c FOR LOCAL spvar inside a trigger — only stored functions are tested for the "not allowed" path;

[sanja-byelkin]

FORM -> FOR

[sanja-byelkin]

FORM -> FOR

[sanja-byelkin]

may be reset m_var here?

or reset it in set where it set conditionally.

[sanja-byelkin]

here if item is NULL so there was an error. Why it after it return return Lex_ident_sys(tmp.str, tmp.length); as if nothing happened. If it is error handling please put comment here.

[sanja-byelkin]

  union
  {
    /*
      m_name contains either PS name or an SP variable name:
        OPEN c FOR PREPARE stmt;
        OPEN c FOR LOCAL spvar;
    */
    Lex_ident_sys_st m_name;
    Item *m_dynamic_string;      // OPEN c FOR expr;
  };

so both part of union is assigned