MDEV-29069 follow-up: support partially usable keys

30756775 · Nikita Malyavin · Sergei Golubchik · bac728a2 · 30756775 · 30756775
Commit 30756775 authored Jul 25, 2022 by Nikita Malyavin Committed by Sergei Golubchik Aug 15, 2023
7 changed files
--- a/mysql-test/main/alter_table_online_debug.result
+++ b/mysql-test/main/alter_table_online_debug.result
@@ -1054,8 +1054,8 @@ update t set a = a + 1 where a = 10;
 set debug_sync= 'now signal goforit';
 connection default;
 Warnings:
-Note	1105	Key chosen: -1
-Note	1105	Key chosen: -1
+Note	1105	Key chosen: 0
+Note	1105	Key chosen: 0
 select a from t;
 a
 11

--- a/mysql-test/suite/rpl/r/rpl_alter_extra_persistent.result
+++ b/mysql-test/suite/rpl/r/rpl_alter_extra_persistent.result
@@ -216,4 +216,149 @@ a
 6
 7
 drop table t1;
+connection slave;
+connection master;
+set binlog_row_image=minimal;
+create table t1(a int primary key auto_increment, b int unique);
+insert into t1 values(1, 1);
+insert into t1 values(2, 2);
+insert into t1 values(3, 3);
+insert into t1 values(4, 4);
+insert into t1 values(5, 5);
+connection slave;
+alter table t1 add column d1 int default (b),
+add column z1 int as (b+1) virtual,
+add column z2 int as (b+2) persistent;
+connection master;
+insert into t1 values(6, 6);
+update t1 set a = 11 where a = 1;
+update t1 set b = 12 where b = 2;
+delete from t1 where a = 3;
+delete from t1 where b = 5;
+update t1 set b = 16 where a = 6;
+connection slave;
+select * from t1;
+a	b	d1	z1	z2
+11	1	1	2	3
+2	12	2	13	14
+4	4	4	5	6
+6	16	6	17	18
+# Cleanup
+connection master;
+drop table t1;
+connection slave;
+connection master;
+set binlog_row_image=minimal;
+#
+# MDEV-29069 ER_KEY_NOT_FOUND upon online autoinc addition and
+# concurrent DELETE
+#
+create or replace table t (a int);
+insert into t values (10),(20),(30);
+connection slave;
+alter table t add pk int auto_increment primary key;
+connection master;
+delete from t where a = 20;
+update t set a = a + 1 where a = 10;
+connection slave;
+select * from t;
+a	pk
+11	1
+30	3
+connection master;
+#
+# Add clumsy DEFAULT
+#
+create or replace table t (a int);
+insert into t values (10),(20),(30);
+connection slave;
+alter table t add b int default(RAND() * 20), add key(b),
+algorithm=copy, lock=none;
+connection master;
+delete from t where a = 20;
+update t set a = a + 1 where a = 10;
+connection slave;
+select a from t;
+a
+11
+30
+connection master;
+# CURRENT_TIMESTAMP
+create or replace table t (a int);
+insert into t values (10),(20),(30);
+connection slave;
+alter table t add b timestamp default CURRENT_TIMESTAMP, add key(b);
+connection master;
+delete from t where a = 20;
+update t set a = a + 1 where a = 10;
+connection slave;
+select a from t;
+a
+11
+30
+connection master;
+# CURRENT_TIMESTAMP, mixed key
+create or replace table t (a int);
+insert into t values (10),(20),(30);
+connection slave;
+alter table t add b timestamp default CURRENT_TIMESTAMP, add key(a, b);
+connection master;
+delete from t where a = 20;
+update t set a = a + 1 where a = 10;
+connection slave;
+select a from t;
+a
+11
+30
+connection master;
+# Mixed primary key
+create or replace table t (a int);
+insert into t values (10),(20),(30);
+connection slave;
+alter table t add b int default (1), add primary key(b, a);
+connection master;
+delete from t where a = 20;
+update t set a = a + 1 where a = 10;
+connection slave;
+select a from t;
+a
+11
+30
+connection master;
+#
+# Normal row, could be used as a key
+#
+create or replace table t (a int);
+insert into t values (10),(20),(30);
+connection slave;
+alter table t add b int as (a * 10) unique;
+connection master;
+delete from t where a = 20;
+update t set a = a + 1 where a = 10;
+connection slave;
+select * from t;
+a	b
+11	110
+30	300
+connection master;
+#
+# Add key for old row
+#
+create or replace table t (a int);
+insert into t values (10),(20),(30);
+connection slave;
+alter table t add unique(a);
+connection master;
+delete from t where a = 20;
+update t set a = a + 1 where a = 10;
+connection slave;
+select * from t;
+a
+11
+30
+# Cleanup
+connection master;
+connection slave;
+connection master;
+drop table t;
 include/rpl_end.inc
