New subject: [Maria-developers] ce5cc8fb905: MDEV-29021 ALTER TABLE fails when a stored virtual column is dropped+added

7 Jul 2022

Hi, Nikita,

This is good, but I think fill_extra_persistent_columns() shouldn't be
used at all.

It doesn't handle default values. I've changed rpl_alter_extra_persistent test
as

-alter table t1 add column z1 int as(a+1) virtual, add column z2 int as (a+2) persistent;
+alter table t1 add column z1 int as(a+1) virtual, add column z2 int default (a+2);

and, of course, new column z2 wasn't updated properly.
An easy fix would be to use your code in all cases and remove
fill_extra_persistent_columns().

Could you do that too, please? And add the test case too. May be just,
append

   , add column z3 int default (a+2)

On Jul 07, Nikita Malyavin wrote:
...
revision-id: ce5cc8fb905 (mariadb-10.6.1-506-gce5cc8fb905)
parent(s): 49ad875902e
author: Nikita Malyavin
committer: Nikita Malyavin
timestamp: 2022-07-04 15:55:03 +0300
message:
MDEV-29021 ALTER TABLE fails when a stored virtual column is dropped+added
We shouldn't rely on `fill_extra_persistent_columns`, as it only updates
fields which have an index > cols->n_bits (replication bitmap width).
Normal update_virtual_fields+update_default_fields shoudl be done.

diff --git a/sql/rpl_record.cc b/sql/rpl_record.cc
index 17731e64600..42f476fc112 100644
--- a/sql/rpl_record.cc
+++ b/sql/rpl_record.cc
@@ -412,6 +412,26 @@ int unpack_row(rpl_group_info *rgi, TABLE *table, uint const colcnt,
     {
       copy->do_copy(copy);
     }
+    if (table->default_field)
+    {
+      error= table->update_default_fields(table->in_use->lex->ignore);
+      if (unlikely(error))
+        DBUG_RETURN(error);
+    }
+    if (table->vfield)
+    {
+      error= table->update_virtual_fields(table->file, VCOL_UPDATE_FOR_WRITE);
+      if (unlikely(error))
+        DBUG_RETURN(error);
+    }
+  }
+  else
+  {
+    /*
+      Add Extra slave persistent columns
+    */
+    if (unlikely(error= fill_extra_persistent_columns(table, cols->n_bits)))
+      DBUG_RETURN(error);
   }
/*
@@ -439,12 +459,6 @@ int unpack_row(rpl_group_info *rgi, TABLE *table, uint const colcnt,
     }
   }
-  /*
-    Add Extra slave persistent columns
-  */
-  if (unlikely(error= fill_extra_persistent_columns(table, cols->n_bits)))
-    DBUG_RETURN(error);
-
   /*
     We should now have read all the null bytes, otherwise something is
     really wrong.
Regards,
Sergei
VP of MariaDB Server Engineering
and security@mariadb.org

    

Re: [Maria-developers] ce5cc8fb905: MDEV-29021 ALTER TABLE fails when a stored virtual column is dropped+added

Sergei Golubchik

Nikita Malyavin

Sergei Golubchik

Nikita Malyavin

Sergei Golubchik

tags

participants (2)