Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow changing a column to be an AUTO_INCREMENT #39143

Open
dveeden opened this issue Nov 14, 2022 · 2 comments
Open

Allow changing a column to be an AUTO_INCREMENT #39143

dveeden opened this issue Nov 14, 2022 · 2 comments
Labels
type/feature-request Categorizes issue or PR as related to a new feature.

Comments

@dveeden
Copy link
Contributor

dveeden commented Nov 14, 2022

Feature Request

Changing an integer column to be an AUTO_INCREMENT after the table is created isn't supported by TiDB, but is supported by MySQL.

TiDB

sql> CREATE TABLE t1 (id INT PRIMARY KEY);
Query OK, 0 rows affected (0.1691 sec)

sql> SHOW CREATE TABLE t1\G
*************************** 1. row ***************************
       Table: t1
Create Table: CREATE TABLE `t1` (
  `id` int(11) NOT NULL,
  PRIMARY KEY (`id`) /*T![clustered_index] CLUSTERED */
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin
1 row in set (0.0014 sec)

sql> ALTER TABLE t1 MODIFY COLUMN `id` int(11) NOT NULL AUTO_INCREMENT;
ERROR: 8200 (HY000): Unsupported modify column: can't set auto_increment

sql> SELECT tidb_version()\G
*************************** 1. row ***************************
tidb_version(): Release Version: v6.5.0-alpha
Edition: Community
Git Commit Hash: bdcb85031e07d5a75f4e7742dcf4f7fed7dbdfe3
Git Branch: heads/refs/tags/v6.5.0-alpha
UTC Build Time: 2022-11-14 14:33:27
GoVersion: go1.19.2
Race Enabled: false
TiKV Min Version: 6.2.0-alpha
Check Table Before Drop: false
Store: tikv
1 row in set (0.0014 sec)

And for AUTO_RANDOM:

sql> ALTER TABLE t1 MODIFY COLUMN `id` int(11) NOT NULL AUTO_RANDOM;
ERROR: 8216 (HY000): Invalid auto random: auto_random can only be converted from auto_increment clustered primary key

MySQL 8.0

sql> CREATE TABLE t1 (id INT PRIMARY KEY);
Query OK, 0 rows affected (0.0524 sec)

sql> SHOW CREATE TABLE t1\G
*************************** 1. row ***************************
       Table: t1
Create Table: CREATE TABLE `t1` (
  `id` int NOT NULL,
  PRIMARY KEY (`id`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_ai_ci
1 row in set (0.0027 sec)

sql> ALTER TABLE t1 MODIFY COLUMN `id` int NOT NULL AUTO_INCREMENT;
Query OK, 0 rows affected (0.1036 sec)

Records: 0  Duplicates: 0  Warnings: 0

sql> SELECT VERSION();
+-----------+
| VERSION() |
+-----------+
| 8.0.31    |
+-----------+
1 row in set (0.0007 sec)

PHPMyAdmin

As an example about why users would need/want this:

A backup made by phpMyAdmin 4.9.7 has this:

  • A CREATE TABLE statement for a table. The table doesn't have indexes.
  • A number of INSERT INTO statements to load the data
  • An ALTER TABLE statement to add multiple indexes and set a primary key
  • An ALTER TABLE statement to change the id column to be AUTO_INCREMENT

This way of re-creating a table with data was probably done to optimize for MySQL with the MyISAM storage engine and/or very old InnoDB versions. However this backup can still be loaded into MySQL 8.0 today.

On TiDB all statements work fine, except for the very last one.

There are probably other situations where changing a column to be an AUTO_INCREMENT or AUTO_RANDOM after the initial creation.

A similar case where changing a table after creation can be a problem is #38453

@dveeden dveeden added the type/feature-request Categorizes issue or PR as related to a new feature. label Nov 14, 2022
@dveeden
Copy link
Contributor Author

dveeden commented Nov 14, 2022

cc @tiancaiamao @hawkingrei

@dveeden
Copy link
Contributor Author

dveeden commented Nov 14, 2022

With a small patch I can actually change the column:

diff --git a/ddl/ddl_api.go b/ddl/ddl_api.go
index bbf3d4fc5..973086f35 100644
--- a/ddl/ddl_api.go
+++ b/ddl/ddl_api.go
@@ -4696,10 +4696,6 @@ func GetModifiableColumnJob(
                }
        }
 
-       // We don't support modifying column from not_auto_increment to auto_increment.
-       if !mysql.HasAutoIncrementFlag(col.GetFlag()) && mysql.HasAutoIncrementFlag(newCol.GetFlag()) {
-               return nil, dbterror.ErrUnsupportedModifyColumn.GenWithStackByArgs("can't set auto_increment")
-       }
        // Disallow modifying column from auto_increment to not auto_increment if the session variable `AllowRemoveAutoInc` is false.
        if !sctx.GetSessionVars().AllowRemoveAutoInc && mysql.HasAutoIncrementFlag(col.GetFlag()) && !mysql.HasAutoIncrementFlag(newCol.GetFlag()) {
                return nil, dbterror.ErrUnsupportedModifyColumn.GenWithStackByArgs("can't remove auto_increment without @@tidb_allow_remove_auto_inc enabled")
sql> CREATE TABLE t1 (id INT PRIMARY KEY);
Query OK, 0 rows affected (0.0318 sec)

sql> INSERT INTO t1 VALUES (1),(2),(3);
Query OK, 3 rows affected (0.0020 sec)

Records: 3  Duplicates: 0  Warnings: 0

sql> ALTER TABLE t1 MODIFY COLUMN `id` int NOT NULL AUTO_INCREMENT;
Query OK, 0 rows affected (0.0291 sec)

sql> INSERT INTO t1() VALUES ();
ERROR: 1062 (23000): Duplicate entry '1' for key 't1.PRIMARY'

sql> INSERT INTO t1() VALUES ();
ERROR: 1062 (23000): Duplicate entry '2' for key 't1.PRIMARY'

sql> INSERT INTO t1() VALUES ();
ERROR: 1062 (23000): Duplicate entry '3' for key 't1.PRIMARY'

sql> INSERT INTO t1() VALUES ();
Query OK, 1 row affected (0.0015 sec)

sql> drop table t1;
Query OK, 0 rows affected (0.1317 sec)
sql> CREATE TABLE t1 (id INT PRIMARY KEY);
Query OK, 0 rows affected (0.0871 sec)

sql> INSERT INTO t1 VALUES (1),(2),(3);
Query OK, 3 rows affected (0.0020 sec)

Records: 3  Duplicates: 0  Warnings: 0

sql> ALTER TABLE t1 MODIFY COLUMN `id` int NOT NULL AUTO_INCREMENT, AUTO_INCREMENT=5;
Query OK, 0 rows affected (0.0825 sec)

sql> INSERT INTO t1() VALUES ();
Query OK, 1 row affected (0.0020 sec)

sql> TABLE t1;
+----+
| id |
+----+
|  1 |
|  2 |
|  3 |
|  5 |
+----+
4 rows in set (0.0018 sec)

So it looks like this works for an empty table and when the AUTO_INCREMENT= is manually set.

So what's likely to be needed besides this is to make sure this is safe for multiple connections and multiple TiDB servers (Lock the table metadata?) and to automatically set the AUTO_INCREMENT= to the MAX(...) of the column.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/feature-request Categorizes issue or PR as related to a new feature.
Projects
None yet
Development

No branches or pull requests

1 participant