--- a/mysql-test/suite/rpl/t/rpl_alter_extra_persistent.test
+++ b/mysql-test/suite/rpl/t/rpl_alter_extra_persistent.test
@@ -103,12 +103,162 @@ start slave;

 --source include/wait_for_slave_sql_to_start.inc

--connection master 
+--connection master
 --sync_slave_with_master
 select * from t1 order by a;

 --connection master
 select * from t1 order by a;
 drop table t1;
+--sync_slave_with_master
+--connection master
+
+set binlog_row_image=minimal;
+
+create table t1(a int primary key auto_increment, b int unique);
+insert into t1 values(1, 1);
+insert into t1 values(2, 2);
+insert into t1 values(3, 3);
+insert into t1 values(4, 4);
+insert into t1 values(5, 5);
+
+--sync_slave_with_master
+alter table t1 add column d1 int default (b),
+               add column z1 int as (b+1) virtual,
+               add column z2 int as (b+2) persistent;
+--connection master
+
+insert into t1 values(6, 6);
+update t1 set a = 11 where a = 1;
+update t1 set b = 12 where b = 2;
+
+delete from t1 where a = 3;
+delete from t1 where b = 5;
+
+update t1 set b = 16 where a = 6;
+
+--sync_slave_with_master
+select * from t1;
+
+--echo # Cleanup
+--connection master
+drop table t1;
+--sync_slave_with_master
+--connection master
+
+
+set binlog_row_image=minimal;
+
+--echo #
+--echo # MDEV-29069 ER_KEY_NOT_FOUND upon online autoinc addition and
+--echo # concurrent DELETE
+--echo #
+
+create or replace table t (a int);
+insert into t values (10),(20),(30);
+
+--sync_slave_with_master
+alter table t add pk int auto_increment primary key;
+
+--connection master
+delete from t where a = 20;
+update t set a = a + 1 where a = 10;
+--sync_slave_with_master
+select * from t;
+--connection master
+
+--echo #
+--echo # Add clumsy DEFAULT
+--echo #
+create or replace table t (a int);
+insert into t values (10),(20),(30);
+
+--sync_slave_with_master
+alter table t add b int default(RAND() * 20), add key(b),
+              algorithm=copy, lock=none;
+--connection master
+delete from t where a = 20;
+update t set a = a + 1 where a = 10;
+--sync_slave_with_master
+select a from t;
+--connection master
+
+--echo # CURRENT_TIMESTAMP
+create or replace table t (a int);
+insert into t values (10),(20),(30);
+
+--sync_slave_with_master
+alter table t add b timestamp default CURRENT_TIMESTAMP, add key(b);
+--connection master
+delete from t where a = 20;
+update t set a = a + 1 where a = 10;
+--sync_slave_with_master
+select a from t;
+--connection master
+
+--echo # CURRENT_TIMESTAMP, mixed key
+create or replace table t (a int);
+insert into t values (10),(20),(30);
+
+--sync_slave_with_master
+alter table t add b timestamp default CURRENT_TIMESTAMP, add key(a, b);
+--connection master
+delete from t where a = 20;
+update t set a = a + 1 where a = 10;
+
+--sync_slave_with_master
+select a from t;
+--connection master
+
+--echo # Mixed primary key
+create or replace table t (a int);
+insert into t values (10),(20),(30);
+
+--sync_slave_with_master
+alter table t add b int default (1), add primary key(b, a);
+--connection master
+delete from t where a = 20;
+update t set a = a + 1 where a = 10;
+
+--sync_slave_with_master
+select a from t;
+--connection master
+
+--echo #
+--echo # Normal row, could be used as a key
+--echo #
+create or replace table t (a int);
+insert into t values (10),(20),(30);
+
+--sync_slave_with_master
+alter table t add b int as (a * 10) unique;
+--connection master
+delete from t where a = 20;
+update t set a = a + 1 where a = 10;
+
+--sync_slave_with_master
+select * from t;
+--connection master
+
+--echo #
+--echo # Add key for old row
+--echo #
+create or replace table t (a int);
+insert into t values (10),(20),(30);
+
+--sync_slave_with_master
+alter table t add unique(a);
+--connection master
+delete from t where a = 20;
+update t set a = a + 1 where a = 10;
+
+--sync_slave_with_master
+select * from t;
+
+--echo # Cleanup
+--connection master
+--sync_slave_with_master
+--connection master
+drop table t;

 --source include/rpl_end.inc
