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 auto_increment on non-primary key columns #936

Merged
merged 21 commits into from
Apr 8, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 81 additions & 0 deletions enginetest/enginetests.go
Original file line number Diff line number Diff line change
Expand Up @@ -2207,6 +2207,87 @@ func TestCreateTable(t *testing.T, harness Harness) {
}
})
//TODO: Implement "CREATE TABLE otherDb.tableName"
t.Run("CREATE TABLE with auto_increment on unique column", func(t *testing.T) {
TestQuery(t, harness, e, "CREATE TABLE auto_t1 (i int primary key, j int auto_increment unique)",
[]sql.Row(nil), nil, nil)

db, err := e.Analyzer.Catalog.Database(ctx, "mydb")
require.NoError(t, err)

ctx := NewContext(harness)
testTable, ok, err := db.GetTableInsensitive(ctx, "auto_t1")
require.NoError(t, err)
require.True(t, ok)

s := sql.Schema{
{Name: "i", Type: sql.Int32, Nullable: false, Source: "auto_t1", PrimaryKey: true},
{Name: "j", Type: sql.Int32, Nullable: true, Source: "auto_t1", AutoIncrement: true, Extra: "auto_increment"},
}

require.Equal(t, s, testTable.Schema())
})

t.Run("CREATE TABLE with auto_increment on index column", func(t *testing.T) {
TestQuery(t, harness, e, "CREATE TABLE auto_t2 (i int primary key, j int auto_increment, index (j))",
[]sql.Row(nil), nil, nil)

db, err := e.Analyzer.Catalog.Database(ctx, "mydb")
require.NoError(t, err)

ctx := NewContext(harness)
testTable, ok, err := db.GetTableInsensitive(ctx, "auto_t2")
require.NoError(t, err)
require.True(t, ok)

s := sql.Schema{
{Name: "i", Type: sql.Int32, Nullable: false, Source: "auto_t2", PrimaryKey: true},
{Name: "j", Type: sql.Int32, Nullable: true, Source: "auto_t2", AutoIncrement: true, Extra: "auto_increment"},
}

require.Equal(t, s, testTable.Schema())
})

t.Run("CREATE TABLE with auto_increment on multiple unique columns", func(t *testing.T) {
TestQuery(t, harness, e, "CREATE TABLE auto_t3 (i int primary key, j int auto_increment, k int, unique(j,k))",
[]sql.Row(nil), nil, nil)

db, err := e.Analyzer.Catalog.Database(ctx, "mydb")
require.NoError(t, err)

ctx := NewContext(harness)
testTable, ok, err := db.GetTableInsensitive(ctx, "auto_t3")
require.NoError(t, err)
require.True(t, ok)

s := sql.Schema{
{Name: "i", Type: sql.Int32, Nullable: false, Source: "auto_t3", PrimaryKey: true},
{Name: "j", Type: sql.Int32, Nullable: true, Source: "auto_t3", AutoIncrement: true, Extra: "auto_increment"},
{Name: "k", Type: sql.Int32, Nullable: true, Source: "auto_t3"},
}

require.Equal(t, s, testTable.Schema())
})

t.Run("CREATE TABLE with auto_increment on index column", func(t *testing.T) {
TestQuery(t, harness, e, "CREATE TABLE auto_t4 (i int primary key, j int auto_increment, k int, index (j,k))",
[]sql.Row(nil), nil, nil)

db, err := e.Analyzer.Catalog.Database(ctx, "mydb")
require.NoError(t, err)

ctx := NewContext(harness)
testTable, ok, err := db.GetTableInsensitive(ctx, "auto_t4")
require.NoError(t, err)
require.True(t, ok)

s := sql.Schema{
{Name: "i", Type: sql.Int32, Nullable: false, Source: "auto_t4", PrimaryKey: true},
{Name: "j", Type: sql.Int32, Nullable: true, Source: "auto_t4", AutoIncrement: true, Extra: "auto_increment"},
{Name: "k", Type: sql.Int32, Nullable: true, Source: "auto_t4"},
}

require.Equal(t, s, testTable.Schema())
})
}

