From 6133fbc0e6220199fcd94e4deb942c263f4c3665 Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Thu, 29 Aug 2024 16:15:27 +0300 Subject: [PATCH] schemadiff: new NonDeterministicDefaultStrategy, can reject non-deterministic function in column's default value Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/vt/schemadiff/errors.go | 10 +++ go/vt/schemadiff/table.go | 45 ++++++++++++ go/vt/schemadiff/table_test.go | 124 ++++++++++++++++++++++++++++----- go/vt/schemadiff/types.go | 32 +++++---- 4 files changed, 179 insertions(+), 32 deletions(-) diff --git a/go/vt/schemadiff/errors.go b/go/vt/schemadiff/errors.go index a941c406be0..d25a21c6c0f 100644 --- a/go/vt/schemadiff/errors.go +++ b/go/vt/schemadiff/errors.go @@ -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) +} diff --git a/go/vt/schemadiff/table.go b/go/vt/schemadiff/table.go index c326b2763b3..8cd0169d261 100644 --- a/go/vt/schemadiff/table.go +++ b/go/vt/schemadiff/table.go @@ -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. @@ -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 { diff --git a/go/vt/schemadiff/table_test.go b/go/vt/schemadiff/table_test.go index 389e55f447c..71e3a72853e 100644 --- a/go/vt/schemadiff/table_test.go +++ b/go/vt/schemadiff/table_test.go @@ -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 }{ @@ -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", @@ -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)) diff --git a/go/vt/schemadiff/types.go b/go/vt/schemadiff/types.go index cb6bc09df85..3e6a86446ff 100644 --- a/go/vt/schemadiff/types.go +++ b/go/vt/schemadiff/types.go @@ -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 {