Skip to content

Commit

Permalink
ddl: disallow dropping auto_increment column attribute (#12107) (#12145)
Browse files Browse the repository at this point in the history
  • Loading branch information
tangenta authored and zz-jason committed Sep 12, 2019
1 parent 8a79382 commit 842c81c
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 @@ -1556,11 +1556,15 @@ func (s *testIntegrationSuite4) 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 @@ -2609,6 +2609,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 @@ -401,6 +401,9 @@ type SessionVars struct {

// DurationCompile is the duration of compiling AST to execution plan of the last query.
DurationCompile time.Duration

// 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 @@ -453,6 +456,7 @@ func NewSessionVars() *SessionVars {
SlowQueryFile: config.GetGlobalConfig().Log.SlowQueryFile,
WaitSplitRegionFinish: DefTiDBWaitSplitRegionFinish,
WaitSplitRegionTimeout: DefWaitSplitRegionTimeout,
AllowRemoveAutoInc: DefTiDBAllowRemoveAutoInc,
}
vars.Concurrency = Concurrency{
IndexLookupConcurrency: DefIndexLookupConcurrency,
Expand Down Expand Up @@ -834,6 +838,8 @@ func (s *SessionVars) SetSystemVar(name string, val string) error {
s.TxnMode = strings.ToUpper(val)
case TiDBLowResolutionTSO:
s.LowResolutionTSO = TiDBOptOn(val)
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 @@ -701,6 +701,7 @@ var defaultSysVars = []*SysVar{
{ScopeSession, TiDBWaitSplitRegionTimeout, strconv.Itoa(DefWaitSplitRegionTimeout)},
{ScopeSession, TiDBLowResolutionTSO, "0"},
{ScopeSession, TiDBExpensiveQueryTimeThreshold, strconv.Itoa(DefTiDBExpensiveQueryTimeThreshold)},
{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 @@ -142,6 +142,9 @@ const (

// TiDBLowResolutionTSO is used for reading data with low resolution TSO which is updated once every two seconds
TiDBLowResolutionTSO = "tidb_low_resolution_tso"

// 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 @@ -344,6 +347,7 @@ const (
DefTiDBWaitSplitRegionFinish = true
DefTiDBExpensiveQueryTimeThreshold = 60 // 60s
DefWaitSplitRegionTimeout = 300 // 300s
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 @@ -569,6 +569,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 842c81c

Please sign in to comment.