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.

Sign up for GitHub

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

Jump to bottom

No-op bad alter table auto increment #1930

Merged
merged 10 commits into from Jul 21, 2021
Merged

No-op bad alter table auto increment #1930

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
3fea971
fix
VinaiRachakonda Jul 19, 2021
63f0a13
fix actual modification
VinaiRachakonda Jul 19, 2021
6f0538b
grammar
VinaiRachakonda Jul 20, 2021
0ee36c1
change getAutoIncrement logic
VinaiRachakonda Jul 20, 2021
d2a6f7f
Merge branch 'master' into vinai/1856
VinaiRachakonda Jul 20, 2021
fb5173d
finish up
VinaiRachakonda Jul 20, 2021
cdcabff
cleanup
VinaiRachakonda Jul 20, 2021
e1dd370
remove newline
VinaiRachakonda Jul 20, 2021
1792147
updt gmod
VinaiRachakonda Jul 21, 2021
5b85af4
bump gms and add column
VinaiRachakonda Jul 21, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion go/go.mod
View file
Open in desktop
Expand Up @@ -18,7 +18,7 @@ require (
github.com/denisbrodbeck/machineid v1.0.1
github.com/dolthub/dolt/go/gen/proto/dolt/services/eventsapi v0.0.0-20201005193433-3ee972b1d078
github.com/dolthub/fslock v0.0.2
github.com/dolthub/go-mysql-server v0.10.1-0.20210716214543-4b21301f5a15
github.com/dolthub/go-mysql-server v0.10.1-0.20210721190541-751f2b303897
github.com/dolthub/ishell v0.0.0-20210205014355-16a4ce758446
github.com/dolthub/mmap-go v1.0.4-0.20201107010347-f9f2a9588a66
github.com/dolthub/sqllogictest/go v0.0.0-20201107003712-816f3ae12d81
Expand Down
6 changes: 4 additions & 2 deletions go/go.sum
View file
Open in desktop
Expand Up @@ -142,8 +142,10 @@ github.com/dgrijalva/jwt-go v3.2.0+incompatible/go.mod h1:E3ru+11k8xSBh+hMPgOLZm
github.com/dgryski/go-sip13 v0.0.0-20181026042036-e10d5fee7954/go.mod h1:vAd38F8PWV+bWy6jNmig1y/TA+kYO4g3RSRF0IAv0no=
github.com/dolthub/fslock v0.0.2 h1:8vUh47iKovgrtXNrXVIzsIoWLlspoXg+3nslhUzgKSw=
github.com/dolthub/fslock v0.0.2/go.mod h1:0i7bsNkK+XHwFL3dIsSWeXSV7sykVzzVr6+jq8oeEo0=
github.com/dolthub/go-mysql-server v0.10.1-0.20210716214543-4b21301f5a15 h1:biqjQK89eo6h4gm+XpQeT9ppOUKjo/9qmx9MdOXoHe4=
github.com/dolthub/go-mysql-server v0.10.1-0.20210716214543-4b21301f5a15/go.mod h1:qmYQCdWV7k5y4AbX+DNJjoqNdo4Y2l5UaLbWibEu7d4=
github.com/dolthub/go-mysql-server v0.10.1-0.20210721000736-54546b626b64 h1:NSaotbaqLGw9X/OgKZworSUN8T0mY9aF8KVY7YBV1uM=
github.com/dolthub/go-mysql-server v0.10.1-0.20210721000736-54546b626b64/go.mod h1:qmYQCdWV7k5y4AbX+DNJjoqNdo4Y2l5UaLbWibEu7d4=
github.com/dolthub/go-mysql-server v0.10.1-0.20210721190541-751f2b303897 h1:DhmAJ0pGsnsam3/6MM8n6T0NcSGPLWZXJdzK77QtlS4=
github.com/dolthub/go-mysql-server v0.10.1-0.20210721190541-751f2b303897/go.mod h1:qmYQCdWV7k5y4AbX+DNJjoqNdo4Y2l5UaLbWibEu7d4=
github.com/dolthub/ishell v0.0.0-20210205014355-16a4ce758446 h1:0ol5pj+QlKUKAtqs1LiPM3ZJKs+rHPgLSsMXmhTrCAM=
github.com/dolthub/ishell v0.0.0-20210205014355-16a4ce758446/go.mod h1:dhGBqcCEfK5kuFmeO5+WOx3hqc1k3M29c1oS/R7N4ms=
github.com/dolthub/mmap-go v1.0.4-0.20201107010347-f9f2a9588a66 h1:WRPDbpJWEnPxPmiuOTndT+lUWUeGjx6eoNOK9O4tQQQ=
Expand Down
4 changes: 3 additions & 1 deletion go/libraries/doltcore/schema/alterschema/modifycolumn.go
View file
Open in desktop
Expand Up @@ -147,7 +147,9 @@ func updateTableWithModifiedColumn(ctx context.Context, tbl *doltdb.Table, oldSc
return nil, err
}
var autoVal types.Value
if schema.HasAutoIncrement(newSch) {
// Note: The correct way to add an auto increment value to an existing schema is to you use the
// ALTER TABLE CHANGE COLUMN syntax not ALTER TABLE autoincrement. (see auto_increment.bats)
if schema.HasAutoIncrement(newSch) && schema.HasAutoIncrement(oldSch) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks much better, could you add a comment explaining the alter table case? Also, what happens if the AUTO_INCREMENT is removed from the schema?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new schema would not have auto increment an a nil value would be passed for autoVal

autoVal, err = tbl.GetAutoIncrementValue(ctx)
if err != nil {
return nil, err
Expand Down
23 changes: 23 additions & 0 deletions integration-tests/bats/auto_increment.bats
View file
Open in desktop
Expand Up @@ -557,3 +557,26 @@ SQL
[[ "${lines[4]}" =~ "5,5" ]] || false
[[ "${lines[5]}" =~ "6,6" ]] || false
}

@test "auto_increment: alter table autoincrement on table with no AI key nops" {
dolt sql -q "create table test2(pk int primary key, name varchar(255), type int);"
run dolt sql -q "alter table test2 auto_increment = 2;"
[ "$status" -eq 0 ]

dolt sql -q "insert into test2 values (0, 'john', 0)"
run dolt sql -q "SELECT * from test2" -r csv
[ "$status" -eq 0 ]
[[ "$output" =~ "0,john,0" ]] || false
}

@test "auto_increment: alter table change column works" {
dolt sql -q "create table t(pk int primary key);"
dolt sql -q "ALTER TABLE t CHANGE COLUMN pk pk int NOT NULL AUTO_INCREMENT PRIMARY KEY;"

dolt sql -q 'insert into t values (NULL), (NULL), (NULL)'
run dolt sql -q "SELECT * FROM t" -r csv
[[ "${lines[0]}" =~ "pk" ]] || false
[[ "${lines[1]}" =~ "1" ]] || false
[[ "${lines[2]}" =~ "2" ]] || false
[[ "${lines[3]}" =~ "3" ]] || false
}