Skip to content

Commit

Permalink
schemadiff: Introduce environment
Browse files Browse the repository at this point in the history
We already had the parser that needed to be injected, but in order to
properly handle MySQL 5.7 we also need to inject collations
configuration. Instead of adding more arguments, we introduce the
schemadiff environment struct and use that to inject the things like the
current parser and current collation environment and default collation.

Also need to refactor the rest that ends up using schemadiff to inject
it similarly.

The core goal here is that the sidecar can properly inject the right
collation as well so we can unblock fixing the case sensitivity there.

Signed-off-by: Dirkjan Bussink <[email protected]>
  • Loading branch information
dbussink committed Jan 11, 2024
1 parent 261c54a commit 5241c23
Show file tree
Hide file tree
Showing 48 changed files with 488 additions and 352 deletions.
2 changes: 1 addition & 1 deletion go/cmd/vtcombo/cli/plugin_grpcvtctldserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import (
func init() {
servenv.OnRun(func() {
if servenv.GRPCCheckServiceMap("vtctld") {
grpcvtctldserver.StartServer(servenv.GRPCServer, ts, parser)
grpcvtctldserver.StartServer(servenv.GRPCServer, ts, collationEnv, parser)
}
})
}
5 changes: 3 additions & 2 deletions go/cmd/vtctl/vtctl.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,8 @@ func main() {
ctx, cancel := context.WithTimeout(context.Background(), waitTime)
installSignalHandlers(cancel)

collationEnv := collations.NewEnvironment(servenv.MySQLServerVersion())

parser, err := sqlparser.New(sqlparser.Options{
MySQLServerVersion: servenv.MySQLServerVersion(),
TruncateUILen: servenv.TruncateUILen,
Expand Down Expand Up @@ -164,7 +166,7 @@ func main() {
// New behavior. Strip off the prefix, and set things up to run through
// the vtctldclient command tree, using the localvtctldclient (in-process)
// client.
vtctld := grpcvtctldserver.NewVtctldServer(ts, parser)
vtctld := grpcvtctldserver.NewVtctldServer(ts, collationEnv, parser)
localvtctldclient.SetServer(vtctld)
command.VtctldClientProtocol = "local"

Expand All @@ -180,7 +182,6 @@ func main() {
fallthrough
default:
log.Warningf("WARNING: vtctl should only be used for VDiff v1 workflows. Please use VDiff v2 and consider using vtctldclient for all other commands.")
collationEnv := collations.NewEnvironment(servenv.MySQLServerVersion())
wr := wrangler.New(logutil.NewConsoleLogger(), ts, tmclient.NewTabletManagerClient(), collationEnv, parser)

if args[0] == "--" {
Expand Down
2 changes: 1 addition & 1 deletion go/cmd/vtctld/cli/plugin_grpcvtctldserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import (
func init() {
servenv.OnRun(func() {
if servenv.GRPCCheckServiceMap("vtctld") {
grpcvtctldserver.StartServer(servenv.GRPCServer, ts, parser)
grpcvtctldserver.StartServer(servenv.GRPCServer, ts, collationEnv, parser)
}
})
}
7 changes: 5 additions & 2 deletions go/cmd/vtctldclient/command/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (

"github.com/spf13/cobra"

"vitess.io/vitess/go/mysql/collations"
"vitess.io/vitess/go/trace"
"vitess.io/vitess/go/vt/log"
"vitess.io/vitess/go/vt/logutil"
Expand Down Expand Up @@ -82,7 +83,8 @@ var (
actionTimeout time.Duration
compactOutput bool

parser *sqlparser.Parser
collationEnv *collations.Environment
parser *sqlparser.Parser

topoOptions = struct {
implementation string
Expand Down Expand Up @@ -212,7 +214,7 @@ func getClientForCommand(cmd *cobra.Command) (vtctldclient.VtctldClient, error)
return nil
})
})
vtctld := grpcvtctldserver.NewVtctldServer(ts, parser)
vtctld := grpcvtctldserver.NewVtctldServer(ts, collationEnv, parser)
localvtctldclient.SetServer(vtctld)
VtctldClientProtocol = "local"
server = ""
Expand All @@ -230,6 +232,7 @@ func init() {
Root.PersistentFlags().StringVar(&topoOptions.globalRoot, "topo-global-root", topoOptions.globalRoot, "the path of the global topology data in the global topology server")
vreplcommon.RegisterCommands(Root)

collationEnv = collations.NewEnvironment(servenv.MySQLServerVersion())
var err error
parser, err = sqlparser.New(sqlparser.Options{
MySQLServerVersion: servenv.MySQLServerVersion(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (

"vitess.io/vitess/go/cmd/vtctldclient/command"
"vitess.io/vitess/go/cmd/vtctldclient/command/vreplication/common"
"vitess.io/vitess/go/mysql/collations"
"vitess.io/vitess/go/vt/sqlparser"
"vitess.io/vitess/go/vt/topo"
"vitess.io/vitess/go/vt/topo/memorytopo"
Expand Down Expand Up @@ -145,7 +146,7 @@ func SetupLocalVtctldClient(t *testing.T, ctx context.Context, cells ...string)
tmclient.RegisterTabletManagerClientFactory("grpc", func() tmclient.TabletManagerClient {
return nil
})
vtctld := grpcvtctldserver.NewVtctldServer(ts, sqlparser.NewTestParser())
vtctld := grpcvtctldserver.NewVtctldServer(ts, collations.MySQL8(), sqlparser.NewTestParser())
localvtctldclient.SetServer(vtctld)
command.VtctldClientProtocol = "local"
client, err := vtctldclient.New(command.VtctldClientProtocol, "")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"sync"
"testing"

"vitess.io/vitess/go/mysql/collations"
"vitess.io/vitess/go/sqltypes"
"vitess.io/vitess/go/vt/grpcclient"
"vitess.io/vitess/go/vt/sqlparser"
Expand Down Expand Up @@ -84,7 +85,7 @@ func newTestVDiffEnv(t testing.TB, ctx context.Context, sourceShards, targetShar
tabletType: topodatapb.TabletType_REPLICA,
tmc: newTestVDiffTMClient(),
}
env.ws = workflow.NewServer(env.topoServ, env.tmc, sqlparser.NewTestParser())
env.ws = workflow.NewServer(env.topoServ, env.tmc, collations.MySQL8(), sqlparser.NewTestParser())
env.tmc.testEnv = env

// Generate a unique dialer name.
Expand Down
20 changes: 10 additions & 10 deletions go/test/endtoend/schemadiff/vrepl/schemadiff_vrepl_suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -345,18 +345,19 @@ func ignoreAutoIncrement(t *testing.T, createTable string) string {

func validateDiff(t *testing.T, fromCreateTable string, toCreateTable string, allowSchemadiffNormalization bool, hints *schemadiff.DiffHints) {
// turn the "from" and "to" create statement strings (which we just read via SHOW CREATE TABLE into sqlparser.CreateTable statement)
fromStmt, err := sqlparser.NewTestParser().ParseStrictDDL(fromCreateTable)
env := schemadiff.NewTestEnv()
fromStmt, err := env.Parser.ParseStrictDDL(fromCreateTable)
require.NoError(t, err)
fromCreateTableStatement, ok := fromStmt.(*sqlparser.CreateTable)
require.True(t, ok)

toStmt, err := sqlparser.NewTestParser().ParseStrictDDL(toCreateTable)
toStmt, err := env.Parser.ParseStrictDDL(toCreateTable)
require.NoError(t, err)
toCreateTableStatement, ok := toStmt.(*sqlparser.CreateTable)
require.True(t, ok)

// The actual diff logic here!
diff, err := schemadiff.DiffTables(fromCreateTableStatement, toCreateTableStatement, hints)
diff, err := schemadiff.DiffTables(env, fromCreateTableStatement, toCreateTableStatement, hints)
assert.NoError(t, err)

// The diff can be empty or there can be an actual ALTER TABLE statement
Expand Down Expand Up @@ -385,7 +386,6 @@ func validateDiff(t *testing.T, fromCreateTable string, toCreateTable string, al
// the table generated by the test's own ALTER statement?

// But wait, there's caveats.

if toCreateTable != resultCreateTable {
// schemadiff's ALTER statement can normalize away CHARACTER SET and COLLATION definitions:
// when altering a column's CHARTSET&COLLATION into the table's values, schemadiff just strips the
Expand All @@ -394,20 +394,20 @@ func validateDiff(t *testing.T, fromCreateTable string, toCreateTable string, al
// structure is identical. And so we accept that there can be a normalization issue.
if allowSchemadiffNormalization {
{
stmt, err := sqlparser.NewTestParser().ParseStrictDDL(toCreateTable)
stmt, err := env.Parser.ParseStrictDDL(toCreateTable)
require.NoError(t, err)
createTableStatement, ok := stmt.(*sqlparser.CreateTable)
require.True(t, ok)
c, err := schemadiff.NewCreateTableEntity(createTableStatement)
c, err := schemadiff.NewCreateTableEntity(env, createTableStatement)
require.NoError(t, err)
toCreateTable = c.Create().CanonicalStatementString()
}
{
stmt, err := sqlparser.NewTestParser().ParseStrictDDL(resultCreateTable)
stmt, err := env.Parser.ParseStrictDDL(resultCreateTable)
require.NoError(t, err)
createTableStatement, ok := stmt.(*sqlparser.CreateTable)
require.True(t, ok)
c, err := schemadiff.NewCreateTableEntity(createTableStatement)
c, err := schemadiff.NewCreateTableEntity(env, createTableStatement)
require.NoError(t, err)
resultCreateTable = c.Create().CanonicalStatementString()
}
Expand All @@ -418,12 +418,12 @@ func validateDiff(t *testing.T, fromCreateTable string, toCreateTable string, al
assert.Equal(t, toCreateTable, resultCreateTable, "mismatched table structure. ALTER query was: %s", diffedAlterQuery)

// Also, let's see that our diff agrees there's no change:
resultStmt, err := sqlparser.NewTestParser().ParseStrictDDL(resultCreateTable)
resultStmt, err := env.Parser.ParseStrictDDL(resultCreateTable)
require.NoError(t, err)
resultCreateTableStatement, ok := resultStmt.(*sqlparser.CreateTable)
require.True(t, ok)

resultDiff, err := schemadiff.DiffTables(toCreateTableStatement, resultCreateTableStatement, hints)
resultDiff, err := schemadiff.DiffTables(env, toCreateTableStatement, resultCreateTableStatement, hints)
assert.NoError(t, err)
assert.Nil(t, resultDiff)
}
Expand Down
8 changes: 2 additions & 6 deletions go/vt/schemadiff/column.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,9 +109,7 @@ func (c *ColumnDefinitionEntity) ColumnDiff(
defer func() {
c.columnDefinition.Type.Options.Collate = ""
}()
if c.columnDefinition.Type.Options.Collate = t1cc.collate; c.columnDefinition.Type.Options.Collate == "" {
c.columnDefinition.Type.Options.Collate = defaultCharsetCollation(t1cc.charset)
}
c.columnDefinition.Type.Options.Collate = t1cc.collate
}
}
if other.columnDefinition.Type.Charset.Name == "" {
Expand All @@ -123,9 +121,7 @@ func (c *ColumnDefinitionEntity) ColumnDiff(
defer func() {
other.columnDefinition.Type.Options.Collate = ""
}()
if other.columnDefinition.Type.Options.Collate = t2cc.collate; other.columnDefinition.Type.Options.Collate == "" {
other.columnDefinition.Type.Options.Collate = defaultCharsetCollation(t2cc.charset)
}
other.columnDefinition.Type.Options.Collate = t2cc.collate
}
}
}
Expand Down
48 changes: 24 additions & 24 deletions go/vt/schemadiff/diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,11 @@ func AllSubsequent(diff EntityDiff) (diffs []EntityDiff) {
// DiffCreateTablesQueries compares two `CREATE TABLE ...` queries (in string form) and returns the diff from table1 to table2.
// Either or both of the queries can be empty. Based on this, the diff could be
// nil, CreateTable, DropTable or AlterTable
func DiffCreateTablesQueries(query1 string, query2 string, hints *DiffHints, parser *sqlparser.Parser) (EntityDiff, error) {
func DiffCreateTablesQueries(env *Environment, query1 string, query2 string, hints *DiffHints) (EntityDiff, error) {
var fromCreateTable *sqlparser.CreateTable
var ok bool
if query1 != "" {
stmt, err := parser.ParseStrictDDL(query1)
stmt, err := env.Parser.ParseStrictDDL(query1)
if err != nil {
return nil, err
}
Expand All @@ -42,7 +42,7 @@ func DiffCreateTablesQueries(query1 string, query2 string, hints *DiffHints, par
}
var toCreateTable *sqlparser.CreateTable
if query2 != "" {
stmt, err := parser.ParseStrictDDL(query2)
stmt, err := env.Parser.ParseStrictDDL(query2)
if err != nil {
return nil, err
}
Expand All @@ -51,34 +51,34 @@ func DiffCreateTablesQueries(query1 string, query2 string, hints *DiffHints, par
return nil, ErrExpectedCreateTable
}
}
return DiffTables(fromCreateTable, toCreateTable, hints)
return DiffTables(env, fromCreateTable, toCreateTable, hints)
}

// DiffTables compares two tables and returns the diff from table1 to table2.
// Either or both of the CreateTable statements can be nil. Based on this, the diff could be
// nil, CreateTable, DropTable or AlterTable
func DiffTables(create1 *sqlparser.CreateTable, create2 *sqlparser.CreateTable, hints *DiffHints) (EntityDiff, error) {
func DiffTables(env *Environment, create1 *sqlparser.CreateTable, create2 *sqlparser.CreateTable, hints *DiffHints) (EntityDiff, error) {
switch {
case create1 == nil && create2 == nil:
return nil, nil
case create1 == nil:
c2, err := NewCreateTableEntity(create2)
c2, err := NewCreateTableEntity(env, create2)
if err != nil {
return nil, err
}
return c2.Create(), nil
case create2 == nil:
c1, err := NewCreateTableEntity(create1)
c1, err := NewCreateTableEntity(env, create1)
if err != nil {
return nil, err
}
return c1.Drop(), nil
default:
c1, err := NewCreateTableEntity(create1)
c1, err := NewCreateTableEntity(env, create1)
if err != nil {
return nil, err
}
c2, err := NewCreateTableEntity(create2)
c2, err := NewCreateTableEntity(env, create2)
if err != nil {
return nil, err
}
Expand All @@ -89,11 +89,11 @@ func DiffTables(create1 *sqlparser.CreateTable, create2 *sqlparser.CreateTable,
// DiffCreateViewsQueries compares two `CREATE TABLE ...` queries (in string form) and returns the diff from table1 to table2.
// Either or both of the queries can be empty. Based on this, the diff could be
// nil, CreateView, DropView or AlterView
func DiffCreateViewsQueries(query1 string, query2 string, hints *DiffHints, parser *sqlparser.Parser) (EntityDiff, error) {
func DiffCreateViewsQueries(env *Environment, query1 string, query2 string, hints *DiffHints) (EntityDiff, error) {
var fromCreateView *sqlparser.CreateView
var ok bool
if query1 != "" {
stmt, err := parser.ParseStrictDDL(query1)
stmt, err := env.Parser.ParseStrictDDL(query1)
if err != nil {
return nil, err
}
Expand All @@ -104,7 +104,7 @@ func DiffCreateViewsQueries(query1 string, query2 string, hints *DiffHints, pars
}
var toCreateView *sqlparser.CreateView
if query2 != "" {
stmt, err := parser.ParseStrictDDL(query2)
stmt, err := env.Parser.ParseStrictDDL(query2)
if err != nil {
return nil, err
}
Expand All @@ -113,34 +113,34 @@ func DiffCreateViewsQueries(query1 string, query2 string, hints *DiffHints, pars
return nil, ErrExpectedCreateView
}
}
return DiffViews(fromCreateView, toCreateView, hints)
return DiffViews(env, fromCreateView, toCreateView, hints)
}

// DiffViews compares two views and returns the diff from view1 to view2
// Either or both of the CreateView statements can be nil. Based on this, the diff could be
// nil, CreateView, DropView or AlterView
func DiffViews(create1 *sqlparser.CreateView, create2 *sqlparser.CreateView, hints *DiffHints) (EntityDiff, error) {
func DiffViews(env *Environment, create1 *sqlparser.CreateView, create2 *sqlparser.CreateView, hints *DiffHints) (EntityDiff, error) {
switch {
case create1 == nil && create2 == nil:
return nil, nil
case create1 == nil:
c2, err := NewCreateViewEntity(create2)
c2, err := NewCreateViewEntity(env, create2)
if err != nil {
return nil, err
}
return c2.Create(), nil
case create2 == nil:
c1, err := NewCreateViewEntity(create1)
c1, err := NewCreateViewEntity(env, create1)
if err != nil {
return nil, err
}
return c1.Drop(), nil
default:
c1, err := NewCreateViewEntity(create1)
c1, err := NewCreateViewEntity(env, create1)
if err != nil {
return nil, err
}
c2, err := NewCreateViewEntity(create2)
c2, err := NewCreateViewEntity(env, create2)
if err != nil {
return nil, err
}
Expand All @@ -151,12 +151,12 @@ func DiffViews(create1 *sqlparser.CreateView, create2 *sqlparser.CreateView, hin
// DiffSchemasSQL compares two schemas and returns the rich diff that turns
// 1st schema into 2nd. Schemas are build from SQL, each of which can contain an arbitrary number of
// CREATE TABLE and CREATE VIEW statements.
func DiffSchemasSQL(sql1 string, sql2 string, hints *DiffHints, parser *sqlparser.Parser) (*SchemaDiff, error) {
schema1, err := NewSchemaFromSQL(sql1, parser)
func DiffSchemasSQL(env *Environment, sql1 string, sql2 string, hints *DiffHints) (*SchemaDiff, error) {
schema1, err := NewSchemaFromSQL(env, sql1)
if err != nil {
return nil, err
}
schema2, err := NewSchemaFromSQL(sql2, parser)
schema2, err := NewSchemaFromSQL(env, sql2)
if err != nil {
return nil, err
}
Expand All @@ -165,12 +165,12 @@ func DiffSchemasSQL(sql1 string, sql2 string, hints *DiffHints, parser *sqlparse

// DiffSchemas compares two schemas and returns the list of diffs that turn
// 1st schema into 2nd. Any of the schemas may be nil.
func DiffSchemas(schema1 *Schema, schema2 *Schema, hints *DiffHints) (*SchemaDiff, error) {
func DiffSchemas(env *Environment, schema1 *Schema, schema2 *Schema, hints *DiffHints) (*SchemaDiff, error) {

Check warning on line 168 in go/vt/schemadiff/diff.go

View check run for this annotation

Codecov / codecov/patch

go/vt/schemadiff/diff.go#L168

Added line #L168 was not covered by tests
if schema1 == nil {
schema1 = newEmptySchema()
schema1 = newEmptySchema(env)

Check warning on line 170 in go/vt/schemadiff/diff.go

View check run for this annotation

Codecov / codecov/patch

go/vt/schemadiff/diff.go#L170

Added line #L170 was not covered by tests
}
if schema2 == nil {
schema2 = newEmptySchema()
schema2 = newEmptySchema(env)

Check warning on line 173 in go/vt/schemadiff/diff.go

View check run for this annotation

Codecov / codecov/patch

go/vt/schemadiff/diff.go#L173

Added line #L173 was not covered by tests
}
return schema1.SchemaDiff(schema2, hints)
}
Loading

0 comments on commit 5241c23

Please sign in to comment.