MDEV-14821 Assertion !is_set() || (m_status == DA_OK_BULK && is_bulk_op()) failed in Diagnostics_area::set_ok_status

[kevgs]

No description provided.

[midenok]

Pruning is based on range constants. vers_update_range_constants() is required for correct pruning after stat changes.

[kevgs]

Force fix part_column_list_val always (after each UPDATE, DELETE) to not read a freed memory from previous queries. Updated.

[midenok]

Can you describe what exactly this fixes? Why you decided to refix col_val when it was not updated? Looks like ad hoc solution. Where Field_timestampf was constructed, where it was destroyed? I don't see such info in analysis.

[kevgs]

Here is some info. Hope it's suitable to get the picture.

Why you decided to refix col_val when it was not updated? Looks like ad hoc solution.

Yes, you're right. It just fixes one of several observed errors: reading freed memory.

Where Field_timestampf was constructed, where it was destroyed?

We don't care because we don't touch it's memory. We touch a memory which this instance points to: TABLE::record[0]

Btw, 3 kinds of timestamp comparison problems happens also in partition.test. But tests pass :(

Do you have some thoughts on how to do a proper fix?

[kevgs]

Btw, 3 kinds of timestamp comparison problems happens also in partition.test. But tests pass :(

That's partially incorrect: no freed memory read in partition.test.

[midenok]

This is not TABLE::record[0]: Field_timestampf::ptr can point to any arbitrary buffer. This is general scheme to fix premature destruction:

1. Find where the buffer was allocated;
2. Find where the buffer was freed;
3. Find where the buffer was accessed;
4. Fix allocation: use longer living pool which covers 1 and 3.

By word where I always mean stack traces.

[kevgs]

#441 (comment)

thd->mem_root is ok for column_value of other types of partitioning but not enough for versioning partition.

Here is patch with another mem_root which is in use in partition_info.cc. Assertion failure disappears. What do you think about it?

diff --git a/sql/partition_info.cc b/sql/partition_info.cc
index e1e83db7b31..d102d5bf379 100644
--- a/sql/partition_info.cc
+++ b/sql/partition_info.cc
@@ -2972,7 +2972,8 @@ bool partition_info::fix_column_value_functions(THD *thd,
         }
         thd->got_warning= save_got_warning;
         thd->variables.sql_mode= save_sql_mode;
-        if (!(val_ptr= (uchar*) thd->memdup(field->ptr, len)))
+        if (!(val_ptr= (uchar *)(vers_info ? memdup_root(&table->s->mem_root, field->ptr, len)
+                                           : thd->memdup(field->ptr, len))))
         {
           mem_alloc_error(len);
           result= TRUE;

[midenok]

This certainly should not be TABLE_SHARE::mem_root as it is inter-thread.
Try this:

--- a/sql/partition_info.h
+++ b/sql/partition_info.h
@@ -562,7 +562,10 @@ class partition_info : public Sql_alloc
       Field *f= part_field_array[i];
       bitmap_set_bit(f->table->write_set, f->field_index);
     }
+    MEM_ROOT *old_root= thd->mem_root;
+    thd->mem_root= &table->mem_root;
     result= check_range_constants(thd, false);
+    thd->mem_root= old_root;
     vers_info->stat_serial= table->s->stat_serial;
     mysql_rwlock_unlock(&table->s->LOCK_stat_serial);
     return result;

or find connection-wide mem_root.

[kevgs]

That worked. PR updated.

[midenok]

Is it reproducible via LIMIT? I don't like 2 seconds delay in test.

[kevgs]

I failed to reproduce it with limit.

[midenok]

Rebase on trunk and use this case:

create or replace table t1 (x int) with system versioning;
insert into t1 values (0), (1);
update t1 set x= x + 1;
alter table t1 partition by system_time limit 1 (
    partition p1 history,
    partition p2 history,
    partition pn current);
delete from t1 where x = 1;
delete from t1 where x = 2;

Merge when done.