MDEV-36322 Comparison ROW(stored_func(),1)=ROW(1,1) calls the function twice per row

Item_func_sp::execute() was called two times per row in this scenario:

SELECT ROW(f1(),1) = ROW(1,1), @counter FROM seq_1_to_5;

- the first time from Item_func_sp::bring_value()
- the second time from Item_func_sp::val_int()

Fix:

Changing Item_func_sp::bring_value() to call execute() only
when the result type is ROW_RESULT.

mysql-test/main/sp-row.result

@@ -2383,3 +2383,130 @@ DROP PROCEDURE p1;
 #
 # End of 11.7 tests
 #
+#
+# MDEV-36322 Comparison ROW(stored_func(),1)=ROW(1,1) calls the function twice per row
+#
+CREATE FUNCTION f1() RETURNS INT
+BEGIN
+SET @counter= COALESCE(@counter, 0) + 1;
+RETURN @counter;
+END;
+/
+#
+# Queries without ROW comparison
+#
+SET @counter=0;
+SELECT f1() FROM seq_1_to_5;
+f1()
+1
+2
+3
+4
+5
+SET @counter=0;
+SELECT f1() AS f FROM seq_1_to_5 ORDER BY f;
+f
+1
+2
+3
+4
+5
+SET @counter=0;
+SELECT f1() AS f FROM seq_1_to_5 ORDER BY f DESC;
+f
+5
+4
+3
+2
+1
+SET @counter=0;
+SELECT f1()=1 AS eq, @counter AS counter FROM seq_1_to_5;
+eq	counter
+1	1
+0	2
+0	3
+0	4
+0	5
+#
+# Queries without ROW comparison + HAVING
+# The counter is incremented by 2 per row.
+#
+SET @counter=0;
+SELECT f1() AS f FROM seq_1_to_5 HAVING f1()<>0;
+f
+2
+4
+6
+8
+10
+SET @counter=0;
+SELECT f1() AS f FROM seq_1_to_5 HAVING f<>0;
+f
+2
+4
+6
+8
+10
+#
+# Queries with ROW comparison.
+# Item_row::bring_value() is called on the left side, which calls
+# Item_func_sp::bring_value() for f1(),
+# which does *not* call Item_func_sp::execute()
+# because the return type of f1() is scalar.
+# Item_func_sp::execute() will be called from Item_func_sp::val_int()
+# from Arg_comparator::compare_int_signed().
+#
+SET @counter=0;
+SELECT ROW(f1(),1) = ROW(1,1) AS eq, @counter AS counter FROM seq_1_to_5;
+eq	counter
+1	1
+0	2
+0	3
+0	4
+0	5
+SET @counter=0;
+SELECT ROW(COALESCE(f1()),1) = ROW(1,1) AS eq, @counter AS counter FROM seq_1_to_5;
+eq	counter
+1	1
+0	2
+0	3
+0	4
+0	5
+SET @counter=0;
+SELECT ROW(@f1:=f1(),1) = ROW(1,1) AS eq, @counter AS counter FROM seq_1_to_5;
+eq	counter
+1	1
+0	2
+0	3
+0	4
+0	5
+SET @counter=0;
+SELECT ROW(f1(),1) IN ((1,1),(1,2)) AS c0, @counter AS counter FROM seq_1_to_5;
+c0	counter
+1	1
+0	2
+0	3
+0	4
+0	5
+DROP FUNCTION f1;
+#
+# Queries with comparison of an SP returning ROW vs a ROW constant.
+# Item_func_sp::bring_value() is called on the left side,
+# which calls execute().
+#
+CREATE FUNCTION f1() RETURNS ROW (a INT,b VARCHAR(10))
+BEGIN
+SET @counter= COALESCE(@counter, 0) + 1;
+RETURN ROW(1,'b1');
+END;
+/
+SET @counter=0;
+SELECT f1() = ROW(1,'b1') AS eq, @counter AS counter FROM seq_1_to_5;
+eq	counter
+1	1
+1	2
+1	3
+1	4
+1	5
+DROP FUNCTION f1;
+# End of 11.8 tests

