Skip to content

Commit

Permalink
Fix TestModifyColumns
Browse files Browse the repository at this point in the history
  • Loading branch information
@fanyang01
fanyang01 committed Dec 18, 2024
1 parent 28f8228 commit 02fad49
Show file tree
Hide file tree
Showing 2 changed files with 65 additions and 52 deletions.
63 changes: 35 additions & 28 deletions catalog/table.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,11 @@ func getPKSchema(ctx *sql.Context, catalogName, dbName, tableName string) (sql.P
defaultValue = sql.NewUnresolvedColumnDefaultValue(decodedComment.Meta.Default)
}

var extra string
if decodedComment.Meta.AutoIncrement {
extra = "auto_increment"
}

column := &sql.Column{
Name: columnInfo.ColumnName,
Type: columnInfo.DataType,
Expand All @@ -142,6 +147,7 @@ func getPKSchema(ctx *sql.Context, catalogName, dbName, tableName string) (sql.P
Default: defaultValue,
AutoIncrement: decodedComment.Meta.AutoIncrement,
Comment: decodedComment.Text,
Extra: extra,
}

schema = append(schema, column)
Expand Down Expand Up @@ -259,19 +265,19 @@ func (t *Table) AddColumn(ctx *sql.Context, column *sql.Column, order *sql.Colum
sqls = append(sqls, `COMMENT ON COLUMN `+FullColumnName(t.db.catalog, t.db.name, t.name, column.Name)+` IS '`+comment.Encode()+`'`)

// Add table comment if it is AUTO_INCREMENT or PRIMARY KEY
extraInfo := t.comment.Meta
changeComment := false
tableInfo := t.comment.Meta
tableInfoChanged := false
if column.AutoIncrement {
extraInfo.Sequence = fullSequenceName
changeComment = true
tableInfo.Sequence = fullSequenceName
tableInfoChanged = true
}
if column.PrimaryKey {
sqls = append(sqls, `ALTER TABLE `+FullTableName(t.db.catalog, t.db.name, t.name)+` ADD PRIMARY KEY (`+QuoteIdentifierANSI(column.Name)+`)`)
extraInfo.PkOrdinals = []int{len(t.schema.Schema)}
changeComment = true
tableInfo.PkOrdinals = []int{len(t.schema.Schema)}
tableInfoChanged = true
}
if changeComment {
comment := NewCommentWithMeta(t.comment.Text, extraInfo)
if tableInfoChanged {
comment := NewCommentWithMeta(t.comment.Text, tableInfo)
sqls = append(sqls, `COMMENT ON TABLE `+FullTableName(t.db.catalog, t.db.name, t.name)+` IS '`+comment.Encode()+`'`)
}

Expand All @@ -282,11 +288,11 @@ func (t *Table) AddColumn(ctx *sql.Context, column *sql.Column, order *sql.Colum

// Update the sequence name only after the column is successfully added.
if column.AutoIncrement {
t.comment.Meta.Sequence = extraInfo.Sequence
t.comment.Meta.Sequence = tableInfo.Sequence
}
// Update the PK ordinals only after the column is successfully added.
if column.PrimaryKey {
t.comment.Meta.PkOrdinals = extraInfo.PkOrdinals
t.comment.Meta.PkOrdinals = tableInfo.PkOrdinals
}
return t.withSchema(ctx)
}
Expand Down Expand Up @@ -377,15 +383,16 @@ func (t *Table) ModifyColumn(ctx *sql.Context, columnName string, column *sql.Co
sqls = append(sqls, baseSQL+` SET DEFAULT `+defaultExpr)
}

extraInfo := t.comment.Meta
extraChanged := false
tableInfo := t.comment.Meta
tableInfoChanged := false

temporary := t.db.catalog == "temp"
var sequenceName, fullSequenceName string

// Handle AUTO_INCREMENT changes
if !oldColumn.AutoIncrement && column.AutoIncrement {
// Adding AUTO_INCREMENT
typ.mysql.AutoIncrement = true
uuid, err := uuid.NewRandom()
if err != nil {
return err
Expand All @@ -405,8 +412,8 @@ func (t *Table) ModifyColumn(ctx *sql.Context, columnName string, column *sql.Co
sqls = append(sqls, baseSQL+` SET DEFAULT nextval('`+fullSequenceName+`')`)

// Update table comment with sequence info
extraInfo.Sequence = fullSequenceName
extraChanged = true
tableInfo.Sequence = fullSequenceName
tableInfoChanged = true

} else if oldColumn.AutoIncrement && !column.AutoIncrement {
// Removing AUTO_INCREMENT
Expand All @@ -416,8 +423,8 @@ func (t *Table) ModifyColumn(ctx *sql.Context, columnName string, column *sql.Co
// sqls = append(sqls, `DROP SEQUENCE IF EXISTS `+t.comment.Meta.Sequence)

// Update table comment to remove sequence info
extraInfo.Sequence = ""
extraChanged = true
tableInfo.Sequence = ""
tableInfoChanged = true
}

// Handle column rename
Expand All @@ -437,15 +444,15 @@ func (t *Table) ModifyColumn(ctx *sql.Context, columnName string, column *sql.Co
sqls = append(sqls, `ALTER TABLE `+FullTableName(t.db.catalog, t.db.name, t.name)+` ADD PRIMARY KEY (`+QuoteIdentifierANSI(column.Name)+`)`)

// Update table comment with PK ordinals
extraInfo.PkOrdinals = []int{oldColumnIndex}
extraChanged = true
tableInfo.PkOrdinals = []int{oldColumnIndex}
tableInfoChanged = true

} else if oldColumn.PrimaryKey && !column.PrimaryKey {
// Remove PRIMARY KEY?
}

if extraChanged {
comment := NewCommentWithMeta(t.comment.Text, extraInfo)
if tableInfoChanged {
comment := NewCommentWithMeta(t.comment.Text, tableInfo)
sqls = append(sqls, `COMMENT ON TABLE `+FullTableName(t.db.catalog, t.db.name, t.name)+` IS '`+comment.Encode()+`'`)
}

Expand Down Expand Up @@ -575,23 +582,23 @@ func (t *Table) CreateIndex(ctx *sql.Context, indexDef sql.IndexDef) error {
}

// Construct the SQL statement for creating the index
var sqlsBuilder strings.Builder
sqlsBuilder.WriteString(fmt.Sprintf(`USE %s; `, FullSchemaName(t.db.catalog, "")))
sqlsBuilder.WriteString(fmt.Sprintf(`CREATE %s INDEX "%s" ON %s (%s)`,
var b strings.Builder
b.WriteString(fmt.Sprintf(`USE %s; `, FullSchemaName(t.db.catalog, "")))
b.WriteString(fmt.Sprintf(`CREATE %s INDEX "%s" ON %s (%s)`,
unique,
EncodeIndexName(t.name, indexDef.Name),
FullTableName("", t.db.name, t.name),
strings.Join(columns, ", ")))

// Add the index comment if provided
if indexDef.Comment != "" {
sqlsBuilder.WriteString(fmt.Sprintf("; COMMENT ON INDEX %s IS '%s'",
b.WriteString(fmt.Sprintf("; COMMENT ON INDEX %s IS '%s'",
FullIndexName(t.db.catalog, t.db.name, EncodeIndexName(t.name, indexDef.Name)),
NewComment[any](indexDef.Comment).Encode()))
}

// Execute the SQL statement to create the index
_, err := adapter.Exec(ctx, sqlsBuilder.String())
_, err := adapter.Exec(ctx, b.String())
if err != nil {
if IsDuckDBIndexAlreadyExistsError(err) {
return sql.ErrDuplicateKey.New(indexDef.Name)
Expand Down Expand Up @@ -726,7 +733,7 @@ func queryColumns(ctx *sql.Context, catalogName, schemaName, tableName string) (
}
defer rows.Close()

var columnsInfo []*ColumnInfo
var columns []*ColumnInfo

for rows.Next() {
var columnName, dataTypes string
Expand All @@ -750,14 +757,14 @@ func queryColumns(ctx *sql.Context, catalogName, schemaName, tableName string) (
ColumnDefault: columnDefault,
Comment: comment,
}
columnsInfo = append(columnsInfo, columnInfo)
columns = append(columns, columnInfo)
}

if err = rows.Err(); err != nil {
return nil, err
}

return columnsInfo, nil
return columns, nil
}

func (t *IndexedTable) LookupPartitions(ctx *sql.Context, lookup sql.IndexLookup) (sql.PartitionIter, error) {
Expand Down
54 changes: 30 additions & 24 deletions main_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1591,48 +1591,48 @@ func TestModifyColumn(t *testing.T) {
{
Name: "column at end with default",
SetUpScript: []string{
"CREATE TABLE mytable_modify_column (i bigint not null, s varchar(20) not null comment 'column s')",
"CREATE TABLE mytable_m1 (i bigint not null, s varchar(20) not null comment 'column s')",
},
Assertions: []queries.ScriptTestAssertion{
{
Query: "ALTER TABLE mytable_modify_column MODIFY COLUMN i bigint NOT NULL COMMENT 'modified'",
Query: "ALTER TABLE mytable_m1 MODIFY COLUMN i bigint NOT NULL COMMENT 'modified'",
Expected: []sql.Row{{types.NewOkResult(0)}},
},
{
Query: "SHOW FULL COLUMNS FROM mytable_modify_column /* 1 */",
Query: "SHOW FULL COLUMNS FROM mytable_m1 /* 1 */",
Expected: []sql.Row{
{"i", "bigint", nil, "NO", "", nil, "", "", "modified"},
{"s", "varchar(20)", "utf8mb4_0900_bin", "NO", "", nil, "", "", "column s"},
},
},
{
Query: "ALTER TABLE mytable_modify_column MODIFY COLUMN i TINYINT NOT NULL COMMENT 'yes'",
Query: "ALTER TABLE mytable_m1 MODIFY COLUMN i TINYINT NOT NULL COMMENT 'yes'",
Expected: []sql.Row{{types.NewOkResult(0)}},
},
{
Query: "SHOW FULL COLUMNS FROM mytable_modify_column /* 2 */",
Query: "SHOW FULL COLUMNS FROM mytable_m1 /* 2 */",
Expected: []sql.Row{
{"i", "tinyint", nil, "NO", "", nil, "", "", "yes"},
{"s", "varchar(20)", "utf8mb4_0900_bin", "NO", "", nil, "", "", "column s"},
},
},
{
Query: "ALTER TABLE mytable_modify_column MODIFY COLUMN i BIGINT NOT NULL COMMENT 'ok'",
Query: "ALTER TABLE mytable_m1 MODIFY COLUMN i BIGINT NOT NULL COMMENT 'ok'",
Expected: []sql.Row{{types.NewOkResult(0)}},
},
{
Query: "SHOW FULL COLUMNS FROM mytable_modify_column /* 3 */",
Query: "SHOW FULL COLUMNS FROM mytable_m1 /* 3 */",
Expected: []sql.Row{
{"i", "bigint", nil, "NO", "", nil, "", "", "ok"},
{"s", "varchar(20)", "utf8mb4_0900_bin", "NO", "", nil, "", "", "column s"},
},
},
{
Query: "ALTER TABLE mytable_modify_column MODIFY COLUMN s VARCHAR(20) NULL COMMENT 'changed'",
Query: "ALTER TABLE mytable_m1 MODIFY COLUMN s VARCHAR(20) NULL COMMENT 'changed'",
Expected: []sql.Row{{types.NewOkResult(0)}},
},
{
Query: "SHOW FULL COLUMNS FROM mytable_modify_column /* 4 */",
Query: "SHOW FULL COLUMNS FROM mytable_m1 /* 4 */",
Expected: []sql.Row{
{"i", "bigint", nil, "NO", "", nil, "", "", "ok"},
{"s", "varchar(20)", "utf8mb4_0900_bin", "YES", "", nil, "", "", "changed"},
Expand All @@ -1641,60 +1641,66 @@ func TestModifyColumn(t *testing.T) {
},
},
{
Name: "auto increment attribute",
SetUpScript: []string{},
Name: "auto increment attribute",
SetUpScript: []string{
"CREATE TABLE mytable_m2 (i bigint not null primary key, s varchar(20) comment 'changed')",
},
Assertions: []queries.ScriptTestAssertion{
{
Query: "ALTER TABLE mytable MODIFY i BIGINT auto_increment",
Query: "ALTER TABLE mytable_m2 MODIFY i BIGINT auto_increment",
Expected: []sql.Row{{types.NewOkResult(0)}},
},
{
Query: "SHOW FULL COLUMNS FROM mytable /* 1 */",
Query: "SHOW FULL COLUMNS FROM mytable_m2 /* 1 */",
Expected: []sql.Row{
{"i", "bigint", nil, "NO", "PRI", nil, "auto_increment", "", ""},
{"s", "varchar(20)", "utf8mb4_0900_bin", "YES", "", nil, "", "", "changed"},
},
},
{
Query: "insert into mytable (s) values ('new row')",
Query: "insert into mytable_m2 (s) values ('new row')",
},
{
Query: "ALTER TABLE mytable add column i2 bigint auto_increment",
Query: "ALTER TABLE mytable_m2 add column i2 bigint auto_increment",
ExpectedErr: sql.ErrInvalidAutoIncCols,
},
{
Query: "alter table mytable add column i2 bigint",
Query: "alter table mytable_m2 add column i2 bigint",
},
{
Query: "ALTER TABLE mytable modify column i2 bigint auto_increment",
Query: "ALTER TABLE mytable_m2 modify column i2 bigint auto_increment",
ExpectedErr: sql.ErrInvalidAutoIncCols,
},
{
Query: "SHOW FULL COLUMNS FROM mytable /* 2 */",
Query: "SHOW FULL COLUMNS FROM mytable_m2 /* 2 */",
Expected: []sql.Row{
{"i", "bigint", nil, "NO", "PRI", nil, "auto_increment", "", ""},
{"s", "varchar(20)", "utf8mb4_0900_bin", "YES", "", nil, "", "", "changed"},
{"i2", "bigint", nil, "YES", "", nil, "", "", ""},
},
},
{
Query: "ALTER TABLE mytable MODIFY COLUMN i BIGINT NOT NULL COMMENT 'ok' FIRST",
Skip: true,
Query: "ALTER TABLE mytable_m2 MODIFY COLUMN i BIGINT NOT NULL COMMENT 'ok' FIRST",
Expected: []sql.Row{{types.NewOkResult(0)}},
},
{
Query: "SHOW FULL COLUMNS FROM mytable /* 3 */",
Skip: true,
Query: "SHOW FULL COLUMNS FROM mytable_m2 /* 3 */",
Expected: []sql.Row{
{"i", "bigint", nil, "NO", "PRI", nil, "", "", "ok"},
{"s", "varchar(20)", "utf8mb4_0900_bin", "YES", "", nil, "", "", "changed"},
{"i2", "bigint", nil, "YES", "", nil, "", "", ""},
},
},
{
Query: "ALTER TABLE mytable MODIFY COLUMN s VARCHAR(20) NULL COMMENT 'changed'",
Skip: true,
Query: "ALTER TABLE mytable_m2 MODIFY COLUMN s VARCHAR(20) NULL COMMENT 'changed'",
Expected: []sql.Row{{types.NewOkResult(0)}},
},
{
Query: "SHOW FULL COLUMNS FROM mytable /* 4 */",
Skip: true,
Query: "SHOW FULL COLUMNS FROM mytable_m2 /* 4 */",
Expected: []sql.Row{
{"i", "bigint", nil, "NO", "PRI", nil, "", "", "ok"},
{"s", "varchar(20)", "utf8mb4_0900_bin", "YES", "", nil, "", "", "changed"},
Expand Down Expand Up @@ -1750,8 +1756,8 @@ func RunModifyColumnTest(t *testing.T, harness enginetest.Harness) {
se.NewConnection(ctx)
}
enginetest.TestQueryWithContext(t, ctx, e, harness, "select database()", []sql.Row{{nil}}, nil, nil, nil)
enginetest.TestQueryWithContext(t, ctx, e, harness, "ALTER TABLE mydb.mytable_modify_column MODIFY COLUMN s VARCHAR(21) NULL COMMENT 'changed again'", []sql.Row{{types.NewOkResult(0)}}, nil, nil, nil)
enginetest.TestQueryWithContext(t, ctx, e, harness, "SHOW FULL COLUMNS FROM mydb.mytable_modify_column", []sql.Row{
enginetest.TestQueryWithContext(t, ctx, e, harness, "ALTER TABLE mydb.mytable_m1 MODIFY COLUMN s VARCHAR(21) NULL COMMENT 'changed again'", []sql.Row{{types.NewOkResult(0)}}, nil, nil, nil)
enginetest.TestQueryWithContext(t, ctx, e, harness, "SHOW FULL COLUMNS FROM mydb.mytable_m1", []sql.Row{
{"i", "bigint", nil, "NO", "", nil, "", "", "ok"},
{"s", "varchar(21)", "utf8mb4_0900_bin", "YES", "", nil, "", "", "changed again"},
}, nil, nil, nil)
Expand Down

0 comments on commit 02fad49

Please sign in to comment.