When an update changes an indexes field, InnoDB undo-logs all fields that
are part of any index, to be used during purge. This requires that all
indexed virtual columns are computed by the server layer. However, InnoDB
was missing code to ensure this.

For example, UPDATE t SET a=2 will put in the read_set only column a and the
primary key. This patch extends ha_innobase::::column_bitmaps_signal() to
add the required indexed virtual columns to the read_set whenever an indexed
column is in the write set.

This problem was previously (before a4e4a56720c) hidden because the server
layer put extra columns in the read_set unconditionally. But it is obviously
wrong for the server to request read of unrelated columns in a statement
such as UPDATE t SET a=2; this could negatively impact the performance of
storage engines that do not require such reads (potentially severely so if
eg. the extra columns contain large blobs).

Signed-off-by: Kristian Nielsen <[email protected]>
---
 mysql-test/suite/innodb/r/vcol_index.result | 52 +++++++++++++++++
 mysql-test/suite/innodb/t/vcol_index.test   | 63 +++++++++++++++++++++
 storage/innobase/handler/ha_innodb.cc       | 37 ++++++++----
 3 files changed, 140 insertions(+), 12 deletions(-)
 create mode 100644 mysql-test/suite/innodb/r/vcol_index.result
 create mode 100644 mysql-test/suite/innodb/t/vcol_index.test

diff --git a/mysql-test/suite/innodb/r/vcol_index.result 
b/mysql-test/suite/innodb/r/vcol_index.result
new file mode 100644
index 00000000000..42a1ccbcfae
--- /dev/null
+++ b/mysql-test/suite/innodb/r/vcol_index.result
@@ -0,0 +1,52 @@
+SET SESSION TRANSACTION ISOLATION LEVEL READ COMMITTED;
+CREATE TABLE t (
+pk INT AUTO_INCREMENT,
+a INT DEFAULT 0,
+b INT DEFAULT 0,
+c BLOB DEFAULT 0,
+KEY (a),
+PRIMARY KEY (pk),
+UNIQUE (b,c)
+) ENGINE=InnoDB PARTITION BY KEY (pk) PARTITIONS 2;
+INSERT IGNORE INTO t (b,c) VALUES (0,0),(0,0);
+Warnings:
+Warning        1062    Duplicate entry '0-0' for key 'b'
+UPDATE t SET c = 1;
+INSERT INTO t (b,c) VALUES (0,0);
+UPDATE t SET a = 2;
+InnoDB         0 transactions not purged
+DROP TABLE t;
+CREATE TABLE t1(
+pk INT AUTO_INCREMENT,
+a INT DEFAULT 0,
+b BLOB DEFAULT 0,
+KEY (a), PRIMARY KEY (pk), UNIQUE (b)
+) ENGINE=InnoDB STATS_PERSISTENT= 0;
+START TRANSACTION WITH CONSISTENT SNAPSHOT;
+CONNECT con1,localhost,root,,;
+INSERT INTO t1 (pk, b) VALUES(1, 0);
+DELETE FROM t1;
+INSERT INTO t1 (pk, b) VALUES(1, 0);
+UPDATE t1 set a = 2;
+disconnect con1;
+connection default;
+COMMIT;
+InnoDB         0 transactions not purged
+DROP TABLE t1;
+CREATE TABLE t1(
+pk INT AUTO_INCREMENT,
+a INT DEFAULT 0,
+b INT as (1 + 1) VIRTUAL,
+KEY (a), PRIMARY KEY (pk), UNIQUE (b)
+) ENGINE=InnoDB STATS_PERSISTENT= 0;
+START TRANSACTION WITH CONSISTENT SNAPSHOT;
+CONNECT con1,localhost,root,,;
+INSERT INTO t1 (pk) VALUES(1);
+DELETE FROM t1;
+INSERT INTO t1 (pk) VALUES(1);
+UPDATE t1 set a = 2;
+disconnect con1;
+connection default;
+COMMIT;
+InnoDB         0 transactions not purged
+DROP TABLE t1;
diff --git a/mysql-test/suite/innodb/t/vcol_index.test 
b/mysql-test/suite/innodb/t/vcol_index.test
new file mode 100644
index 00000000000..6d11ceef298
--- /dev/null
+++ b/mysql-test/suite/innodb/t/vcol_index.test
@@ -0,0 +1,63 @@
+--source include/have_innodb.inc
+--source include/have_partition.inc
+
+SET SESSION TRANSACTION ISOLATION LEVEL READ COMMITTED;
+
+#
+# MDEV-39081: InnoDB: tried to purge non-delete-marked record, assertion fails
+#
+CREATE TABLE t (
+  pk INT AUTO_INCREMENT,
+  a INT DEFAULT 0,
+  b INT DEFAULT 0,
+  c BLOB DEFAULT 0,
+  KEY (a),
+  PRIMARY KEY (pk),
+  UNIQUE (b,c)
+  ) ENGINE=InnoDB PARTITION BY KEY (pk) PARTITIONS 2;
+INSERT IGNORE INTO t (b,c) VALUES (0,0),(0,0);
+UPDATE t SET c = 1;
+INSERT INTO t (b,c) VALUES (0,0);
+UPDATE t SET a = 2;
+
+--source include/wait_all_purged.inc
+DROP TABLE t;
+
+
+CREATE TABLE t1(
+  pk INT AUTO_INCREMENT,
+  a INT DEFAULT 0,
+  b BLOB DEFAULT 0,
+  KEY (a), PRIMARY KEY (pk), UNIQUE (b)
+  ) ENGINE=InnoDB STATS_PERSISTENT= 0;
+START TRANSACTION WITH CONSISTENT SNAPSHOT;
+
+CONNECT(con1,localhost,root,,);
+INSERT INTO t1 (pk, b) VALUES(1, 0);
+DELETE FROM t1;
+INSERT INTO t1 (pk, b) VALUES(1, 0);
+UPDATE t1 set a = 2;
+DISCONNECT con1;
+CONNECTION default;
+COMMIT;
+--source include/wait_all_purged.inc
+DROP TABLE t1;
+
+CREATE TABLE t1(
+  pk INT AUTO_INCREMENT,
+  a INT DEFAULT 0,
+  b INT as (1 + 1) VIRTUAL,
+  KEY (a), PRIMARY KEY (pk), UNIQUE (b)
+  ) ENGINE=InnoDB STATS_PERSISTENT= 0;
+START TRANSACTION WITH CONSISTENT SNAPSHOT;
+
+CONNECT(con1,localhost,root,,);
+INSERT INTO t1 (pk) VALUES(1);
+DELETE FROM t1;
+INSERT INTO t1 (pk) VALUES(1);
+UPDATE t1 set a = 2;
+DISCONNECT con1;
+CONNECTION default;
+COMMIT;
+--source include/wait_all_purged.inc
+DROP TABLE t1;
diff --git a/storage/innobase/handler/ha_innodb.cc 
b/storage/innobase/handler/ha_innodb.cc
index 5c8ed59c48e..10628a44fb5 100644
--- a/storage/innobase/handler/ha_innodb.cc
+++ b/storage/innobase/handler/ha_innodb.cc
@@ -5053,6 +5053,8 @@ Needs to be done for indexes that are being added with 
inplace ALTER
 in a different thread, because from the server point of view these
 columns are not yet indexed.
 Also needed if the primary key is being updated.