mysql-test/main/sp-row.test

@@ -1,3 +1,5 @@
+--source include/have_sequence.inc
+
 --echo #
 --echo # MDEV-10914 ROW data type for stored routine variables
 --echo #
@@ -1594,3 +1596,102 @@ DROP PROCEDURE p1;
 --echo #
 --echo # End of 11.7 tests
 --echo #
+
+
+--echo #
+--echo # MDEV-36322 Comparison ROW(stored_func(),1)=ROW(1,1) calls the function twice per row
+--echo #
+
+--disable_ps2_protocol
+
+DELIMITER /;
+CREATE FUNCTION f1() RETURNS INT
+BEGIN
+  SET @counter= COALESCE(@counter, 0) + 1;
+  RETURN @counter;
+END;
+/
+DELIMITER ;/
+
+
+--echo #
+--echo # Queries without ROW comparison
+--echo #
+
+
+SET @counter=0;
+SELECT f1() FROM seq_1_to_5;
+
+--disable_view_protocol
+SET @counter=0;
+SELECT f1() AS f FROM seq_1_to_5 ORDER BY f;
+
+SET @counter=0;
+SELECT f1() AS f FROM seq_1_to_5 ORDER BY f DESC;
+--enable_view_protocol
+
+SET @counter=0;
+SELECT f1()=1 AS eq, @counter AS counter FROM seq_1_to_5;
+
+--echo #
+--echo # Queries without ROW comparison + HAVING
+--echo # The counter is incremented by 2 per row.
+--echo #
+
+SET @counter=0;
+SELECT f1() AS f FROM seq_1_to_5 HAVING f1()<>0;
+
+SET @counter=0;
+SELECT f1() AS f FROM seq_1_to_5 HAVING f<>0;
+
+
+--echo #
+--echo # Queries with ROW comparison.
+--echo # Item_row::bring_value() is called on the left side, which calls
+--echo # Item_func_sp::bring_value() for f1(),
+--echo # which does *not* call Item_func_sp::execute()
+--echo # because the return type of f1() is scalar.
+--echo # Item_func_sp::execute() will be called from Item_func_sp::val_int()
+--echo # from Arg_comparator::compare_int_signed().
+--echo #
+
+SET @counter=0;
+SELECT ROW(f1(),1) = ROW(1,1) AS eq, @counter AS counter FROM seq_1_to_5;
+
+SET @counter=0;
+SELECT ROW(COALESCE(f1()),1) = ROW(1,1) AS eq, @counter AS counter FROM seq_1_to_5;
+
+SET @counter=0;
+SELECT ROW(@f1:=f1(),1) = ROW(1,1) AS eq, @counter AS counter FROM seq_1_to_5;
+
+SET @counter=0;
+SELECT ROW(f1(),1) IN ((1,1),(1,2)) AS c0, @counter AS counter FROM seq_1_to_5;
+
+DROP FUNCTION f1;
+
+
+--echo #
+--echo # Queries with comparison of an SP returning ROW vs a ROW constant.
+--echo # Item_func_sp::bring_value() is called on the left side,
+--echo # which calls execute().
+--echo #
+
+DELIMITER /;
+CREATE FUNCTION f1() RETURNS ROW (a INT,b VARCHAR(10))
+BEGIN
+  SET @counter= COALESCE(@counter, 0) + 1;
+  RETURN ROW(1,'b1');
+END;
+/
+DELIMITER ;/
+
+--disable_view_protocol
+SET @counter=0;
+SELECT f1() = ROW(1,'b1') AS eq, @counter AS counter FROM seq_1_to_5;
+--enable_view_protocol
+
+DROP FUNCTION f1;
+
+--enable_ps2_protocol
+
+--echo # End of 11.8 tests

sql/item.h