--- a/sql/log_event.cc
+++ b/sql/log_event.cc
@@ -2965,7 +2965,7 @@ Rows_log_event::Rows_log_event(const uchar *buf, uint event_len,
 #if !defined(MYSQL_CLIENT) && defined(HAVE_REPLICATION)
    , m_curr_row(NULL), m_curr_row_end(NULL),
    m_key(NULL), m_key_info(NULL), m_key_nr(0),
-    master_had_triggers(0)
+    m_usable_key_parts(0), master_had_triggers(0)
 #endif
 {
  DBUG_ENTER("Rows_log_event::Rows_log_event(const char*,...)");

--- a/sql/log_event.h
+++ b/sql/log_event.h
@@ -4751,6 +4751,7 @@ class Rows_log_event : public Log_event
  uchar    *m_key;      /* Buffer to keep key value during searches */
  KEY      *m_key_info; /* Pointer to KEY info for m_key_nr */
  uint      m_key_nr;   /* Key number */
+  uint      m_usable_key_parts; /* A number of key_parts suited to lookup */
  bool master_had_triggers;     /* set after tables opening */

  /*
@@ -4799,7 +4800,7 @@ class Rows_log_event : public Log_event
  };

  int find_key(const rpl_group_info *); // Find a best key to use in find_row()
-  bool is_key_usable(const KEY *key) const;
+  uint find_key_parts(const KEY *key) const;
  bool use_pk_position() const;
  int find_row(rpl_group_info *);
  int write_row(rpl_group_info *, const bool);

--- a/sql/log_event_server.cc
+++ b/sql/log_event_server.cc
@@ -7104,28 +7104,29 @@ static bool record_compare(TABLE *table, bool vers_from_plain= false)
  Basically we exclude all the default-filled fields based on
  has_explicit_value bitmap.
 */
-bool Rows_log_event::is_key_usable(const KEY *key) const
+uint Rows_log_event::find_key_parts(const KEY *key) const
 {
  RPL_TABLE_LIST *tl= (RPL_TABLE_LIST*)m_table->pos_in_table_list;
  const bool online_alter= tl->m_online_alter_copy_fields;
+  uint p;

  if (!m_table->s->keys_in_use.is_set(uint(key - m_table->key_info)))
-    return false;
+    return 0;

  if (!online_alter)
  {
    if (m_cols.n_bits >= m_table->s->fields)
-      return true;
+      return key->user_defined_key_parts;
    if (m_table->s->virtual_fields + m_table->s->stored_fields == 0)
    {
-      for (uint p= 0; p < key->user_defined_key_parts; p++)
+      for (p= 0; p < key->user_defined_key_parts; p++)
        if (key->key_part[p].fieldnr > m_cols.n_bits)
-          return false;
-      return true;
+          break;
+      return p;
    }
  }

-  for (uint p= 0; p < key->user_defined_key_parts; p++)
+  for (p= 0; p < key->user_defined_key_parts; p++)
  {
    Field *f= key->key_part[p].field;
    /*
@@ -7138,9 +7139,9 @@ bool Rows_log_event::is_key_usable(const KEY *key) const
    bool next_number_field= f == f->table->next_number_field;
    if (!bitmap_is_set(&m_table->has_value_set, f->field_index) &&
        (!online_alter || non_deterministic_default || next_number_field))
-      return false;
+      break;
  }
-  return true;
+  return p;
 }


@@ -7150,8 +7151,8 @@ bool Rows_log_event::is_key_usable(const KEY *key) const
  A primary key is preferred if it exists; otherwise a unique index is
  preferred. Else we pick the index with the smalles rec_per_key value.

-  If a suitable key is found, set @c m_key, @c m_key_nr and @c m_key_info
-  member fields appropriately.
+  If a suitable key is found, set @c m_key, @c m_key_nr, @c m_key_info,
+  and @c m_usable_key_parts member fields appropriately.

  @returns Error code on failure, 0 on success.
 */
@@ -7159,13 +7160,16 @@ int Rows_log_event::find_key(const rpl_group_info *rgi)
 {
  DBUG_ASSERT(m_table);
  RPL_TABLE_LIST *tl= (RPL_TABLE_LIST*)m_table->pos_in_table_list;
-  uint i, best_key_nr;
+  uint i, best_key_nr= 0, best_usable_key_parts= 0;
  KEY *key;
  ulong UNINIT_VAR(best_rec_per_key), tmp;
  DBUG_ENTER("Rows_log_event::find_key");

  if ((best_key_nr= tl->cached_key_nr) != ~0U)
+  {
    DBUG_ASSERT(best_key_nr <= MAX_KEY); // use the cached value
+    best_usable_key_parts= tl->cached_usable_key_parts;
+  }
  else
  {
    best_key_nr= MAX_KEY;
@@ -7206,23 +7210,26 @@ int Rows_log_event::find_key(const rpl_group_info *rgi)
    */
    for (i= 0, key= m_table->key_info; i < m_table->s->keys; i++, key++)
    {
-      if (!is_key_usable(key))
+      uint usable_key_parts= find_key_parts(key);
+      if (usable_key_parts == 0)
        continue;
      /*
        We cannot use a unique key with NULL-able columns to uniquely identify
        a row (but we can still select it for range scan below if nothing better
        is available).
      */
-      if ((key->flags & (HA_NOSAME | HA_NULL_PART_KEY)) == HA_NOSAME)
+      if ((key->flags & (HA_NOSAME | HA_NULL_PART_KEY)) == HA_NOSAME &&
+           usable_key_parts == key->user_defined_key_parts)
      {
        best_key_nr= i;
+        best_usable_key_parts= usable_key_parts;
        break;
      }
      /*
        We can only use a non-unique key if it allows range scans (ie. skip
        FULLTEXT indexes and such).
      */
-      uint last_part= key->user_defined_key_parts - 1;
+      uint last_part= usable_key_parts - 1;
      DBUG_PRINT("info", ("Index %s rec_per_key[%u]= %lu",
                          key->name.str, last_part, key->rec_per_key[last_part]));
      if (!(m_table->file->index_flags(i, last_part, 1) & HA_READ_NEXT))
@@ -7232,13 +7239,16 @@ int Rows_log_event::find_key(const rpl_group_info *rgi)
      if (best_key_nr == MAX_KEY || (tmp > 0 && tmp < best_rec_per_key))
      {
        best_key_nr= i;
+        best_usable_key_parts= usable_key_parts;
        best_rec_per_key= tmp;
      }
    }
    tl->cached_key_nr= best_key_nr;
+    tl->cached_usable_key_parts= best_usable_key_parts;
  }

  m_key_nr= best_key_nr;
+  m_usable_key_parts= best_usable_key_parts;
  if (best_key_nr == MAX_KEY)
    m_key_info= NULL;
  else
@@ -7326,7 +7336,8 @@ bool Rows_log_event::use_pk_position() const
 {
  return m_table->file->ha_table_flags() & HA_PRIMARY_KEY_REQUIRED_FOR_POSITION
      && m_table->s->primary_key < MAX_KEY
-      && m_key_nr == m_table->s->primary_key;
+      && m_key_nr == m_table->s->primary_key
+      && m_usable_key_parts == m_table->key_info->user_defined_key_parts;
 }

 /**
@@ -7483,8 +7494,12 @@ int Rows_log_event::find_row(rpl_group_info *rgi)
      table->record[0][table->s->null_bytes - 1]|=
        256U - (1U << table->s->last_null_bit_pos);

-    error= table->file->ha_index_read_map(table->record[0], m_key, HA_WHOLE_KEY,
-                                          HA_READ_KEY_EXACT);
+    const enum ha_rkey_function find_flag=
+      m_usable_key_parts == m_key_info->user_defined_key_parts
+      ? HA_READ_KEY_EXACT : HA_READ_KEY_OR_NEXT;
+    error= table->file->ha_index_read_map(table->record[0], m_key,
+                                          make_keypart_map(m_usable_key_parts),
+                                          find_flag);
    if (unlikely(error))
    {
      DBUG_PRINT("info",("no record matching the key found in the table"));
@@ -7517,7 +7532,7 @@ int Rows_log_event::find_row(rpl_group_info *rgi)
      found.  I can see no scenario where it would be incorrect to
      chose the row to change only using a PK or an UNNI.
    */
-    if (table->key_info->flags & HA_NOSAME)
+    if (find_flag == HA_READ_KEY_EXACT && table->key_info->flags & HA_NOSAME)
    {
      /* Unique does not have non nullable part */
      if (!(table->key_info->flags & HA_NULL_PART_KEY))

--- a/sql/rpl_utility.h
+++ b/sql/rpl_utility.h
@@ -256,6 +256,7 @@ struct RPL_TABLE_LIST : public TABLE_LIST
  const Copy_field *m_online_alter_copy_fields;
  const Copy_field *m_online_alter_copy_fields_end;
  uint cached_key_nr;                  // [0..MAX_KEY] if set, ~0U if unset
+  uint cached_usable_key_parts;
  bool m_tabledef_valid;
  bool master_had_triggers;