maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

Hi, Oleksandr,

On Apr 10, Oleksandr Byelkin wrote:
> revision-id: e3e59c9e38d (mariadb-10.6.6-83-ge3e59c9e38d)
> parent(s): 4e1ca388381
> author: Oleksandr Byelkin
> committer: Oleksandr Byelkin
> timestamp: 2022-04-08 10:37:17 +0200
> message:
> 
> MDEV-28266 Crash in Field_string::type_handler when calling procedures
> 
> In case of only metadata request the cursor lack the call to redirect
> Field::orig_table to the temporary table.
> 
> diff --git a/sql/sql_cursor.cc b/sql/sql_cursor.cc
> index 0add5845558..155346341f1 100644
> --- a/sql/sql_cursor.cc
> +++ b/sql/sql_cursor.cc
> @@ -183,6 +183,16 @@ int mysql_open_cursor(THD *thd, select_result *result,
>    {
>      Materialized_cursor *materialized_cursor=
>        result_materialize->materialized_cursor;
> +    if (result_materialize->view_structure_only())
> +    {
> +      /*
> +        If it was view_structure_only then only send metadata call was made
> +        and there was no other call. So we have to "close" the cursor
> +        explicitly (which redirect orig_table in the fields, and untie
> +        it with closed tables)
> +      */
> +      materialized_cursor->on_table_fill_finished();
> +    }
>
>     if ((rc= materialized_cursor->open(0)))
>     {
>       delete materialized_cursor;
>       goto end;
>     }

normally on_table_fill_finished() is called from
Select_materialize::send_eof() and
Select_materialize::abort_result_set().

Note that directly after your added code there's
materialized_cursor->open(), and there we have

  int Materialized_cursor::open(JOIN *join __attribute__((unused)))
  {
    ...
    if (!rc)
    {
      thd->server_status|= SERVER_STATUS_CURSOR_EXISTS;
      result->send_eof();
    }
    else
    {
      result->abort_result_set();
    }

That is, one could expect that even without your added code,
on_table_fill_finished() would be called.

The problem is that result here is sp_cursor::Select_fetch_into_spvars,
and sp_cursor::Select_fetch_into_spvars::send_eof() is simply
{ return FALSE; }

So, as a fix I'd suggest to remove on_table_fill_finished() from
send_eof/abort_result_set, and put it unconditionally after the if(),
to make it work no matter what select_result was chosen.

Like this:
======================================
--- a/sql/sql_cursor.cc
+++ b/sql/sql_cursor.cc
@@ -80,19 +80,7 @@ class Select_materialize: public select_unit
   Select_materialize(THD *thd_arg, select_result *result_arg):
     select_unit(thd_arg), result(result_arg), materialized_cursor(0) {}
   virtual bool send_result_set_metadata(List<Item> &list, uint flags);
-  bool send_eof()
-  {
-    if (materialized_cursor)
-      materialized_cursor->on_table_fill_finished();
-    return false;
-  }
-
-  void abort_result_set()
-  {
-    if (materialized_cursor)
-      materialized_cursor->on_table_fill_finished();
-  }
-
+  bool send_eof() { return false; }
   bool view_structure_only() const
   {
     return result->view_structure_only();
@@ -333,6 +321,8 @@ int Materialized_cursor::open(JOIN *join
     result->abort_result_set();
   }

+  on_table_fill_finished();
+
   return rc;
 }
======================================

with the commit comment saying that "on_table_fill_finished() should
always be done at the end of open(), even if result is not
Select_materialize, but, for example, Select_fetch_into_spvars"

Regards,
Sergei
VP of MariaDB Server Engineering
and security@xxxxxxxxxxx