[MDEV-25816] alter table may change primary key from 0 to 1 Created: 2021-05-29  Updated: 2021-08-06  Resolved: 2021-08-06

Status: Closed
Project: MariaDB Server
Component/s: Data Definition - Alter Table, Documentation
Affects Version/s: 10.5.10
Fix Version/s: N/A

Type: Bug Priority: Major
Reporter: chengyao jiang Assignee: Ian Gilfillan
Resolution: Fixed Votes: 0
Labels: None


 Description   

Hi,

MariaDB may change the primary key from 0 to 1 under following scenario.
And this problem happened in our production environment, which has caused a disaster for the business.

MariaDB [test]> select version();
+-----------------+
| version()       |
+-----------------+
| 10.5.10-MariaDB |
+-----------------+
1 row in set (0.000 sec)
 
MariaDB [test]> create table y ( a int primary key, b varchar(5));
Query OK, 0 rows affected (0.032 sec)
 
MariaDB [test]> insert into y values (0,'a'),(2,'b');
Query OK, 2 rows affected (0.004 sec)
Records: 2  Duplicates: 0  Warnings: 0
 
MariaDB [test]> select * from y;
+---+------+
| a | b    |
+---+------+
| 0 | a    |
| 2 | b    |
+---+------+
2 rows in set (0.000 sec)
 
MariaDB [test]> alter table y modify column a int auto_increment;
Query OK, 2 rows affected (0.095 sec)
Records: 2  Duplicates: 0  Warnings: 0
 
MariaDB [test]> select * from y;
+---+------+
| a | b    |
+---+------+
| 1 | a    |
| 2 | b    |
+---+------+
2 rows in set (0.001 sec)

As we can see, MariaDB will change primary key value from 0 to 1 if column changes to auto_increment.

I review the source code in sql/sql_table.cc, MariaDB does the logic of preserving zero value to auto_increment column.

  /*
          If we are going to copy contents of one auto_increment column to
          another auto_increment column it is sensible to preserve zeroes.
          This condition also covers case when we are don't actually alter
          auto_increment column.
        */
       if (def->field == from->found_next_number_field)
           thd->variables.sql_mode |= MODE_NO_AUTO_VALUE_ON_ZERO;

From the source code, we can see that MariaDB will preserve zero value. But it miss the scenario when changing from 0 column value to 0 auto_increment column value.

Followings are my fix. When new table has auto_increment column, sql_mode should enable MODE_NO_AUTO_VALUE_ON_ZERO automatically.

--- sql_table2.cc       2021-05-26 15:20:41.974362582 +0800
+++ sql_table.cc        2021-05-26 15:01:18.592772749 +0800
@@ -17925,8 +17925,8 @@
           This condition also covers case when we are don't actually alter
           auto_increment column.
         */
-        if (def->field == from->found_next_number_field)
-            thd->variables.sql_mode |= MODE_NO_AUTO_VALUE_ON_ZERO;
+        //if (def->field == from->found_next_number_field)
+        thd->variables.sql_mode |= MODE_NO_AUTO_VALUE_ON_ZERO;
       }
       (copy_end++)->set(*ptr, def->field, false);

Now the result of alter table is right, 0 value will not change to 1.



 Comments   
Comment by Alice Sherepa [ 2021-05-31 ]

Just FYI, KB recommends to use sql_mode=NO_AUTO_VALUE_ON_ZERO for handling 0:

MariaDB [test]> create table y ( a int primary key, b varchar(5));
Query OK, 0 rows affected (0.009 sec)
 
MariaDB [test]> insert into y values (0,'a'),(3,'b');
Query OK, 2 rows affected (0.001 sec)
Records: 2  Duplicates: 0  Warnings: 0
 
MariaDB [test]> select * from y;
+---+------+
| a | b    |
+---+------+
| 0 | a    |
| 3 | b    |
+---+------+
2 rows in set (0.000 sec)
 
MariaDB [test]> set sql_mode=NO_AUTO_VALUE_ON_ZERO;
Query OK, 0 rows affected (0.000 sec)
 
MariaDB [test]> alter table y modify column a int auto_increment;
Query OK, 2 rows affected (0.273 sec)              
Records: 2  Duplicates: 0  Warnings: 0
 
MariaDB [test]> select * from y;
+---+------+
| a | b    |
+---+------+
| 0 | a    |
| 3 | b    |
+---+------+
2 rows in set (0.001 sec)

Comment by Sergei Golubchik [ 2021-05-31 ]

This is intentional. That if() condition you're referring to specifically disables auto-increment value generation when the old column already is already declared with auto_increment. When an auto_increment attribute is added it is supposed to generate new values according to the auto-increment rules.
For example:

MariaDB [test]> create table t1 (a int);
Query OK, 0 rows affected (0.017 sec)
 
MariaDB [test]> insert t1 values (0),(0),(0);
Query OK, 3 rows affected (0.001 sec)
Records: 3  Duplicates: 0  Warnings: 0
 
