Skip to content

Commit

Permalink
schemadiff: new NonDeterministicDefaultStrategy, can reject non-deter…
Browse files Browse the repository at this point in the history
…ministic function in column's default value

Signed-off-by: Shlomi Noach <[email protected]>
  • Loading branch information
shlomi-noach committed Aug 29, 2024
1 parent e89f684 commit 6133fbc
Show file tree
Hide file tree
Showing 4 changed files with 179 additions and 32 deletions.
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: %v", sqlescape.EscapeID(e.Table), sqlescape.EscapeID(e.Column), e.Function)
}
45 changes: 45 additions & 0 deletions go/vt/schemadiff/table.go
Original file line number Diff line number Diff line change
Expand Up @@ -1731,6 +1731,40 @@ 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
default:
// recurse into function argument expressions
for _, exprArg := range node.Exprs {
if foundFunction = findNoNondeterministicFunction(exprArg); foundFunction != "" {
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 +1886,17 @@ func (c *CreateTableEntity) diffColumns(alterTable *sqlparser.AlterTable,
addColumn := &sqlparser.AddColumns{
Columns: []*sqlparser.ColumnDefinition{t2Col},
}
if hints.NonDeterministicDefaultStrategy == NonDeterministicDefaultReject {
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
124 changes: 105 additions & 19 deletions go/vt/schemadiff/table_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,25 +28,28 @@ 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
autoinc int
rotation int
fulltext int
colrename int
constraint int
charset int
algorithm int
enumreorder int
subsequent int
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
colrename int
constraint int
charset int
algorithm int
enumreorder int
subsequent int
nondeterministic int
//
textdiffs []string
atomicdiffs []string
}{
Expand Down Expand Up @@ -449,6 +452,88 @@ func TestCreateTableDiff(t *testing.T) {
"+ `y` int,",
},
},
{
name: "added column with non deterministic expression, allow",
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()))",
diff: "alter table t1 add column v varchar(36) not null default (uuid())",
cdiff: "ALTER TABLE `t1` ADD COLUMN `v` varchar(36) NOT NULL DEFAULT (uuid())",
textdiffs: []string{
"+ `v` varchar(36) NOT NULL DEFAULT (uuid()),",
},
nondeterministic: NonDeterministicDefaultAllow,
},
{
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()))",
nondeterministic: NonDeterministicDefaultReject,
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()))",
nondeterministic: NonDeterministicDefaultReject,
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 ()))",
nondeterministic: NonDeterministicDefaultReject,
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)))",
nondeterministic: NonDeterministicDefaultReject,
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()))",
nondeterministic: NonDeterministicDefaultReject,
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()))",
nondeterministic: NonDeterministicDefaultReject,
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())))",
nondeterministic: NonDeterministicDefaultReject,
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()),",
},
nondeterministic: NonDeterministicDefaultReject,
},
{
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())),",
},
nondeterministic: NonDeterministicDefaultReject,
},
// enum
{
name: "expand enum",
Expand Down Expand Up @@ -2090,6 +2175,7 @@ func TestCreateTableDiff(t *testing.T) {
hints.AlterTableAlgorithmStrategy = ts.algorithm
hints.EnumReorderStrategy = ts.enumreorder
hints.SubsequentDiffStrategy = ts.subsequent
hints.NonDeterministicDefaultStrategy = ts.nondeterministic
alter, err := c.Diff(other, &hints)

require.Equal(t, len(ts.diffs), len(ts.cdiffs))
Expand Down
32 changes: 19 additions & 13 deletions go/vt/schemadiff/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,21 +141,27 @@ const (
SubsequentDiffStrategyReject
)

const (
NonDeterministicDefaultAllow int = iota
NonDeterministicDefaultReject
)

// DiffHints is an assortment of rules for diffing entities
type DiffHints struct {
StrictIndexOrdering bool
AutoIncrementStrategy int
RangeRotationStrategy int
ConstraintNamesStrategy int
ColumnRenameStrategy int
TableRenameStrategy int
FullTextKeyStrategy int
TableCharsetCollateStrategy int
TableQualifierHint int
AlterTableAlgorithmStrategy int
EnumReorderStrategy int
ForeignKeyCheckStrategy int
SubsequentDiffStrategy int
StrictIndexOrdering bool
AutoIncrementStrategy int
RangeRotationStrategy int
ConstraintNamesStrategy int
ColumnRenameStrategy int
TableRenameStrategy int
FullTextKeyStrategy int
TableCharsetCollateStrategy int
TableQualifierHint int
AlterTableAlgorithmStrategy int
EnumReorderStrategy int
ForeignKeyCheckStrategy int
SubsequentDiffStrategy int
NonDeterministicDefaultStrategy int
}

func EmptyDiffHints() *DiffHints {
Expand Down

0 comments on commit 6133fbc

Please sign in to comment.