MDEV-29069 follow-up: improve DEFAULT rules

previously, fields with DEFAULTs were allowed just when expression is
deterministic. In case of online alter, we should recursively check that
underlying fields of expression also either have explicit values, or
have DEFAULT following this validity rule.

4 files changed, 127 insertions, 12 deletions

diff --git a/mysql-test/main/alter_table_online_debug.result b/mysql-test/main/alter_table_online_debug.result
index e06adf168fe..bb61566c61f 100644
--- a/mysql-test/main/alter_table_online_debug.result
+++ b/mysql-test/main/alter_table_online_debug.result
@@ -1111,6 +1111,48 @@ connection default;
 Warnings:
 Note	1105	Key chosen: 0
 Note	1105	Key chosen: 0
+#
+# Useful UNIQUE, though a virtual column on another extra field
+#
+create or replace table t (a int primary key, b int default (a+1));
+insert into t values (10, 10),(20, 20),(30, 30);
+set debug_sync= 'alter_table_online_downgraded signal downgraded wait_for goforit';
+alter table t drop primary key, add c int default(a),
+add d int as (c) stored unique, algorithm=copy, lock=none;
+connection con2;
+set debug_sync= 'now wait_for downgraded';
+delete from t where a = 20;
+update t set a = a + 2 where a = 10;
+set debug_sync= 'now signal goforit';
+connection default;
+Warnings:
+Note	1105	Key chosen: 0
+Note	1105	Key chosen: 0
+select * from t;
+a	b	c	d
+12	10	12	12
+30	30	30	30
+#
+# Now this index is not usable (missing DEFAULT on field c)
+#
+create or replace table t (a int primary key, b int);
+insert into t values (10, 10),(20, 20),(30, 30);
+set debug_sync= 'alter_table_online_downgraded signal downgraded wait_for goforit';
+alter table t drop primary key, add c real default(rand(a)),
+add d real as (c) stored unique, algorithm=copy, lock=none;
+connection con2;
+set debug_sync= 'now wait_for downgraded';
+delete from t where a = 20;
+update t set a = a + 2 where a = 10;
+set debug_sync= 'now signal goforit';
+connection default;
+Warnings:
+Note	1105	Key chosen: -1
+Note	1105	Key chosen: -1
+select a, b from t;
+a	b
+12	10
+30	30
 drop table t;
 set debug_sync= reset;
 set debug_dbug= @old_debug;
diff --git a/mysql-test/main/alter_table_online_debug.test b/mysql-test/main/alter_table_online_debug.test
index 97a6eb63b5f..b169be818d9 100644
--- a/mysql-test/main/alter_table_online_debug.test
+++ b/mysql-test/main/alter_table_online_debug.test
@@ -1284,6 +1284,42 @@ set debug_sync= 'now signal goforit';
 --reap
 
 
+--echo #
+--echo # Useful UNIQUE, though a virtual column on another extra field
+--echo #
+create or replace table t (a int primary key, b int default (a+1));
+insert into t values (10, 10),(20, 20),(30, 30);
+set debug_sync= 'alter_table_online_downgraded signal downgraded wait_for goforit';
+--send
+alter table t drop primary key, add c int default(a),
+              add d int as (c) stored unique, algorithm=copy, lock=none;
+--connection con2
+set debug_sync= 'now wait_for downgraded';
+delete from t where a = 20;
+update t set a = a + 2 where a = 10;
+set debug_sync= 'now signal goforit';
+--connection default
+--reap
+select * from t;
+
+--echo #
+--echo # Now this index is not usable (missing DEFAULT on field c)
+--echo #
+create or replace table t (a int primary key, b int);
+insert into t values (10, 10),(20, 20),(30, 30);
+set debug_sync= 'alter_table_online_downgraded signal downgraded wait_for goforit';
+--send
+alter table t drop primary key, add c real default(rand(a)),
+              add d real as (c) stored unique, algorithm=copy, lock=none;
+--connection con2
+set debug_sync= 'now wait_for downgraded';
+delete from t where a = 20;
+update t set a = a + 2 where a = 10;
+set debug_sync= 'now signal goforit';
+--connection default
+--reap
+select a, b from t;
+
 # Cleanup
 drop table t;
 set debug_sync= reset;
diff --git a/sql/field.h b/sql/field.h
index 7b6896b0012..5b10613f67e 100644
--- a/sql/field.h
+++ b/sql/field.h
@@ -1192,7 +1192,7 @@ public:
   {
     bitmap_set_bit(&table->has_value_set, field_index);
   }
-  bool has_explicit_value()
+  bool has_explicit_value() const
   {
     return bitmap_is_set(&table->has_value_set, field_index);
   }
diff --git a/sql/log_event_server.cc b/sql/log_event_server.cc
index a923de2e7bf..4be484d52a0 100644
--- a/sql/log_event_server.cc
+++ b/sql/log_event_server.cc
@@ -7014,6 +7014,51 @@ record_compare_exit:
 record_compare_differ:
   return true;
 }
+/**
+  Traverses default item expr of a field, and underlying field's default values.
+  If it is an extra field and has no value replicated, then its default expr
+  should be also checked.
+ */
+class Rpl_key_part_checker: public Field_enumerator
+{
+  bool online_alter;
+  Field *next_number_field;
+  bool field_usable;
+public:
+
+
+  void visit_field(Item_field *item) override
+  {
+    if (!field_usable)
+      return;
+    field_usable= check_field(item->field);
+  }
+
+  bool check_field(Field *f)
+  {
+    if (f->has_explicit_value())
+      return true;
+
+    if ((!f->vcol_info && !online_alter) || f == next_number_field)
+      return false;
+
+    Virtual_column_info *computed= f->vcol_info ? f->vcol_info
+                                   : f->default_value;
+
+    if (computed == NULL)
+      return true; // No DEFAULT, or constant DEFAULT
+
+    // Deterministic DEFAULT or vcol expression
+    return !(computed->flags & VCOL_NOT_STRICTLY_DETERMINISTIC)
+           && !computed->expr->walk(&Item::enumerate_field_refs_processor,
+                                    false, this)
+           && field_usable;
+  }
+
+  Rpl_key_part_checker(bool online_alter, Field *next_number_field):
+    online_alter(online_alter), next_number_field(next_number_field),
+    field_usable(true) {}
+};
 
 
 /**
@@ -7044,19 +7089,11 @@ uint Rows_log_event::find_key_parts(const KEY *key) const
     }
   }
 
+  Rpl_key_part_checker key_part_checker(online_alter,
+                                        m_table->found_next_number_field);
   for (p= 0; p < key->user_defined_key_parts; p++)
   {
-    Field *f= key->key_part[p].field;
-    /*
-      in the online alter case (but not in replication) we don't have
-      to reject an index if it includes new columns, as long as
-      their values are deterministic.
-    */
-    bool non_deterministic_default= f->default_value &&
-                     f->default_value->flags & VCOL_NOT_STRICTLY_DETERMINISTIC;
-    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))
+    if (!key_part_checker.check_field(key->key_part[p].field))
       break;
   }
   return p;