MariaDB [test]> alter table t1 modify a int not null auto_increment primary key; 
Query OK, 3 rows affected (0.020 sec)              
Records: 3  Duplicates: 0  Warnings: 0
 
MariaDB [test]> select * from t1;
+---+
| a |
+---+
| 1 |
| 2 |
| 3 |
+---+
3 rows in set (0.002 sec)

That is, it's a legitimate way to fill a column with increasing numbers.

But the documentation, apparently, is not sufficiently clear about this behavior. We'll clarify it.

Comment by chengyao jiang [ 2021-06-01 ]

serg

For your test, I suggest alter table should fail and throw duplicate key error.

And you will see following example shows 0 value will not increase to 1 because the code handles this scenario:

mysql> show create table x\G
*************************** 1. row ***************************
       Table: x
Create Table: CREATE TABLE `x` (
  `a` int NOT NULL AUTO_INCREMENT,
  `b` varchar(1) COLLATE utf8mb4_general_ci DEFAULT NULL,
  PRIMARY KEY (`a`)
) ENGINE=InnoDB AUTO_INCREMENT=3 DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_general_ci
1 row in set (0.00 sec)
 
mysql> select * from x;
+---+------+
| a | b    |
+---+------+
| 0 | a    |
| 2 | b    |
+---+------+
2 rows in set (0.00 sec)
 
mysql> alter table x modify column b varchar(5);
Query OK, 0 rows affected (0.14 sec)
Records: 0  Duplicates: 0  Warnings: 0
 
mysql> select * from x;
+---+------+
| a | b    |
+---+------+
| 0 | a    |
| 2 | b    |
+---+------+
2 rows in set (0.00 sec)

But it does not cover the situation when changing from non auto_increment column to auto_increment as I showed before.

As for NO_AUTO_VALUE_ON_ZERO, from my previous understanding, it only affects insert behavior. Alter table should try it best to preserve the old value in the table.

And for alter table tool, like gh-ost, pt-osc, they will preserve zero value when altering table from non-auto_increment to auto_increment. What they do is to enable NO_AUTO_VALUE_ON_ZERO automatically before inserting old value to table.

See: https://github.com/github/gh-ost/issues/722

We also encountered this problem in our production environment, which has caused big problem for our business. Primary key = 0 is a very important record, and now it changes to 1 after altering table.

So I suggest MariaDB do the same step as gh-ost and pt-osc do so as to preserve the old value.
Or else, users will get a different alter table result after using MariaDB or gh-ost/pt-osc.

Comment by Sergei Golubchik [ 2021-06-05 ]

I cannot speak for design decisions made by gh-ost and pt-osc authors.

This ALTER TABLE behavior is most certainly not a bug, but an intentional feature, a specifically implemented way to generate auto-increment values for a column.

We can, of course, change it. But it will be changing an intentional behavior that worked for many years, an incompatible change that will very likely break applications of some of MariaDB users. It is not something we do lightly, we generally need a very strong and convincing reason to do it and a very good explanation for the users whose applications will be broken by this change.

Generally, unless you really use 0 (and not only NULL) to generate auto-increment values, I'd recommend you to set sql_mode=NO_AUTO_VALUE_ON_ZERO in the my.cnf file, globally. This will solve this ALTER TABLE issue and possibly other cases where your 0 is unintentionally replaced with something else.

Comment by chengyao jiang [ 2021-06-08 ]

This ALTER TABLE behavior is most certainly not a bug, but an intentional feature, a specifically implemented way to generate auto-increment values for a column.

If this was an intentional feature, my test2 alter table should change value from 0 to 1, but it doesn't happen.

What alter table do is :

1. create tmp table
2. insert rows from old table to tmp table
3. rename tmp table to old table

The problem lies in when inserting rows to tmp table, MySQL will treat zero value as auto_increment value, and the value will be change. And the source code has the logic to preserve zero value. But it does not cover the scenario when change from non-auto_increment primary key to auto_increment primary key.

gh-ost user has met the problem in there business, it happened in our production environment as well.

And I cannot image who will use existing 0 value to generate auto_increment values when doing internationally alter table , it's so crazy.

I do agree with you when enable sql_mode=NO_AUTO_VALUE_ON_ZERO, zero value will preserve.
After enable NO_AUTO_VALUE_ON_ZERO in MySQL internally, user has no choice to generate auto_increment value if they really want to.

However, I think more choice is not a good choice, it makes mess.

Comment by Ian Gilfillan [ 2021-07-27 ]

I've added documentation to clarify the ALTER TABLE scenario. I'll leave the issue open for now for serg or others to address the final comment, but since this is intentional, if a change is desired, it should be a new feature request.

Generated at Thu Feb 08 09:40:37 UTC 2024 using Jira 8.20.16#820016-sha1:9d11dbea5f4be3d4cc21f03a88dd11d8c8687422.