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

schemadiff: reject non-deterministic function in new column's default value #16684

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
10 changes: 10 additions & 0 deletions go/vt/schemadiff/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -487,3 +487,13 @@ type PartitionSpecNonExclusiveError struct {
func (e *PartitionSpecNonExclusiveError) Error() string {
return fmt.Sprintf("ALTER TABLE on %s, may only have a single partition spec change, and other changes are not allowed. Found spec: %s; and change: %s", sqlescape.EscapeID(e.Table), sqlparser.CanonicalString(e.PartitionSpec), e.ConflictingStatement)
}

type NonDeterministicDefaultError struct {
Table string
Column string
Function string
}

func (e *NonDeterministicDefaultError) Error() string {
return fmt.Sprintf("column %s.%s default value uses non-deterministic function: %s", sqlescape.EscapeID(e.Table), sqlescape.EscapeID(e.Column), e.Function)
}
37 changes: 37 additions & 0 deletions go/vt/schemadiff/table.go
Original file line number Diff line number Diff line change
Expand Up @@ -1731,6 +1731,33 @@ func evaluateColumnReordering(t1SharedColumns, t2SharedColumns []*sqlparser.Colu
return minimalColumnReordering
}

// This function looks for a non-deterministic function call in the given expression.
// If recurses into all function arguments.
// The known non-deterministic function we handle are:
// - UUID()
// - RAND()
// - SYSDATE()
func findNoNondeterministicFunction(expr sqlparser.Expr) (foundFunction string) {
_ = sqlparser.Walk(func(node sqlparser.SQLNode) (kontinue bool, err error) {
switch node := node.(type) {
case *sqlparser.CurTimeFuncExpr:
switch node.Name.Lowered() {
case "sysdate":
foundFunction = node.Name.String()
return false, nil
}
case *sqlparser.FuncExpr:
switch node.Name.Lowered() {
case "uuid", "rand":
foundFunction = node.Name.String()
return false, nil
}
}
return true, nil
}, expr)
return foundFunction
}

// Diff compares this table statement with another table statement, and sees what it takes to
// change this table to look like the other table.
// It returns an AlterTable statement if changes are found, or nil if not.
Expand Down Expand Up @@ -1852,6 +1879,16 @@ func (c *CreateTableEntity) diffColumns(alterTable *sqlparser.AlterTable,
addColumn := &sqlparser.AddColumns{
Columns: []*sqlparser.ColumnDefinition{t2Col},
}
// See whether this ADD COLUMN has a non-deterministic default value
if t2Col.Type.Options.Default != nil && !t2Col.Type.Options.DefaultLiteral {
if function := findNoNondeterministicFunction(t2Col.Type.Options.Default); function != "" {
return &NonDeterministicDefaultError{
Table: c.Name(),
Column: t2Col.Name.String(),
Function: function,
}
}
}
if t2ColIndex < expectAppendIndex {
// This column is added somewhere in between existing columns, not appended at end of column list
if t2ColIndex == 0 {
Expand Down
84 changes: 74 additions & 10 deletions go/vt/schemadiff/table_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,16 +28,17 @@ import (

func TestCreateTableDiff(t *testing.T) {
tt := []struct {
name string
from string
to string
fromName string
toName string
diff string
diffs []string
cdiff string
cdiffs []string
errorMsg string
name string
from string
to string
fromName string
toName string
diff string
diffs []string
cdiff string
cdiffs []string
errorMsg string
// hints:
autoinc int
rotation int
fulltext int
Expand All @@ -47,6 +48,7 @@ func TestCreateTableDiff(t *testing.T) {
algorithm int
enumreorder int
subsequent int
//
textdiffs []string
atomicdiffs []string
}{
Expand Down Expand Up @@ -449,6 +451,68 @@ func TestCreateTableDiff(t *testing.T) {
"+ `y` int,",
},
},
{
name: "added column with non deterministic expression, uuid, reject",
from: "create table t1 (id int primary key, a int)",
to: "create table t2 (id int primary key, a int, v varchar(36) not null default (uuid()))",
errorMsg: (&NonDeterministicDefaultError{Table: "t1", Column: "v", Function: "uuid"}).Error(),
},
{
name: "added column with non deterministic expression, UUID, reject",
from: "create table t1 (id int primary key, a int)",
to: "create table t2 (id int primary key, a int, v varchar(36) not null default (UUID()))",
errorMsg: (&NonDeterministicDefaultError{Table: "t1", Column: "v", Function: "UUID"}).Error(),
},
{
name: "added column with non deterministic expression, uuid, spacing, reject",
from: "create table t1 (id int primary key, a int)",
to: "create table t2 (id int primary key, a int, v varchar(36) not null default (uuid ()))",
errorMsg: (&NonDeterministicDefaultError{Table: "t1", Column: "v", Function: "uuid"}).Error(),
},
{
name: "added column with non deterministic expression, uuid, inner, reject",
from: "create table t1 (id int primary key, a int)",
to: "create table t2 (id int primary key, a int, v varchar(36) not null default (left(uuid(),10)))",
errorMsg: (&NonDeterministicDefaultError{Table: "t1", Column: "v", Function: "uuid"}).Error(),
},
{
name: "added column with non deterministic expression, rand, reject",
from: "create table t1 (id int primary key, a int)",
to: "create table t2 (id int primary key, a int, v varchar(36) not null default (2.0 + rand()))",
errorMsg: (&NonDeterministicDefaultError{Table: "t1", Column: "v", Function: "rand"}).Error(),
},
{
name: "added column with non deterministic expression, sysdate, reject",
from: "create table t1 (id int primary key, a int)",
to: "create table t2 (id int primary key, a int, v varchar(36) not null default (sysdate()))",
errorMsg: (&NonDeterministicDefaultError{Table: "t1", Column: "v", Function: "sysdate"}).Error(),
},
{
name: "added column with non deterministic expression, sysdate, reject",
from: "create table t1 (id int primary key, a int)",
to: "create table t2 (id int primary key, a int, v varchar(36) not null default (to_days(sysdate())))",
errorMsg: (&NonDeterministicDefaultError{Table: "t1", Column: "v", Function: "sysdate"}).Error(),
},
{
name: "added column with deterministic expression, now, reject does not apply",
from: "create table t1 (id int primary key, a int)",
to: "create table t2 (id int primary key, a int, v varchar(36) not null default (now()))",
diff: "alter table t1 add column v varchar(36) not null default (now())",
cdiff: "ALTER TABLE `t1` ADD COLUMN `v` varchar(36) NOT NULL DEFAULT (now())",
textdiffs: []string{
"+ `v` varchar(36) NOT NULL DEFAULT (now()),",
},
},
{
name: "added column with deterministic expression, curdate, reject does not apply",
from: "create table t1 (id int primary key, a int)",
to: "create table t2 (id int primary key, a int, v varchar(36) not null default (to_days(curdate())))",
diff: "alter table t1 add column v varchar(36) not null default (to_days(curdate()))",
cdiff: "ALTER TABLE `t1` ADD COLUMN `v` varchar(36) NOT NULL DEFAULT (to_days(curdate()))",
textdiffs: []string{
"+ `v` varchar(36) NOT NULL DEFAULT (to_days(curdate())),",
},
},
// enum
{
name: "expand enum",
Expand Down
Loading