MDEV-39179: Incorrect NULL handling in UPDATE ... RETURNING result

Analysis:
OLD_VALUE() swapped only field->ptr, leaving null_ptr pointing to the
current record. This caused incorrect NULL results.

Fix:
Store null_ptr_old for record[1] and swap it together with ptr to
preserve correct NULL semantics.

Author: mariadb-RuchaDeodhar

mysql-test/main/update.result

@@ -1137,4 +1137,15 @@ UPDATE t3 SET a = 10 RETURNING OLD_VALUE(a) AS old_value;
 old_value
 1
 DROP TABLE t3;
+#
+# MDEV-39179: Incorrect NULL handling in UPDATE ... RETURNING result
+#
+CREATE OR REPLACE TABLE t1(a VARCHAR(10));
+INSERT INTO t1 VALUES('') RETURNING *;
+a
+
+UPDATE t1 SET a=NULL RETURNING OLD_VALUE(a) AS old, a AS new;
+old	new
+	NULL
+DROP TABLE t1;
 # End of 13.0 test

mysql-test/main/update.test

@@ -995,4 +995,14 @@ UPDATE t3 SET a = 10 RETURNING OLD_VALUE(a) AS old_value;
 
 DROP TABLE t3;
 
+--echo #
+--echo # MDEV-39179: Incorrect NULL handling in UPDATE ... RETURNING result
+--echo #
+
+CREATE OR REPLACE TABLE t1(a VARCHAR(10));
+
+INSERT INTO t1 VALUES('') RETURNING *;
+UPDATE t1 SET a=NULL RETURNING OLD_VALUE(a) AS old, a AS new;
+DROP TABLE t1;
+
 --echo # End of 13.0 test

sql/field.cc

@@ -1934,8 +1934,8 @@ String *Field::val_int_as_str(String *val_buffer, bool unsigned_val)
 Field::Field(uchar *ptr_arg,uint32 length_arg,uchar *null_ptr_arg,
 	     uchar null_bit_arg,
 	     utype unireg_check_arg, const LEX_CSTRING *field_name_arg)
-  :ptr(ptr_arg),
-  null_ptr(null_ptr_arg), table(0), orig_table(0),
+  :ptr(ptr_arg),  ptr_old(nullptr),
+  null_ptr(null_ptr_arg), null_ptr_old(nullptr), table(0), orig_table(0),
   table_name(0), field_name(*field_name_arg), option_list(0),
   option_struct(0), key_start(0), part_of_key(0),
   part_of_key_not_clustered(0), vcol_direct_part_of_key(0), part_of_sortkey(0),

sql/field.h

@@ -809,8 +809,10 @@ class Field: public Value_source
   /**
      Byte where the @c NULL bit is stored inside a record. If this Field is a
      @c NOT @c NULL field, this member is @c NULL.
+     null_ptr_old points to old field.
   */
   uchar		*null_ptr;
