Skip to content

Commit eed4f23

Browse files
elianddbclaude
andcommitted
dolthub/dolt#9481 - Move auto_increment validation to GMS
Move auto_increment constraint validation entirely from Dolt to go-mysql-server for better separation of concerns and MySQL compatibility. Changes: - Add validateAutoIncrementType() function using existing type checking - Fix validation order to prioritize auto_increment before index validation - Unskip comprehensive test coverage for all invalid data types - Fix bugs: YEAR and BIT types were incorrectly allowed - Ensure exact MySQL error message compatibility 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
1 parent 8b0896d commit eed4f23

File tree

2 files changed

+47
-29
lines changed

2 files changed

+47
-29
lines changed

enginetest/queries/script_queries.go

Lines changed: 0 additions & 13 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{},
@@ -8239,7 +8234,6 @@ where
82398234

82408235
// Text Tests
82418236
{
8242-
Skip: true,
82438237
Name: "text with auto_increment",
82448238
Dialect: "mysql",
82458239
SetUpScript: []string{},
@@ -9969,7 +9963,6 @@ where
99699963

99709964
// Bit Tests
99719965
{
9972-
Skip: true,
99739966
Name: "bit with auto_increment",
99749967
Dialect: "mysql",
99759968
SetUpScript: []string{},
@@ -10095,7 +10088,6 @@ where
1009510088

1009610089
// Decimal Tests
1009710090
{
10098-
Skip: true,
1009910091
Name: "decimal with auto_increment",
1010010092
Dialect: "mysql",
1010110093
SetUpScript: []string{},
@@ -10113,7 +10105,6 @@ where
1011310105

1011410106
// Date Tests
1011510107
{
10116-
Skip: true,
1011710108
Name: "date with auto_increment",
1011810109
Dialect: "mysql",
1011910110
SetUpScript: []string{},
@@ -10127,7 +10118,6 @@ where
1012710118

1012810119
// Datetime Tests
1012910120
{
10130-
Skip: true,
1013110121
Name: "datetime with auto_increment",
1013210122
Dialect: "mysql",
1013310123
SetUpScript: []string{},
@@ -10145,7 +10135,6 @@ where
1014510135

1014610136
// Timestamp Tests
1014710137
{
10148-
Skip: true,
1014910138
Name: "timestamp with auto_increment",
1015010139
Dialect: "mysql",
1015110140
SetUpScript: []string{},
@@ -10163,7 +10152,6 @@ where
1016310152

1016410153
// Time Tests
1016510154
{
10166-
Skip: true,
1016710155
Name: "time with auto_increment",
1016810156
Dialect: "mysql",
1016910157
SetUpScript: []string{},
@@ -10181,7 +10169,6 @@ where
1018110169

1018210170
// Year Tests
1018310171
{
10184-
Skip: true,
1018510172
Name: "year with auto_increment",
1018610173
Dialect: "mysql",
1018710174
SetUpScript: []string{},

sql/analyzer/validate_create_table.go

Lines changed: 47 additions & 16 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
}
@@ -787,12 +788,42 @@ func removeInSchema(sch sql.Schema, colName, tableName string) sql.Schema {
787788
return schCopy
788789
}
789790

791+
// validateAutoIncrementType returns true if the given type can be used with AUTO_INCREMENT
792+
func validateAutoIncrementType(t sql.Type) bool {
793+
// Check for invalid types first
794+
if types.IsEnum(t) || types.IsSet(t) || types.IsBit(t) {
795+
return false
796+
}
797+
798+
// Check for text/string types - not allowed (includes TEXT, VARCHAR, CHAR, BLOB, BINARY, etc.)
799+
if types.IsText(t) {
800+
return false
801+
}
802+
803+
// Check for datetime/time types - not allowed
804+
if types.IsTime(t) || types.IsDateType(t) || types.IsDatetimeType(t) || types.IsTimestampType(t) || types.IsYear(t) {
805+
return false
806+
}
807+
808+
// Check for numeric types - only these are potentially allowed
809+
if types.IsNumber(t) {
810+
// DECIMAL is not allowed for auto_increment per MySQL behavior
811+
if types.IsDecimal(t) {
812+
return false
813+
}
814+
return true
815+
}
816+
817+
// Default to false for any other types (JSON, Geometry, etc.)
818+
return false
819+
}
820+
790821
func validateAutoIncrementModify(schema sql.Schema, keyedColumns map[string]bool) error {
791822
seen := false
792823
for _, col := range schema {
793824
if col.AutoIncrement {
794825
// Check if column type is valid for auto_increment
795-
if types.IsEnum(col.Type) {
826+
if !validateAutoIncrementType(col.Type) {
796827
return sql.ErrInvalidColumnSpecifier.New(col.Name)
797828
}
798829
// keyedColumns == nil means they are trying to add auto_increment column
@@ -820,7 +851,7 @@ func validateAutoIncrementAdd(schema sql.Schema, keyColumns map[string]bool) err
820851
if col.AutoIncrement {
821852
{
822853
// Check if column type is valid for auto_increment
823-
if types.IsEnum(col.Type) {
854+
if !validateAutoIncrementType(col.Type) {
824855
return sql.ErrInvalidColumnSpecifier.New(col.Name)
825856
}
826857
if !col.PrimaryKey && !keyColumns[col.Name] {

0 commit comments

Comments
 (0)