@@ -2530,7 +2530,17 @@ public:
   bool check_type_can_return_time(const LEX_CSTRING &opname) const;
   // It is not row => null inside is impossible
   virtual bool null_inside() { return 0; }
-  // used in row subselects to get value of elements
+  /*
+    bring_value()
+    - For scalar Item types this method does not do anything.
+    - For Items which can be of the ROW data type,
+      this method brings the row, so its component values become available
+      for calling their value methods (such as val_int(), get_date() etc).
+      * Item_singlerow_subselect stores component values in
+        the array of Item_cache in Item_singlerow_subselect::row.
+      * Item_func_sp stores component values in Field_row::m_table
+        of the Field_row instance pointed by Item_func_sp::sp_result_field.
+  */
   virtual void bring_value() {}
 
   const Type_handler *type_handler_long_or_longlong() const

sql/item_func.h

@@ -4023,7 +4023,79 @@ public:
 
   void bring_value() override
   {
-    execute();
+    DBUG_ASSERT(fixed());
+    /*
+      This comment describes the difference between a single row
+      subselect and a stored function returning ROW.
+
+      In case of a single column subselect:
+        SELECT 1=(SELECT a FROM t1) FROM seq_1_to_5;
+      Item_singlerow_subselect pretends to be a scalar,
+      so its type_handler() returns the type handler of the column "a".
+      (*) This is according to the SQL scandard, which says:
+          The declared type of a <scalar subquery> is the declared
+          type of the column of QE (i.e. its query expression).
+      In the above SELECT statement Arg_comparator calls a scalar comparison
+      function e.g. compare_int_signed(), which does not call bring_value().
+      Item_singlerow_subselect::exec() is called when
+      Arg_comparator::compare_int_signed(), or another scalar comparison
+      function, calls a value method like Item_singlerow_subselect::val_int().
+
+      In case of a multiple-column subselect:
+        SELECT (1,1)=(SELECT a,a FROM t1) FROM seq_1_to_5;
+      Item_singlerow_subselect::type_handler() returns &type_handler_row.
+      Arg_comparator uses compare_row() to compare its arguments.
+      compare_row() calls bring_value(), which calls
+      Item_singlerow_subselect::exec().
+
+      Unlike a single row subselect, a stored function returning a ROW does
+      not pretend to be a scalar when there is only one column in the ROW:
+        SELECT sp_row_func_with_one_col()=sp_row_var_with_one_col FROM ...;
+      Item_function_sp::type_handler() still returns &type_handler_row when
+      the return type is a ROW with one column.
+      Arg_comparator choses compare_row() as the comparison function.
+      So the execution comes to here.
+
+      This chart summarizes how a comparison of ROW values works.
+      In particular, how Item_singlerow_subselect::exec() vs
+      Item_func_sp::execute() are called.
+
+                         Single row subselect    ROW value stored function
+                         --------------------    -------------------------
+      1. bring_value()     Yes                     Yes
+         is called when
+         cols>1
+      2. exec()/execute()  Yes                     Yes
+         is called from
+         bring_value()
+         when cols>1
+      3. Pretends          Yes                     No
+         to be a scalar
+         when cols==1
+      4. bring_value()     No                      Yes
+         is called
+         when cols==1
+      5. exec()/execute()  N/A                     No
+         is called from
+         bring_value()
+         when cols==1
+      6. exec()/execute()  Yes                     Yes
+         is called from
+         a value method,
+         like val_int()
+         when cols==1
+    */
+    if (result_type() == ROW_RESULT)
+    {
+      /*
+        The condition in the "if" above catches the *intentional* difference
+        in the chart lines 3,4,5 (between a single row subselect and a stored
+        function returning ROW). Thus the condition makes #6 work in the same
+        way. See (*) in the beginning of the comment why the difference is
+        intentional.
+      */
+      execute();
+    }
   }
 
   Field *create_tmp_field_ex(MEM_ROOT *root, TABLE *table, Tmp_field_src *src,