New simpler bugfix for UPDATE and virtual BLOBs

When updating a table with virtual BLOB columns, the following might
happen:
- an old record is read from the table, it has no virtual blob values
- update_virtual_fields() is run, vcol blob gets its value into the
  record. But only a pointer to the value is in the table->record[0],
  the value is in Field_blob::value String (but it doesn't have to be!
  it can be in the record, if the column is just a copy of another
  columns: ... b VARCHAR, c BLOB AS (b) ...)
- store_record(table,record[1]), old record now is in record[1]
- fill_record() prepares new values in record[0], vcol blob is updated,
  new value replaces the old one in the Field_blob::value
- now both record[1] and record[0] have a pointer that points to the
  *new* vcol blob value. Or record[1] has a pointer to nowhere if
   Field_blob::value had to realloc.

To fix this I have introduced a new String object 'read_value' in
Field_blob.  When updating virtual columns when a row has been read,
the allocated value is stored in 'read_value' instead of 'value'.  The
allocated blobs for the new row is stored in 'value' as before.

I also made, as a safety precaution, the insert delayed handling of
blobs more general by using value to store strings instead of the
record.  This ensures that virtual functions on delayed insert should
work in as in the case of normal insert.

Triggers are now properly updating the read, write and vcol maps for used
fields. This means that we don't need VCOL_UPDATE_FOR_READ_WRITE anymore
and there is no need for any other special handling of triggers in
update_virtual_fields().

To be able to test how many times virtual fields are invoked, I also
relaxed rules that one can use local (@) variables in DEFAULT and non
persistent virtual field expressions.

1 files changed, 37 insertions, 4 deletions

diff --git a/sql/sql_insert.cc b/sql/sql_insert.cc
index 7b583e1ddec..cea42667c48 100644
--- a/sql/sql_insert.cc
+++ b/sql/sql_insert.cc
@@ -1691,8 +1691,12 @@ int write_record(THD *thd, TABLE *table,COPY_INFO *info)
       }
       if (table->vfield)
       {
+        /*
+          We have not yet called update_virtual_fields(VOL_UPDATE_FOR_READ)
+          in handler methods for the just read row in record[1].
+        */
         table->move_fields(table->field, table->record[1], table->record[0]);
-        table->update_virtual_fields(VCOL_UPDATE_INDEXED);
+        table->update_virtual_fields(VCOL_UPDATE_FOR_REPLACE);
         table->move_fields(table->field, table->record[0], table->record[1]);
       }
       if (info->handle_duplicates == DUP_UPDATE)
@@ -2912,6 +2916,8 @@ pthread_handler_t handle_delayed_insert(void *arg)
     thd->mdl_context.set_needs_thr_lock_abort(TRUE);
 
     di->table->mark_columns_needed_for_insert();
+    /* Mark all columns for write as we don't know which columns we get from user */
+    bitmap_set_all(di->table->write_set);
 
     /* Now wait until we get an insert or lock to handle */
     /* We will not abort as long as a client thread uses this thread */
@@ -3079,7 +3085,7 @@ pthread_handler_t handle_delayed_insert(void *arg)
 }
 
 
-/* Remove pointers from temporary fields to allocated values */
+/* Remove all pointers to data for blob fields so that original table doesn't try to free them */
 
 static void unlink_blobs(register TABLE *table)
 {
@@ -3097,9 +3103,23 @@ static void free_delayed_insert_blobs(register TABLE *table)
   for (Field **ptr=table->field ; *ptr ; ptr++)
   {
     if ((*ptr)->flags & BLOB_FLAG)
+      ((Field_blob *) *ptr)->free();
+  }
+}
+
+
+/* set value field for blobs to point to data in record */
+
+static void set_delayed_insert_blobs(register TABLE *table)
+{
+  for (Field **ptr=table->field ; *ptr ; ptr++)
+  {
+    if ((*ptr)->flags & BLOB_FLAG)
     {
-      my_free(((Field_blob *) (*ptr))->get_ptr());
-      ((Field_blob *) (*ptr))->reset();
+      Field_blob *blob= ((Field_blob *) *ptr);
+      uchar *data= blob->get_ptr();
+      if (data)
+        blob->set_value(data);     // Set value.ptr() to point to data
     }
   }
 }
@@ -3157,6 +3177,8 @@ bool Delayed_insert::handle_inserts(void)
     stacked_inserts--;
     mysql_mutex_unlock(&mutex);
     memcpy(table->record[0],row->record,table->s->reclength);
+    if (table->s->blob_fields)
+      set_delayed_insert_blobs(table);
 
     thd.start_time=row->start_time;
     thd.query_start_used=row->query_start_used;
@@ -3227,6 +3249,16 @@ bool Delayed_insert::handle_inserts(void)
     if (info.handle_duplicates == DUP_UPDATE)
       table->file->extra(HA_EXTRA_INSERT_WITH_UPDATE);
     thd.clear_error(); // reset error for binlog
+
+    if (table->vfield)
+    {
+      /*
+        Virtual fields where not calculated by caller as the temporary TABLE object used
+        had vcol_set empty. Better to calculate them here to make the caller faster.
+      */
+     table->update_virtual_fields(VCOL_UPDATE_FOR_WRITE);
+    }
+
     if (write_record(&thd, table, &info))
     {
       info.error_count++;				// Ignore errors
@@ -3348,6 +3380,7 @@ bool Delayed_insert::handle_inserts(void)
     if (table->s->blob_fields)
     {
       memcpy(table->record[0],row->record,table->s->reclength);
+      set_delayed_insert_blobs(table);
       free_delayed_insert_blobs(table);
     }
     delete row;