func TestDropTable(t *testing.T, harness Harness) {
Expand Down
60 changes: 60 additions & 0 deletions enginetest/insert_queries.go
Original file line number Diff line number Diff line change
Expand Up @@ -664,6 +664,66 @@ var InsertScripts = []ScriptTest{
},
},
},
{
Name: "insert into auto_increment unique key column",
SetUpScript: []string{
"create table auto (pk int primary key, npk int unique auto_increment)",
"insert into auto (pk) values (10), (20), (30)",
},
Assertions: []ScriptTestAssertion{
{
Query: "select * from auto order by 1",
Expected: []sql.Row{
{10, 1}, {20, 2}, {30, 3},
},
},
},
},
{
Name: "insert into auto_increment with multiple unique key columns",
SetUpScript: []string{
"create table auto (pk int primary key, npk1 int auto_increment, npk2 int, unique(npk1, npk2))",
"insert into auto (pk) values (10), (20), (30)",
},
Assertions: []ScriptTestAssertion{
{
Query: "select * from auto order by 1",
Expected: []sql.Row{
{10, 1, nil}, {20, 2, nil}, {30, 3, nil},
},
},
},
},
{
Name: "insert into auto_increment key/index column",
SetUpScript: []string{
"create table auto_no_primary (i int auto_increment, index(i))",
"insert into auto_no_primary (i) values (0), (0), (0)",
},
Assertions: []ScriptTestAssertion{
{
Query: "select * from auto_no_primary order by 1",
Expected: []sql.Row{
{1}, {2}, {3},
},
},
},
},
{
Name: "insert into auto_increment with multiple key/index columns",
SetUpScript: []string{
"create table auto_no_primary (i int auto_increment, j int, index(i))",
"insert into auto_no_primary (i) values (0), (0), (0)",
},
Assertions: []ScriptTestAssertion{
{
Query: "select * from auto_no_primary order by 1",
Expected: []sql.Row{
{1, nil}, {2, nil}, {3, nil},
},
},
},
},
{
Name: "auto increment table handles deletes",
SetUpScript: []string{
Expand Down
83 changes: 83 additions & 0 deletions enginetest/script_queries.go
Original file line number Diff line number Diff line change
Expand Up @@ -1016,6 +1016,52 @@ var ScriptTests = []ScriptTest{
},
},
},
{
Name: "ALTER TABLE MODIFY column with UNIQUE KEY",
SetUpScript: []string{
"CREATE table test (pk int primary key, uk int unique)",
"ALTER TABLE `test` MODIFY column uk int auto_increment",
},
Assertions: []ScriptTestAssertion{
{
Query: "describe test",
Expected: []sql.Row{
{"pk", "int", "NO", "PRI", "", ""},
{"uk", "int", "YES", "UNI", "", "auto_increment"},
},
},
},
},
{
Name: "ALTER TABLE MODIFY column with KEY",
SetUpScript: []string{
"CREATE table test (pk int primary key, mk int, index (mk))",
"ALTER TABLE `test` MODIFY column mk int auto_increment",
},
Assertions: []ScriptTestAssertion{
{
Query: "describe test",
Expected: []sql.Row{
{"pk", "int", "NO", "PRI", "", ""},
{"mk", "int", "YES", "MUL", "", "auto_increment"},
},
},
},
},
{
Name: "ALTER TABLE AUTO INCREMENT no-ops on table with no original auto increment key",
SetUpScript: []string{
"CREATE table test (pk int primary key)",
"ALTER TABLE `test` auto_increment = 2;",
"INSERT INTO test VALUES (1)",
},
Assertions: []ScriptTestAssertion{
{
Query: "SELECT * FROM test",
Expected: []sql.Row{{1}},
},
},
},
{
Name: "Run through some complex queries with DISTINCT and aggregates",
SetUpScript: []string{
Expand Down Expand Up @@ -2139,3 +2185,40 @@ var CreateCheckConstraintsScripts = []ScriptTest{
},
},
}

var BrokenScriptTests = []ScriptTest{
{
Name: "ALTER TABLE MODIFY column with multiple UNIQUE KEYS",
SetUpScript: []string{
"CREATE table test (pk int primary key, uk1 int, uk2 int, unique(uk1, uk2))",
"ALTER TABLE `test` MODIFY column uk1 int auto_increment",
},
Assertions: []ScriptTestAssertion{
{
Query: "describe test",
Expected: []sql.Row{
{"pk", "int", "NO", "PRI", "", ""},
{"uk1", "int", "YES", "UNI", "", "auto_increment"},
{"uk1", "int", "YES", "UNI", "", "auto_increment"},
},
},
},
},
{
Name: "ALTER TABLE MODIFY column with multiple KEYS",
SetUpScript: []string{
"CREATE table test (pk int primary key, mk1 int, mk2 int, index(uk1, uk2))",
"ALTER TABLE `test` MODIFY column mk1 int auto_increment",
},
Assertions: []ScriptTestAssertion{
{
Query: "describe test",
Expected: []sql.Row{
{"pk", "int", "NO", "PRI", "", ""},
{"mk1", "int", "YES", "MUL", "", "auto_increment"},
{"mk1", "int", "YES", "MUL", "", "auto_increment"},
},
},
},
},
}
52 changes: 43 additions & 9 deletions sql/analyzer/validate_create_table.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,21 @@ func validateCreateTable(ctx *sql.Context, a *Analyzer, n sql.Node, scope *Scope
return n, transform.SameTree, nil
}

