MDEV-16385 ROW SP variable is allowed in unexpected context

The problem described in the bug report happened because the code
did not test check_cols(1) after fix_fields() in a few places.

Additionally, fix_fields() could be called multiple times for SP variables,
because they are all fixed at a early stage in append_for_log().

Solution:
1. Adding a few helper methods
   - fix_fields_if_needed()
   - fix_fields_if_needed_for_scalar()
   - fix_fields_if_needed_for_bool()
   - fix_fields_if_needed_for_order_by()
  and using it in many cases instead of fix_fields() where
  the "fixed" status is not definitely known to be "false".

2. Adding DBUG_ASSERT(!fixed) into Item_splocal*::fix_fields()
   to catch double execution.

3. Adding tests.

As a good side effect, the patch removes a lot of duplicate code (~60 lines):

   if (!item->fixed &&
       item->fix_fields(..) &&
       item->check_cols(1))
     return true;

1 files changed, 5 insertions, 7 deletions

diff --git a/sql/item_cmpfunc.cc b/sql/item_cmpfunc.cc
index 76f4788c1cf..3034636dca3 100644
--- a/sql/item_cmpfunc.cc
+++ b/sql/item_cmpfunc.cc
@@ -1242,7 +1242,7 @@ bool Item_in_optimizer::fix_left(THD *thd)
     ref0= &(((Item_in_subselect *)args[1])->left_expr);
     args[0]= ((Item_in_subselect *)args[1])->left_expr;
   }
-  if ((!(*ref0)->fixed && (*ref0)->fix_fields(thd, ref0)) ||
+  if ((*ref0)->fix_fields_if_needed(thd, ref0) ||
       (!cache && !(cache= (*ref0)->get_cache(thd))))
     DBUG_RETURN(1);
   /*
@@ -1327,7 +1327,7 @@ bool Item_in_optimizer::fix_fields(THD *thd, Item **ref)
   if (args[0]->maybe_null)
     maybe_null=1;
 
-  if (!args[1]->fixed && args[1]->fix_fields(thd, args+1))
+  if (args[1]->fix_fields_if_needed(thd, args + 1))
     return TRUE;
   if (!invisible_mode() &&
       ((sub && ((col= args[0]->cols()) != sub->engine->cols())) ||
@@ -4586,11 +4586,9 @@ Item_cond::fix_fields(THD *thd, Item **ref)
         thd->restore_active_arena(arena, &backup);
     }
 
-    // item can be substituted in fix_fields
-    if ((!item->fixed &&
-	 item->fix_fields(thd, li.ref())) ||
-	(item= *li.ref())->check_cols(1))
+    if (item->fix_fields_if_needed_for_bool(thd, li.ref()))
       return TRUE; /* purecov: inspected */
+    item= *li.ref(); // item can be substituted in fix_fields
     used_tables_cache|=     item->used_tables();
     if (item->const_item() && !item->with_param &&
         !item->is_expensive() && !cond_has_datetime_is_null(item))
@@ -5306,7 +5304,7 @@ bool Item_func_like::fix_fields(THD *thd, Item **ref)
 {
   DBUG_ASSERT(fixed == 0);
   if (Item_bool_func2::fix_fields(thd, ref) ||
-      escape_item->fix_fields(thd, &escape_item) ||
+      escape_item->fix_fields_if_needed_for_scalar(thd, &escape_item) ||
       fix_escape_item(thd, escape_item, &cmp_value1, escape_used_in_parsing,
                       cmp_collation.collation, &escape))
     return TRUE;