+  uchar   *null_ptr_old;
   /*
     Note that you can use table->in_use as replacement for current_thd member
     only inside of val_*() and store() members (e.g. you can't use it in cons)

sql/item.cc

@@ -7966,13 +7966,19 @@ bool Item_old_field::send(Protocol *protocol, st_value *buffer)
 {
   bool result;
 
-  std::swap(field->ptr_old, field->ptr);
+  change_field_ptr();
   result= Item_field::send(protocol, buffer);
-  std::swap(field->ptr_old, field->ptr);
+  change_field_ptr();
 
   return result;
 }
 
+void Item_old_field::change_field_ptr()
+{
+  std::swap(field->ptr_old, field->ptr);
+  std::swap(field->null_ptr, field->null_ptr_old);
+}
+
 
 bool Item_old_field::fix_fields(THD *thd, Item **reference)
 {
@@ -7985,6 +7991,16 @@ bool Item_old_field::fix_fields(THD *thd, Item **reference)
   {
     field->ptr_old= field->table->record[1] +
                   field->offset(field->table->record[0]);
+    if (field->null_ptr)
+    {
+      field->null_ptr_old =
+        field->table->record[1] +
+        (field->null_ptr - field->table->record[0]);
+    }
+    else
+    {
+      field->null_ptr_old = nullptr;
+    }
   }
   else
   {

sql/item.h

@@ -4114,6 +4114,7 @@ class Item_old_field: public Item_field
   bool send(Protocol *protocol, st_value *buffer) override;
   Type type() const override { return FIELD_OLD_ITEM; }
   bool fix_fields(THD *, Item **) override;
+  void change_field_ptr();
 
 private:
   uchar *saved_row_ref;

sql/sql_update.cc

@@ -391,6 +391,7 @@ bool Sql_cmd_update::update_single_table(THD *thd)
   Explain_update *explain;
   query_plan.index= MAX_KEY;
   query_plan.using_filesort= FALSE;
+  bool record_was_same, need_update, trg_skip_row;
 
   // For System Versioning (may need to insert new fields to a table).
   ha_rows rows_inserted= 0;
@@ -977,7 +978,7 @@ bool Sql_cmd_update::update_single_table(THD *thd)
     if (unlikely(returning_result->send_result_set_metadata(
                             thd->lex->returning()->returning_list,
                             Protocol::SEND_NUM_ROWS | Protocol::SEND_EOF)))
-      error= 1;
+      goto error;
   }
 
   while (!(error=info.read_record()) && !thd->killed)
@@ -996,7 +997,7 @@ bool Sql_cmd_update::update_single_table(THD *thd)
         cut_fields_for_portion_of_time(thd, table,
                                        table_list->period_conditions);
 
-      bool trg_skip_row= false;
+      trg_skip_row= false;
       if (fill_record_n_invoke_before_triggers(thd, table, *fields, *values, 0,
                                                TRG_EVENT_UPDATE,
                                                &trg_skip_row))
@@ -1011,8 +1012,8 @@ bool Sql_cmd_update::update_single_table(THD *thd)
 
       found++;
 
-      bool record_was_same= false;
-      bool need_update= !can_compare_record || compare_record(table);
+      record_was_same= false;
+      need_update= !can_compare_record || compare_record(table);
 
       if (need_update)
       {
@@ -3262,7 +3263,10 @@ bool Sql_cmd_update::execute_inner(THD *thd)
       /* This is UPDATE ... RETURNING.  It will return output to the client */
       if (thd->lex->analyze_stmt)
       {
-        returning_result= new (thd->mem_root) select_send_analyze(thd);
+        if (!(returning_result= new (thd->mem_root) select_send_analyze(thd)))
+        {
+          return true;
+        }
         save_protocol= thd->protocol;
         thd->protocol= new Protocol_discard(thd);
       }
@@ -3294,6 +3298,7 @@ bool Sql_cmd_update::execute_inner(THD *thd)
   {
     delete thd->protocol;
     thd->protocol= save_protocol;
+    save_protocol= nullptr;
   }
   if (unlikely(res))
   {

sql/sql_update.h

@@ -48,7 +48,7 @@ class Sql_cmd_update final : public Sql_cmd_dml
   ha_rows found{0}, updated{0};
   Sql_cmd_update(bool multitable_arg)
     : orig_multitable(multitable_arg), multitable(multitable_arg),
-    save_protocol(nullptr)
+     returning_result(nullptr), save_protocol(nullptr)
   {}
 
   enum_sql_command sql_command_code() const override

sql/sql_yacc.yy

@@ -9935,6 +9935,7 @@ expr:
                 lex->sql_command == SQLCOM_UPDATE_MULTI))
             {
               my_error(ER_WRONG_USAGE, MYF(0), "OLD_VALUE", "non-UPDATE");
+              MYSQL_YYABORT;
             }
             $$= new (thd->mem_root)
                                 Item_old_field(thd,
@@ -14583,6 +14584,7 @@ opt_returning:
             {
               my_error(ER_NOT_SUPPORTED_YET, MYF(0),
                          "RETURNING for multi-table UPDATE");
+              MYSQL_YYABORT;
             }
 
             /*