Skip to content

Commit

Permalink
ddl: disallow dropping auto_increment column attribute (#12107)
Browse files Browse the repository at this point in the history
  • Loading branch information
tangenta authored and crazycs520 committed Sep 10, 2019
1 parent 46cb64a commit bbd131b
Show file tree
Hide file tree
Showing 6 changed files with 28 additions and 1 deletion.
6 changes: 5 additions & 1 deletion ddl/db_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1542,11 +1542,15 @@ func (s *testIntegrationSuite3) TestAlterColumn(c *C) {
createSQL = result.Rows()[0][1]
expected = "CREATE TABLE `mc` (\n `a` bigint(20) NOT NULL AUTO_INCREMENT,\n `b` int(11) DEFAULT NULL,\n PRIMARY KEY (`a`)\n) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin"
c.Assert(createSQL, Equals, expected)
s.tk.MustExec("alter table mc modify column a bigint") // Drops auto_increment
_, err = s.tk.Exec("alter table mc modify column a bigint") // Droppping auto_increment is not allow when @@tidb_allow_remove_auto_inc == 'off'
c.Assert(err, NotNil)
s.tk.MustExec("set @@tidb_allow_remove_auto_inc = on")
s.tk.MustExec("alter table mc modify column a bigint") // Dropping auto_increment is ok when @@tidb_allow_remove_auto_inc == 'on'
result = s.tk.MustQuery("show create table mc")
createSQL = result.Rows()[0][1]
expected = "CREATE TABLE `mc` (\n `a` bigint(20) NOT NULL,\n `b` int(11) DEFAULT NULL,\n PRIMARY KEY (`a`)\n) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin"
c.Assert(createSQL, Equals, expected)

_, err = s.tk.Exec("alter table mc modify column a bigint auto_increment") // Adds auto_increment should throw error
c.Assert(err, NotNil)

Expand Down
4 changes: 4 additions & 0 deletions ddl/ddl_api.go
Original file line number Diff line number Diff line change
Expand Up @@ -2690,6 +2690,10 @@ func (d *ddl) getModifiableColumnJob(ctx sessionctx.Context, ident ast.Ident, or
if !mysql.HasAutoIncrementFlag(col.Flag) && mysql.HasAutoIncrementFlag(newCol.Flag) {
return nil, errUnsupportedModifyColumn.GenWithStackByArgs("set auto_increment")
}
// Disallow modifying column from auto_increment to not auto_increment if the session variable `AllowRemoveAutoInc` is false.
if !ctx.GetSessionVars().AllowRemoveAutoInc && mysql.HasAutoIncrementFlag(col.Flag) && !mysql.HasAutoIncrementFlag(newCol.Flag) {
return nil, errUnsupportedModifyColumn.GenWithStackByArgs("to remove auto_increment without @@tidb_allow_remove_auto_inc enabled")
}

// We support modifying the type definitions of 'null' to 'not null' now.
var modifyColumnTp byte
Expand Down
6 changes: 6 additions & 0 deletions sessionctx/variable/session.go
Original file line number Diff line number Diff line change
Expand Up @@ -417,6 +417,9 @@ type SessionVars struct {

// PrevStmt is used to store the previous executed statement in the current session.
PrevStmt string

// AllowRemoveAutoInc indicates whether a user can drop the auto_increment column attribute or not.
AllowRemoveAutoInc bool
}

// ConnectionInfo present connection used by audit.
Expand Down Expand Up @@ -473,6 +476,7 @@ func NewSessionVars() *SessionVars {
EnableIndexMerge: false,
EnableNoopFuncs: DefTiDBEnableNoopFuncs,
ReplicaRead: kv.ReplicaReadLeader,
AllowRemoveAutoInc: DefTiDBAllowRemoveAutoInc,
}
vars.Concurrency = Concurrency{
IndexLookupConcurrency: DefIndexLookupConcurrency,
Expand Down Expand Up @@ -857,6 +861,8 @@ func (s *SessionVars) SetSystemVar(name string, val string) error {
} else if strings.EqualFold(val, "leader") || len(val) == 0 {
s.ReplicaRead = kv.ReplicaReadLeader
}
case TiDBAllowRemoveAutoInc:
s.AllowRemoveAutoInc = TiDBOptOn(val)
}
s.systems[name] = val
return nil
Expand Down
1 change: 1 addition & 0 deletions sessionctx/variable/sysvar.go
Original file line number Diff line number Diff line change
Expand Up @@ -706,6 +706,7 @@ var defaultSysVars = []*SysVar{
{ScopeSession, TiDBExpensiveQueryTimeThreshold, strconv.Itoa(DefTiDBExpensiveQueryTimeThreshold)},
{ScopeGlobal | ScopeSession, TiDBEnableNoopFuncs, BoolToIntStr(DefTiDBEnableNoopFuncs)},
{ScopeSession, TiDBReplicaRead, "leader"},
{ScopeSession, TiDBAllowRemoveAutoInc, BoolToIntStr(DefTiDBAllowRemoveAutoInc)},
}

// SynonymsSysVariables is synonyms of system variables.
Expand Down
4 changes: 4 additions & 0 deletions sessionctx/variable/tidb_vars.go
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,9 @@ const (

// TiDBReplicaRead is used for reading data from replicas, followers for example.
TiDBReplicaRead = "tidb_replica_read"

// TiDBAllowRemoveAutoInc indicates whether a user can drop the auto_increment column attribute or not.
TiDBAllowRemoveAutoInc = "tidb_allow_remove_auto_inc"
)

// TiDB system variable names that both in session and global scope.
Expand Down Expand Up @@ -358,6 +361,7 @@ const (
DefTiDBWaitSplitRegionFinish = true
DefWaitSplitRegionTimeout = 300 // 300s
DefTiDBEnableNoopFuncs = false
DefTiDBAllowRemoveAutoInc = false
)

// Process global variables.
Expand Down
8 changes: 8 additions & 0 deletions sessionctx/variable/varsutil.go
Original file line number Diff line number Diff line change
Expand Up @@ -574,6 +574,14 @@ func ValidateSetSystemVar(vars *SessionVars, name string, value string) (string,
default:
return value, ErrWrongValueForVar.GenWithStackByArgs(TiDBTxnMode, value)
}
case TiDBAllowRemoveAutoInc:
switch {
case strings.EqualFold(value, "ON") || value == "1":
return "on", nil
case strings.EqualFold(value, "OFF") || value == "0":
return "off", nil
}
return value, ErrWrongValueForVar.GenWithStackByArgs(name, value)
}
return value, nil
}
Expand Down

0 comments on commit bbd131b

Please sign in to comment.