+Also needed for an update of any indexed column, as the undo logging of
+such an update logs the values of all keys.
 */
 void ha_innobase::column_bitmaps_signal()
 {
@@ -5063,31 +5065,42 @@ void ha_innobase::column_bitmaps_signal()
   bool is_online_log= clust_index->online_log;
 
   /*
-    Check if the clustered index is being updated.
-    If so, it is necessary to compute virtual columns that are part of
-    secondary indexes, to be able to re-compute those indexes.
+    Check if a key is being updated.
+
+    If the clustered index is being updated, it is necessary to compute
+    virtual columns that are part of secondary indexes, to be able to
+    re-compute those indexes.
+
+    If an UPDATE statement modifies any key, all indexed virtual columns must
+    be computed so that their values can be undo-logged.
   */
-  bool upd_pk= false;
-  for (uint j= 0; j < clust_index->n_user_defined_cols; ++j)
-  {
-    dict_col_t *col= clust_index->fields[j].col;
-    if (bitmap_is_set(table->write_set, static_cast<uint>(col->ind)))
+  bool upd_ord= false;
+  uint num= 0;
+  uint num_v= 0;
+  const dict_table_t *ib_table= m_prebuilt->table;
+  for (uint i = 0; i < table->s->fields; i++) {
+    const dict_col_t *col;
+    if (table->field[i]->stored_in_db())
+      col= dict_table_get_nth_col(ib_table, num++);
+    else
+      col= &dict_table_get_nth_v_col(ib_table, num_v++)->m_col;
+    if (bitmap_is_set(table->write_set, i) && col->ord_part)
     {
-      upd_pk= 1;
+      upd_ord= true;
       break;
     }
   }
 
-  if (!is_online_log && !upd_pk)
+  if (!is_online_log && !upd_ord)
     return;
 
-  uint num_v= 0;
+  num_v= 0;
   for (uint j = 0; j < table->s->virtual_fields; j++)
   {
     if (table->vfield[j]->stored_in_db())
       continue;
 
-    dict_col_t *col= &m_prebuilt->table->v_cols[num_v].m_col;
+    dict_col_t *col= &ib_table->v_cols[num_v].m_col;
     if (col->ord_part ||
         (is_online_log && dict_index_is_online_ddl(clust_index) &&
          row_log_col_is_indexed(clust_index, num_v)))
-- 
2.47.3

_______________________________________________
commits mailing list -- [email protected]
To unsubscribe send an email to [email protected]

Reply via email to