Skip to content

Commit d3ba56f

Browse files
elianddbelianddb
authored
Merge pull request #3084 from dolthub/elianddb/9481-auto-increment-type-validation
dolthub/dolt#9481 - Move auto_increment validation to GMS
2 parents 06c9a9d + e57e563 commit d3ba56f

File tree

3 files changed

+24
-99
lines changed

3 files changed

+24
-99
lines changed

‎enginetest/queries/insert_queries.go

Lines changed: 0 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1285,38 +1285,6 @@ var InsertScripts = []ScriptTest{
12851285
},
12861286
},
12871287
},
1288-
{
1289-
Name: "auto increment on float",
1290-
Dialect: "mysql",
1291-
SetUpScript: []string{
1292-
"create table auto (pk float primary key auto_increment)",
1293-
"insert into auto values (NULL),(10),(0)",
1294-
},
1295-
Assertions: []ScriptTestAssertion{
1296-
{
1297-
Query: "select * from auto order by 1",
1298-
Expected: []sql.Row{
1299-
{float64(1)}, {float64(10)}, {float64(11)},
1300-
},
1301-
},
1302-
},
1303-
},
1304-
{
1305-
Name: "auto increment on double",
1306-
Dialect: "mysql",
1307-
SetUpScript: []string{
1308-
"create table auto (pk double primary key auto_increment)",
1309-
"insert into auto values (NULL),(10),(0)",
1310-
},
1311-
Assertions: []ScriptTestAssertion{
1312-
{
1313-
Query: "select * from auto order by 1",
1314-
Expected: []sql.Row{
1315-
{float64(1)}, {float64(10)}, {float64(11)},
1316-
},
1317-
},
1318-
},
1319-
},
13201288
{
13211289
Name: "sql_mode=NO_auto_value_ON_ZERO",
13221290
Dialect: "mysql",

‎enginetest/queries/script_queries.go

Lines changed: 6 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -8157,7 +8157,6 @@ where
81578157

81588158
// Char tests
81598159
{
8160-
Skip: true,
81618160
Name: "char with auto_increment",
81628161
Dialect: "mysql",
81638162
SetUpScript: []string{},
@@ -8171,7 +8170,6 @@ where
81718170

81728171
// Varchar tests
81738172
{
8174-
Skip: true,
81758173
Name: "varchar with auto_increment",
81768174
Dialect: "mysql",
81778175
SetUpScript: []string{},
@@ -8185,7 +8183,6 @@ where
81858183

81868184
// Binary tests
81878185
{
8188-
Skip: true,
81898186
Name: "binary with auto_increment",
81908187
Dialect: "mysql",
81918188
SetUpScript: []string{},
@@ -8199,7 +8196,6 @@ where
81998196

82008197
// Varbinary tests
82018198
{
8202-
Skip: true,
82038199
Name: "varbinary with auto_increment",
82048200
Dialect: "mysql",
82058201
SetUpScript: []string{},
@@ -8213,7 +8209,6 @@ where
82138209

82148210
// Blob tests
82158211
{
8216-
Skip: true,
82178212
Name: "blob with auto_increment",
82188213
Dialect: "mysql",
82198214
SetUpScript: []string{},
@@ -8224,22 +8219,21 @@ where
82248219
},
82258220
{
82268221
Query: "create table bad (tb tinyblob primary key auto_increment);",
8227-
ExpectedErrStr: "Incorrect column specifier for column 'b'",
8222+
ExpectedErrStr: "Incorrect column specifier for column 'tb'",
82288223
},
82298224
{
82308225
Query: "create table bad (mb mediumblob primary key auto_increment);",
8231-
ExpectedErrStr: "Incorrect column specifier for column 'b'",
8226+
ExpectedErrStr: "Incorrect column specifier for column 'mb'",
82328227
},
82338228
{
82348229
Query: "create table bad (lb longblob primary key auto_increment);",
8235-
ExpectedErrStr: "Incorrect column specifier for column 'b'",
8230+
ExpectedErrStr: "Incorrect column specifier for column 'lb'",
82368231
},
82378232
},
82388233
},
82398234

82408235
// Text Tests
82418236
{
8242-
Skip: true,
82438237
Name: "text with auto_increment",
82448238
Dialect: "mysql",
82458239
SetUpScript: []string{},
@@ -9659,7 +9653,6 @@ where
96599653
},
96609654
},
96619655
{
9662-
Skip: true,
96639656
Name: "set with auto increment",
96649657
Dialect: "mysql",
96659658
SetUpScript: []string{
@@ -10016,7 +10009,6 @@ where
1001610009

1001710010
// Bit Tests
1001810011
{
10019-
Skip: true,
1002010012
Name: "bit with auto_increment",
1002110013
Dialect: "mysql",
1002210014
SetUpScript: []string{},
@@ -10114,35 +10106,32 @@ where
1011410106

1011510107
// Float Tests
1011610108
{
10117-
Skip: true,
1011810109
Name: "float with auto_increment",
1011910110
Dialect: "mysql",
1012010111
SetUpScript: []string{},
1012110112
Assertions: []ScriptTestAssertion{
1012210113
{
10123-
Query: "create table bad (f float primary key auto_increment);",
10114+
Query: "create table float_tbl (f float primary key auto_increment);",
1012410115
ExpectedErrStr: "Incorrect column specifier for column 'f'",
1012510116
},
1012610117
},
1012710118
},
1012810119

1012910120
// Double Tests
1013010121
{
10131-
Skip: true,
1013210122
Name: "double with auto_increment",
1013310123
Dialect: "mysql",
1013410124
SetUpScript: []string{},
1013510125
Assertions: []ScriptTestAssertion{
1013610126
{
10137-
Query: "create table bad (d double primary key auto_increment);",
10138-
ExpectedErrStr: "Incorrect column specifier for column 'vc'",
10127+
Query: "create table double_tbl (d double primary key auto_increment);",
10128+
ExpectedErrStr: "Incorrect column specifier for column 'd'",
1013910129
},
1014010130
},
1014110131
},
1014210132

1014310133
// Decimal Tests
1014410134
{
10145-
Skip: true,
1014610135
Name: "decimal with auto_increment",
1014710136
Dialect: "mysql",
1014810137
SetUpScript: []string{},
@@ -10226,7 +10215,6 @@ where
1022610215

1022710216
// Date Tests
1022810217
{
10229-
Skip: true,
1023010218
Name: "date with auto_increment",
1023110219
Dialect: "mysql",
1023210220
SetUpScript: []string{},
@@ -10240,7 +10228,6 @@ where
1024010228

1024110229
// Datetime Tests
1024210230
{
10243-
Skip: true,
1024410231
Name: "datetime with auto_increment",
1024510232
Dialect: "mysql",
1024610233
SetUpScript: []string{},
@@ -10258,7 +10245,6 @@ where
1025810245

1025910246
// Timestamp Tests
1026010247
{
10261-
Skip: true,
1026210248
Name: "timestamp with auto_increment",
1026310249
Dialect: "mysql",
1026410250
SetUpScript: []string{},
@@ -10276,7 +10262,6 @@ where
1027610262

1027710263
// Time Tests
1027810264
{
10279-
Skip: true,
1028010265
Name: "time with auto_increment",
1028110266
Dialect: "mysql",
1028210267
SetUpScript: []string{},
@@ -10294,7 +10279,6 @@ where
1029410279

1029510280
// Year Tests
1029610281
{
10297-
Skip: true,
1029810282
Name: "year with auto_increment",
1029910283
Dialect: "mysql",
1030010284
SetUpScript: []string{},

‎sql/analyzer/validate_create_table.go

Lines changed: 18 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -40,30 +40,31 @@ func validateCreateTable(ctx *sql.Context, a *Analyzer, n sql.Node, scope *plan.
4040

4141
sch := ct.PkSchema().Schema
4242
idxs := ct.Indexes()
43-
strictMySQLCompat, err := isStrictMysqlCompatibilityEnabled(ctx)
44-
if err != nil {
45-
return nil, transform.SameTree, err
43+
44+
// First validate auto_increment columns before other validations
45+
// This ensures proper error precedence matching MySQL behavior
46+
keyedColumns := make(map[string]bool)
47+
for _, index := range idxs {
48+
for _, col := range index.Columns {
49+
keyedColumns[col.Name] = true
50+
}
4651
}
47-
err = validateIndexes(ctx, sch, idxs, strictMySQLCompat)
52+
53+
err = validateAutoIncrementModify(sch, keyedColumns)
4854
if err != nil {
4955
return nil, transform.SameTree, err
5056
}
5157

52-
err = validateNoVirtualColumnsInPrimaryKey(sch)
58+
strictMySQLCompat, err := isStrictMysqlCompatibilityEnabled(ctx)
5359
if err != nil {
5460
return nil, transform.SameTree, err
5561
}
56-
57-
// passed validateIndexes, so they all must be valid indexes
58-
// extract map of columns that have indexes defined over them
59-
keyedColumns := make(map[string]bool)
60-
for _, index := range idxs {
61-
for _, col := range index.Columns {
62-
keyedColumns[col.Name] = true
63-
}
62+
err = validateIndexes(ctx, sch, idxs, strictMySQLCompat)
63+
if err != nil {
64+
return nil, transform.SameTree, err
6465
}
6566

66-
err = validateAutoIncrementModify(sch, keyedColumns)
67+
err = validateNoVirtualColumnsInPrimaryKey(sch)
6768
if err != nil {
6869
return nil, transform.SameTree, err
6970
}
@@ -336,7 +337,7 @@ func resolveAlterColumn(ctx *sql.Context, a *Analyzer, n sql.Node, scope *plan.S
336337
// We can't evaluate auto-increment until the end of the analysis, since we break adding a new auto-increment unique
337338
// column into two steps: first add the column, then create the index. If there was no index created, that's an error.
338339
if addedColumn {
339-
err = validateAutoIncrementAdd(sch, keyedColumns)
340+
err = validateAutoIncrementModify(sch, keyedColumns)
340341
if err != nil {
341342
return nil, false, err
342343
}
@@ -791,8 +792,8 @@ func validateAutoIncrementModify(schema sql.Schema, keyedColumns map[string]bool
791792
seen := false
792793
for _, col := range schema {
793794
if col.AutoIncrement {
794-
// Check if column type is valid for auto_increment
795-
if types.IsEnum(col.Type) {
795+
// Under MySQL 8.4+, AUTO_INCREMENT columns must be integer types.
796+
if !types.IsInteger(col.Type) {
796797
return sql.ErrInvalidColumnSpecifier.New(col.Name)
797798
}
798799
// keyedColumns == nil means they are trying to add auto_increment column
@@ -814,34 +815,6 @@ func validateAutoIncrementModify(schema sql.Schema, keyedColumns map[string]bool
814815
return nil
815816
}
816817

817-
func validateAutoIncrementAdd(schema sql.Schema, keyColumns map[string]bool) error {
818-
seen := false
819-
for _, col := range schema {
820-
if col.AutoIncrement {
821-
{
822-
// Check if column type is valid for auto_increment
823-
if types.IsEnum(col.Type) {
824-
return sql.ErrInvalidColumnSpecifier.New(col.Name)
825-
}
826-
if !col.PrimaryKey && !keyColumns[col.Name] {
827-
// AUTO_INCREMENT col must be a key
828-
return sql.ErrInvalidAutoIncCols.New()
829-
}
830-
if col.Default != nil {
831-
// AUTO_INCREMENT col cannot have default
832-
return sql.ErrInvalidAutoIncCols.New()
833-
}
834-
if seen {
835-
// there can be at most one AUTO_INCREMENT col
836-
return sql.ErrInvalidAutoIncCols.New()
837-
}
838-
seen = true
839-
}
840-
}
841-
}
842-
return nil
843-
}
844-
845818
func schToColMap(sch sql.Schema) map[string]*sql.Column {
846819
colMap := make(map[string]*sql.Column, len(sch))
847820
for _, col := range sch {

0 commit comments

Comments
 (0)