err := validateAutoIncrement(ct.CreateSchema.Schema)
err := validateIndexes(ct.TableSpec())
if err != nil {
return nil, transform.SameTree, err
}

err = validateIndexes(ct.TableSpec())
// passed validateIndexes, so they all must be valid indexes
// extract map of columns that have indexes defined over them
keyedColumns := make(map[string]bool)
for _, index := range ct.TableSpec().IdxDefs {
for _, col := range index.Columns {
keyedColumns[col.Name] = true
}
}

err = validateAutoIncrement(ct.CreateSchema.Schema, keyedColumns)
if err != nil {
return nil, transform.SameTree, err
}
Expand All @@ -51,11 +60,13 @@ func validateAlterColumn(ctx *sql.Context, a *Analyzer, n sql.Node, scope *Scope

var sch sql.Schema
var indexes []string
var keyedColumns map[string]bool
var err error
transform.Inspect(n, func(n sql.Node) bool {
switch n := n.(type) {
case *plan.ModifyColumn:
sch = n.Table.Schema()
keyedColumns, err = getTableIndexColumns(ctx, n.Table)
return false
case *plan.RenameColumn:
sch = n.Table.Schema()
Expand Down Expand Up @@ -96,10 +107,12 @@ func validateAlterColumn(ctx *sql.Context, a *Analyzer, n sql.Node, scope *Scope
transform.Inspect(n, func(n sql.Node) bool {
switch n := n.(type) {
case *plan.ModifyColumn:
sch, err = validateModifyColumn(initialSch, sch, n)
sch, err = validateModifyColumn(initialSch, sch, n, keyedColumns)
case *plan.RenameColumn:
sch, err = validateRenameColumn(initialSch, sch, n)
case *plan.AddColumn:
// TODO: can't `alter table add column j int unique auto_increment` as it ignores unique
// TODO: when above works, need to make sure unique index exists first then do what we did for modify
sch, err = validateAddColumn(initialSch, sch, n)
case *plan.DropColumn:
sch, err = validateDropColumn(initialSch, sch, n)
Expand Down Expand Up @@ -177,15 +190,15 @@ func validateAddColumn(initialSch sql.Schema, schema sql.Schema, ac *plan.AddCol
newSch := append(schema, newCol)

// TODO: more validation possible to do here
err := validateAutoIncrement(newSch)
err := validateAutoIncrement(newSch, nil)
if err != nil {
return nil, err
}

return newSch, nil
}

func validateModifyColumn(intialSch sql.Schema, schema sql.Schema, mc *plan.ModifyColumn) (sql.Schema, error) {
func validateModifyColumn(intialSch sql.Schema, schema sql.Schema, mc *plan.ModifyColumn, keyedColumns map[string]bool) (sql.Schema, error) {
table := mc.Table
nameable := table.(sql.Nameable)

Expand All @@ -196,7 +209,7 @@ func validateModifyColumn(intialSch sql.Schema, schema sql.Schema, mc *plan.Modi

newSch := replaceInSchema(schema, mc.NewColumn(), nameable.Name())

err := validateAutoIncrement(newSch)
err := validateAutoIncrement(newSch, keyedColumns)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -361,12 +374,13 @@ func removeInSchema(sch sql.Schema, colName, tableName string) sql.Schema {
return schCopy
}

func validateAutoIncrement(schema sql.Schema) error {
func validateAutoIncrement(schema sql.Schema, keyedColumns map[string]bool) error {
seen := false
for _, col := range schema {
if col.AutoIncrement {
if !col.PrimaryKey {
// AUTO_INCREMENT col must be a pk
// keyedColumns == nil means they are trying to add auto_increment column
if !col.PrimaryKey && !keyedColumns[col.Name] {
// AUTO_INCREMENT col must be a key
return sql.ErrInvalidAutoIncCols.New()
}
if col.Default != nil {
Expand Down Expand Up @@ -400,6 +414,26 @@ func validateIndexes(tableSpec *plan.TableSpec) error {
return nil
}

// getTableIndexColumns returns the columns over which indexes are defined
func getTableIndexColumns(ctx *sql.Context, table sql.Node) (map[string]bool, error) {
ia, err := newIndexAnalyzerForNode(ctx, table)
if err != nil {
return nil, err
}

keyedColumns := make(map[string]bool)
indexes := ia.IndexesByTable(ctx, ctx.GetCurrentDatabase(), getTableName(table))
for _, index := range indexes {
for _, expr := range index.Expressions() {
if col := plan.GetColumnFromIndexExpr(expr, getTable(table)); col != nil {
keyedColumns[col.Name] = true
}
}
}

return keyedColumns, nil
}

// getTableIndexNames returns the names of indexes associated with a table.
func getTableIndexNames(ctx *sql.Context, a *Analyzer, table sql.Node) ([]string, error) {
ia, err := newIndexAnalyzerForNode(ctx, table)
Expand Down