From a207a69f5b17d18f305be9eadd3ec6958ee93bf0 Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Thu, 18 Jan 2024 07:34:25 +0200 Subject: [PATCH] `schemadiff`: formalize `InstantDDLCapability` (#14900) Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/mysql/capabilities/capability.go | 102 ++++++ go/mysql/capabilities/capability_test.go | 260 +++++++++++++ go/mysql/flavor.go | 49 +-- go/mysql/flavor_mysql.go | 50 +-- go/mysql/flavor_mysql_test.go | 1 - go/mysql/flavor_mysqlgr.go | 30 +- go/mysql/flavor_mysqlgr_test.go | 133 ++++++- go/mysql/flavor_test.go | 91 +---- go/vt/schemadiff/capability.go | 24 -- go/vt/schemadiff/schema.go | 20 + go/vt/schemadiff/schema_diff.go | 35 +- go/vt/schemadiff/schema_diff_test.go | 444 ++++++++++++----------- go/vt/schemadiff/table.go | 24 ++ go/vt/schemadiff/types.go | 13 + go/vt/schemadiff/view.go | 15 + 15 files changed, 841 insertions(+), 450 deletions(-) create mode 100644 go/mysql/capabilities/capability_test.go diff --git a/go/mysql/capabilities/capability.go b/go/mysql/capabilities/capability.go index 19d1361c3e6..d4b66821096 100644 --- a/go/mysql/capabilities/capability.go +++ b/go/mysql/capabilities/capability.go @@ -1,5 +1,33 @@ +/* +Copyright 2024 The Vitess Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + package capabilities +import ( + "strconv" + "strings" + + vtrpcpb "vitess.io/vitess/go/vt/proto/vtrpc" + "vitess.io/vitess/go/vt/vterrors" +) + +var ( + ErrUnspecifiedServerVersion = vterrors.Errorf(vtrpcpb.Code_INTERNAL, "server version unspecified") +) + type FlavorCapability int const ( @@ -21,3 +49,77 @@ const ( ) type CapableOf func(capability FlavorCapability) (bool, error) + +// ServerVersionAtLeast returns true if current server is at least given value. +// Example: if input is []int{8, 0, 23}... the function returns 'true' if we're +// on MySQL 8.0.23, 8.0.24, ... +func ServerVersionAtLeast(serverVersion string, parts ...int) (bool, error) { + if serverVersion == "" { + return false, ErrUnspecifiedServerVersion + } + versionPrefix := strings.Split(serverVersion, "-")[0] + versionTokens := strings.Split(versionPrefix, ".") + for i, part := range parts { + if len(versionTokens) <= i { + return false, nil + } + tokenValue, err := strconv.Atoi(versionTokens[i]) + if err != nil { + return false, err + } + if tokenValue > part { + return true, nil + } + if tokenValue < part { + return false, nil + } + } + return true, nil +} + +// MySQLVersionHasCapability is specific to MySQL flavors (of all versions) and answers whether +// the given server version has the requested capability. +func MySQLVersionHasCapability(serverVersion string, capability FlavorCapability) (bool, error) { + atLeast := func(parts ...int) (bool, error) { + return ServerVersionAtLeast(serverVersion, parts...) + } + // Capabilities sorted by version. + switch capability { + case MySQLJSONFlavorCapability: + return atLeast(5, 7, 0) + case InstantDDLFlavorCapability, + InstantExpandEnumCapability, + InstantAddLastColumnFlavorCapability, + InstantAddDropVirtualColumnFlavorCapability, + InstantChangeColumnDefaultFlavorCapability: + return atLeast(8, 0, 0) + case PerformanceSchemaDataLocksTableCapability: + return atLeast(8, 0, 1) + case MySQLUpgradeInServerFlavorCapability: + return atLeast(8, 0, 16) + case CheckConstraintsCapability: + return atLeast(8, 0, 16) + case TransactionalGtidExecutedFlavorCapability: + return atLeast(8, 0, 17) + case DisableRedoLogFlavorCapability: + return atLeast(8, 0, 21) + case FastDropTableFlavorCapability: + return atLeast(8, 0, 23) + case InstantAddDropColumnFlavorCapability: + return atLeast(8, 0, 29) + case DynamicRedoLogCapacityFlavorCapability: + return atLeast(8, 0, 30) + default: + return false, nil + } +} + +// MySQLVersionCapableOf returns a CapableOf function specific to MySQL flavors +func MySQLVersionCapableOf(serverVersion string) CapableOf { + if serverVersion == "" { + return nil + } + return func(capability FlavorCapability) (bool, error) { + return MySQLVersionHasCapability(serverVersion, capability) + } +} diff --git a/go/mysql/capabilities/capability_test.go b/go/mysql/capabilities/capability_test.go new file mode 100644 index 00000000000..339c05f1017 --- /dev/null +++ b/go/mysql/capabilities/capability_test.go @@ -0,0 +1,260 @@ +/* +Copyright 2024 The Vitess Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package capabilities + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestServerVersionAtLeast(t *testing.T) { + testcases := []struct { + version string + parts []int + expect bool + expectError bool + }{ + { + version: "8.0.14", + parts: []int{8, 0, 14}, + expect: true, + }, + { + version: "8.0.14-log", + parts: []int{8, 0, 14}, + expect: true, + }, + { + version: "8.0.14", + parts: []int{8, 0, 13}, + expect: true, + }, + { + version: "8.0.14-log", + parts: []int{8, 0, 13}, + expect: true, + }, + { + version: "8.0.14", + parts: []int{8, 0, 15}, + expect: false, + }, + { + version: "8.0.14-log", + parts: []int{8, 0, 15}, + expect: false, + }, + { + version: "8.0.14", + parts: []int{7, 5, 20}, + expect: true, + }, + { + version: "8.0.14", + parts: []int{7, 5}, + expect: true, + }, + { + version: "8.0.14", + parts: []int{5, 7}, + expect: true, + }, + { + version: "8.0.14-log", + parts: []int{7, 5, 20}, + expect: true, + }, + { + version: "8.0.14", + parts: []int{8, 1, 2}, + expect: false, + }, + { + version: "8.0.14", + parts: []int{10, 1, 2}, + expect: false, + }, + { + version: "8.0", + parts: []int{8, 0, 14}, + expect: false, + }, + { + version: "8.0.x", + parts: []int{8, 0, 14}, + expectError: true, + }, + { + version: "", + parts: []int{8, 0, 14}, + expectError: true, + }, + } + for _, tc := range testcases { + result, err := ServerVersionAtLeast(tc.version, tc.parts...) + if tc.expectError { + assert.Error(t, err) + } else { + assert.NoError(t, err) + assert.Equal(t, tc.expect, result) + } + } +} + +func TestMySQLVersionCapableOf(t *testing.T) { + testcases := []struct { + version string + capability FlavorCapability + isCapable bool + expectNil bool + }{ + { + version: "8.0.14", + capability: InstantDDLFlavorCapability, + isCapable: true, + }, + { + version: "8.0.20", + capability: TransactionalGtidExecutedFlavorCapability, + isCapable: true, + }, + { + version: "8.0.0", + capability: InstantAddLastColumnFlavorCapability, + isCapable: true, + }, + { + version: "8.0.0", + capability: InstantAddDropColumnFlavorCapability, + isCapable: false, + }, + { + version: "5.6.7", + capability: InstantDDLFlavorCapability, + isCapable: false, + }, + { + version: "5.7.29", + capability: TransactionalGtidExecutedFlavorCapability, + isCapable: false, + }, + { + version: "5.6.7", + capability: MySQLJSONFlavorCapability, + isCapable: false, + }, + { + version: "5.7.29", + capability: MySQLJSONFlavorCapability, + isCapable: true, + }, + { + version: "8.0.30", + capability: DynamicRedoLogCapacityFlavorCapability, + isCapable: true, + }, + { + version: "8.0.29", + capability: DynamicRedoLogCapacityFlavorCapability, + isCapable: false, + }, + { + version: "5.7.38", + capability: DynamicRedoLogCapacityFlavorCapability, + isCapable: false, + }, + { + version: "8.0.21", + capability: DisableRedoLogFlavorCapability, + isCapable: true, + }, + { + version: "8.0.20", + capability: DisableRedoLogFlavorCapability, + isCapable: false, + }, + { + version: "8.0.15", + capability: CheckConstraintsCapability, + isCapable: false, + }, + { + version: "8.0.15-log", + capability: CheckConstraintsCapability, + isCapable: false, + }, + { + version: "8.0.20", + capability: CheckConstraintsCapability, + isCapable: true, + }, + { + version: "8.0.20-log", + capability: CheckConstraintsCapability, + isCapable: true, + }, + { + version: "5.7.38", + capability: PerformanceSchemaDataLocksTableCapability, + isCapable: false, + }, + { + version: "8.0", + capability: PerformanceSchemaDataLocksTableCapability, + isCapable: false, + }, + { + version: "8.0.0", + capability: PerformanceSchemaDataLocksTableCapability, + isCapable: false, + }, + { + version: "8.0.20", + capability: PerformanceSchemaDataLocksTableCapability, + isCapable: true, + }, + { + // What happens if server version is unspecified + version: "", + capability: CheckConstraintsCapability, + isCapable: false, + expectNil: true, + }, + { + // Some ridiculous version. But seeing that we force the question on a MySQLVersionCapableOf + // then this far futuristic version should actually work. + version: "5914.234.17", + capability: CheckConstraintsCapability, + isCapable: true, + }, + } + for _, tc := range testcases { + name := fmt.Sprintf("%s %v", tc.version, tc.capability) + t.Run(name, func(t *testing.T) { + capableOf := MySQLVersionCapableOf(tc.version) + if tc.expectNil { + assert.Nil(t, capableOf) + return + } + isCapable, err := capableOf(tc.capability) + assert.NoError(t, err) + assert.Equal(t, tc.isCapable, isCapable) + }) + } +} diff --git a/go/mysql/flavor.go b/go/mysql/flavor.go index 1931d6f72ff..619496a9246 100644 --- a/go/mysql/flavor.go +++ b/go/mysql/flavor.go @@ -38,8 +38,6 @@ var ( // ErrNoPrimaryStatus means no status was returned by ShowPrimaryStatus(). ErrNoPrimaryStatus = errors.New("no master status") - - ErrUnspecifiedServerVersion = vterrors.Errorf(vtrpc.Code_INTERNAL, "server version unspecified") ) const ( @@ -141,44 +139,10 @@ type flavor interface { supportsCapability(capability capabilities.FlavorCapability) (bool, error) } -// flavors maps flavor names to their implementation. +// flavorFuncs maps flavor names to their implementation. // Flavors need to register only if they support being specified in the // connection parameters. -var flavors = make(map[string]func() flavor) - -// ServerVersionAtLeast returns true if current server is at least given value. -// Example: if input is []int{8, 0, 23}... the function returns 'true' if we're -// on MySQL 8.0.23, 8.0.24, ... -func ServerVersionAtLeast(serverVersion string, parts ...int) (bool, error) { - if serverVersion == "" { - return false, ErrUnspecifiedServerVersion - } - versionPrefix := strings.Split(serverVersion, "-")[0] - versionTokens := strings.Split(versionPrefix, ".") - for i, part := range parts { - if len(versionTokens) <= i { - return false, nil - } - tokenValue, err := strconv.Atoi(versionTokens[i]) - if err != nil { - return false, err - } - if tokenValue > part { - return true, nil - } - if tokenValue < part { - return false, nil - } - } - return true, nil -} - -// flavorCapableOf is a utility function that returns a CapableOf function for a given flavor -func flavorCapableOf(f flavor) capabilities.CapableOf { - return func(capability capabilities.FlavorCapability) (bool, error) { - return f.supportsCapability(capability) - } -} +var flavorFuncs = make(map[string]func() flavor) // GetFlavor fills in c.Flavor. If the params specify the flavor, // that is used. Otherwise, we auto-detect. @@ -212,9 +176,10 @@ func GetFlavor(serverVersion string, flavorFunc func() flavor) (f flavor, capabl case strings.HasPrefix(serverVersion, mysql80VersionPrefix): f = mysqlFlavor80{mysqlFlavor{serverVersion: serverVersion}} default: + // If unknown, return the most basic flavor: MySQL 56. f = mysqlFlavor56{mysqlFlavor{serverVersion: serverVersion}} } - return f, flavorCapableOf(f), canonicalVersion + return f, f.supportsCapability, canonicalVersion } // ServerVersionCapableOf is a convenience function that returns a CapableOf function given a server version. @@ -237,14 +202,14 @@ func ServerVersionCapableOf(serverVersion string) (capableOf capabilities.Capabl // as well (not matching what c.ServerVersion is, but matching after we remove // the prefix). func (c *Conn) fillFlavor(params *ConnParams) { - flavorFunc := flavors[params.Flavor] + flavorFunc := flavorFuncs[params.Flavor] c.flavor, _, c.ServerVersion = GetFlavor(c.ServerVersion, flavorFunc) } // ServerVersionAtLeast returns 'true' if server version is equal or greater than given parts. e.g. // "8.0.14-log" is at least [8, 0, 13] and [8, 0, 14], but not [8, 0, 15] func (c *Conn) ServerVersionAtLeast(parts ...int) (bool, error) { - return ServerVersionAtLeast(c.ServerVersion, parts...) + return capabilities.ServerVersionAtLeast(c.ServerVersion, parts...) } // @@ -474,5 +439,5 @@ func (c *Conn) SupportsCapability(capability capabilities.FlavorCapability) (boo } func init() { - flavors[replication.FilePosFlavorID] = newFilePosFlavor + flavorFuncs[replication.FilePosFlavorID] = newFilePosFlavor } diff --git a/go/mysql/flavor_mysql.go b/go/mysql/flavor_mysql.go index 2406a7e001f..c0a06540368 100644 --- a/go/mysql/flavor_mysql.go +++ b/go/mysql/flavor_mysql.go @@ -374,11 +374,10 @@ func (mysqlFlavor56) baseShowTablesWithSizes() string { } // supportsCapability is part of the Flavor interface. -func (mysqlFlavor56) supportsCapability(capability capabilities.FlavorCapability) (bool, error) { - switch capability { - default: - return false, nil - } +func (f mysqlFlavor56) supportsCapability(capability capabilities.FlavorCapability) (bool, error) { + // We do not support MySQL 5.6. Also, for consistency, and seeing that MySQL56 is the default + // flavor if no version specified, we return false for all capabilities. + return false, nil } // baseShowTablesWithSizes is part of the Flavor interface. @@ -387,13 +386,8 @@ func (mysqlFlavor57) baseShowTablesWithSizes() string { } // supportsCapability is part of the Flavor interface. -func (mysqlFlavor57) supportsCapability(capability capabilities.FlavorCapability) (bool, error) { - switch capability { - case capabilities.MySQLJSONFlavorCapability: - return true, nil - default: - return false, nil - } +func (f mysqlFlavor57) supportsCapability(capability capabilities.FlavorCapability) (bool, error) { + return capabilities.MySQLVersionHasCapability(f.serverVersion, capability) } // baseShowTablesWithSizes is part of the Flavor interface. @@ -403,35 +397,5 @@ func (mysqlFlavor80) baseShowTablesWithSizes() string { // supportsCapability is part of the Flavor interface. func (f mysqlFlavor80) supportsCapability(capability capabilities.FlavorCapability) (bool, error) { - serverVersionAtLeast := func(parts ...int) (bool, error) { - return ServerVersionAtLeast(f.serverVersion, parts...) - } - switch capability { - case capabilities.InstantDDLFlavorCapability, - capabilities.InstantExpandEnumCapability, - capabilities.InstantAddLastColumnFlavorCapability, - capabilities.InstantAddDropVirtualColumnFlavorCapability, - capabilities.InstantChangeColumnDefaultFlavorCapability: - return true, nil - case capabilities.InstantAddDropColumnFlavorCapability: - return serverVersionAtLeast(8, 0, 29) - case capabilities.TransactionalGtidExecutedFlavorCapability: - return serverVersionAtLeast(8, 0, 17) - case capabilities.FastDropTableFlavorCapability: - return serverVersionAtLeast(8, 0, 23) - case capabilities.MySQLJSONFlavorCapability: - return true, nil - case capabilities.MySQLUpgradeInServerFlavorCapability: - return serverVersionAtLeast(8, 0, 16) - case capabilities.DynamicRedoLogCapacityFlavorCapability: - return serverVersionAtLeast(8, 0, 30) - case capabilities.DisableRedoLogFlavorCapability: - return serverVersionAtLeast(8, 0, 21) - case capabilities.CheckConstraintsCapability: - return serverVersionAtLeast(8, 0, 16) - case capabilities.PerformanceSchemaDataLocksTableCapability: - return true, nil - default: - return false, nil - } + return capabilities.MySQLVersionHasCapability(f.serverVersion, capability) } diff --git a/go/mysql/flavor_mysql_test.go b/go/mysql/flavor_mysql_test.go index 0e1b749633a..7a6ddd47005 100644 --- a/go/mysql/flavor_mysql_test.go +++ b/go/mysql/flavor_mysql_test.go @@ -73,5 +73,4 @@ func TestMysql56SetReplicationSourceCommandSSL(t *testing.T) { conn := &Conn{flavor: mysqlFlavor57{}} got := conn.SetReplicationSourceCommand(params, host, port, connectRetry) assert.Equal(t, want, got, "mysqlFlavor.SetReplicationSourceCommand(%#v, %#v, %#v, %#v) = %#v, want %#v", params, host, port, connectRetry, got, want) - } diff --git a/go/mysql/flavor_mysqlgr.go b/go/mysql/flavor_mysqlgr.go index 563552464d3..c572a554ca2 100644 --- a/go/mysql/flavor_mysqlgr.go +++ b/go/mysql/flavor_mysqlgr.go @@ -250,35 +250,9 @@ func (mysqlGRFlavor) baseShowTablesWithSizes() string { // supportsCapability is part of the Flavor interface. func (f mysqlGRFlavor) supportsCapability(capability capabilities.FlavorCapability) (bool, error) { - serverVersionAtLeast := func(parts ...int) (bool, error) { - return ServerVersionAtLeast(f.serverVersion, parts...) - } - switch capability { - case capabilities.InstantDDLFlavorCapability, - capabilities.InstantExpandEnumCapability, - capabilities.InstantAddLastColumnFlavorCapability, - capabilities.InstantAddDropVirtualColumnFlavorCapability, - capabilities.InstantChangeColumnDefaultFlavorCapability: - return serverVersionAtLeast(8, 0, 0) - case capabilities.InstantAddDropColumnFlavorCapability: - return serverVersionAtLeast(8, 0, 29) - case capabilities.TransactionalGtidExecutedFlavorCapability: - return serverVersionAtLeast(8, 0, 17) - case capabilities.FastDropTableFlavorCapability: - return serverVersionAtLeast(8, 0, 23) - case capabilities.MySQLJSONFlavorCapability: - return serverVersionAtLeast(5, 7, 0) - case capabilities.MySQLUpgradeInServerFlavorCapability: - return serverVersionAtLeast(8, 0, 16) - case capabilities.DynamicRedoLogCapacityFlavorCapability: - return serverVersionAtLeast(8, 0, 30) - case capabilities.CheckConstraintsCapability: - return serverVersionAtLeast(8, 0, 16) - default: - return false, nil - } + return capabilities.MySQLVersionHasCapability(f.serverVersion, capability) } func init() { - flavors[GRFlavorID] = newMysqlGRFlavor + flavorFuncs[GRFlavorID] = newMysqlGRFlavor } diff --git a/go/mysql/flavor_mysqlgr_test.go b/go/mysql/flavor_mysqlgr_test.go index df7876eca1c..348aefca934 100644 --- a/go/mysql/flavor_mysqlgr_test.go +++ b/go/mysql/flavor_mysqlgr_test.go @@ -16,13 +16,15 @@ limitations under the License. package mysql import ( + "fmt" "testing" - "gotest.tools/assert" + "github.com/stretchr/testify/assert" + "vitess.io/vitess/go/mysql/capabilities" "vitess.io/vitess/go/mysql/replication" - "vitess.io/vitess/go/sqltypes" + querypb "vitess.io/vitess/go/vt/proto/query" ) @@ -53,3 +55,130 @@ func TestMysqlGRReplicationApplierLagParse(t *testing.T) { parseReplicationApplierLag(&res, row) assert.Equal(t, uint32(100), res.ReplicationLagSeconds) } + +func TestMysqlGRSupportCapability(t *testing.T) { + testcases := []struct { + version string + capability capabilities.FlavorCapability + isCapable bool + expectError error + }{ + { + version: "8.0.14", + capability: capabilities.InstantDDLFlavorCapability, + isCapable: true, + }, + { + version: "8.0.20", + capability: capabilities.TransactionalGtidExecutedFlavorCapability, + isCapable: true, + }, + { + version: "8.0.0", + capability: capabilities.InstantAddLastColumnFlavorCapability, + isCapable: true, + }, + { + version: "8.0.0", + capability: capabilities.InstantAddDropColumnFlavorCapability, + isCapable: false, + }, + { + version: "5.6.7", + capability: capabilities.InstantDDLFlavorCapability, + isCapable: false, + }, + { + version: "5.7.29", + capability: capabilities.TransactionalGtidExecutedFlavorCapability, + isCapable: false, + }, + { + version: "5.6.7", + capability: capabilities.MySQLJSONFlavorCapability, + isCapable: false, + }, + { + version: "5.7.29", + capability: capabilities.MySQLJSONFlavorCapability, + isCapable: true, + }, + { + version: "8.0.30", + capability: capabilities.DynamicRedoLogCapacityFlavorCapability, + isCapable: true, + }, + { + version: "8.0.29", + capability: capabilities.DynamicRedoLogCapacityFlavorCapability, + isCapable: false, + }, + { + version: "5.7.38", + capability: capabilities.DynamicRedoLogCapacityFlavorCapability, + isCapable: false, + }, + { + version: "8.0.21", + capability: capabilities.DisableRedoLogFlavorCapability, + isCapable: true, + }, + { + version: "8.0.20", + capability: capabilities.DisableRedoLogFlavorCapability, + isCapable: false, + }, + { + version: "8.0.15", + capability: capabilities.CheckConstraintsCapability, + isCapable: false, + }, + { + version: "8.0.20", + capability: capabilities.CheckConstraintsCapability, + isCapable: true, + }, + { + version: "8.0.20-log", + capability: capabilities.CheckConstraintsCapability, + isCapable: true, + }, + { + version: "5.7.38", + capability: capabilities.PerformanceSchemaDataLocksTableCapability, + isCapable: false, + }, + { + version: "8.0.20", + capability: capabilities.PerformanceSchemaDataLocksTableCapability, + isCapable: true, + }, + { + // What happens if server version is unspecified + version: "", + capability: capabilities.CheckConstraintsCapability, + isCapable: false, + expectError: capabilities.ErrUnspecifiedServerVersion, + }, + { + // Some ridiculous version. But seeing that we force the flavor to be mysqlGR, + // then this far futuristic version should actually work. + version: "5914.234.17", + capability: capabilities.CheckConstraintsCapability, + isCapable: true, + }, + } + for _, tc := range testcases { + name := fmt.Sprintf("%s %v", tc.version, tc.capability) + t.Run(name, func(t *testing.T) { + flavor := &mysqlGRFlavor{mysqlFlavor{serverVersion: tc.version}} + isCapable, err := flavor.supportsCapability(tc.capability) + if tc.expectError != nil { + assert.ErrorContains(t, err, tc.expectError.Error()) + return + } + assert.NoError(t, err) + assert.Equal(t, tc.isCapable, isCapable) + }) + } +} diff --git a/go/mysql/flavor_test.go b/go/mysql/flavor_test.go index 997101d6b7c..b0ef7a1aafc 100644 --- a/go/mysql/flavor_test.go +++ b/go/mysql/flavor_test.go @@ -22,96 +22,7 @@ import ( "vitess.io/vitess/go/mysql/capabilities" ) -func TestServerVersionAtLeast(t *testing.T) { - testcases := []struct { - version string - parts []int - expect bool - expectError bool - }{ - { - version: "8.0.14", - parts: []int{8, 0, 14}, - expect: true, - }, - { - version: "8.0.14-log", - parts: []int{8, 0, 14}, - expect: true, - }, - { - version: "8.0.14", - parts: []int{8, 0, 13}, - expect: true, - }, - { - version: "8.0.14-log", - parts: []int{8, 0, 13}, - expect: true, - }, - { - version: "8.0.14", - parts: []int{8, 0, 15}, - expect: false, - }, - { - version: "8.0.14-log", - parts: []int{8, 0, 15}, - expect: false, - }, - { - version: "8.0.14", - parts: []int{7, 5, 20}, - expect: true, - }, - { - version: "8.0.14", - parts: []int{7, 5}, - expect: true, - }, - { - version: "8.0.14-log", - parts: []int{7, 5, 20}, - expect: true, - }, - { - version: "8.0.14", - parts: []int{8, 1, 2}, - expect: false, - }, - { - version: "8.0.14", - parts: []int{10, 1, 2}, - expect: false, - }, - { - version: "8.0", - parts: []int{8, 0, 14}, - expect: false, - }, - { - version: "8.0.x", - parts: []int{8, 0, 14}, - expectError: true, - }, - { - version: "", - parts: []int{8, 0, 14}, - expectError: true, - }, - } - for _, tc := range testcases { - result, err := ServerVersionAtLeast(tc.version, tc.parts...) - if tc.expectError { - assert.Error(t, err) - } else { - assert.NoError(t, err) - assert.Equal(t, tc.expect, result) - } - } -} - -func TestGetFlavor(t *testing.T) { +func TestServerVersionCapableOf(t *testing.T) { testcases := []struct { version string capability capabilities.FlavorCapability diff --git a/go/vt/schemadiff/capability.go b/go/vt/schemadiff/capability.go index 3008fbda617..532cfd72fe8 100644 --- a/go/vt/schemadiff/capability.go +++ b/go/vt/schemadiff/capability.go @@ -4,13 +4,7 @@ import ( "strings" "vitess.io/vitess/go/mysql/capabilities" - "vitess.io/vitess/go/vt/proto/vtrpc" "vitess.io/vitess/go/vt/sqlparser" - "vitess.io/vitess/go/vt/vterrors" -) - -var ( - ErrUnexpectedDiffType = vterrors.Errorf(vtrpc.Code_INTERNAL, "unexpected diff type") ) // alterOptionAvailableViaInstantDDL checks if the specific alter option is eligible to run via ALGORITHM=INSTANT @@ -181,21 +175,3 @@ func AlterTableCapableOfInstantDDL(alterTable *sqlparser.AlterTable, createTable } return true, nil } - -// diffCapableOfInstantDDL checks whether the given diff is either trivially instantaneous (e.g. CREATE TABLE) or -// is capable of `ALGORITHM=INSTANT`. -func diffCapableOfInstantDDL(diff EntityDiff, capableOf capabilities.CapableOf) (bool, error) { - switch diff := diff.(type) { - case *CreateTableEntityDiff, - *RenameTableEntityDiff, - *DropTableEntityDiff, - *CreateViewEntityDiff, - *AlterViewEntityDiff, - *DropViewEntityDiff: - return true, nil - case *AlterTableEntityDiff: - return AlterTableCapableOfInstantDDL(diff.AlterTable(), diff.from.CreateTable, capableOf) - default: - return false, ErrUnexpectedDiffType - } -} diff --git a/go/vt/schemadiff/schema.go b/go/vt/schemadiff/schema.go index 47372eada25..fafe2dbc189 100644 --- a/go/vt/schemadiff/schema.go +++ b/go/vt/schemadiff/schema.go @@ -23,6 +23,7 @@ import ( "sort" "strings" + "vitess.io/vitess/go/mysql/capabilities" "vitess.io/vitess/go/vt/proto/vtrpc" "vitess.io/vitess/go/vt/sqlparser" "vitess.io/vitess/go/vt/vterrors" @@ -964,6 +965,25 @@ func (s *Schema) SchemaDiff(other *Schema, hints *DiffHints) (*SchemaDiff, error // No need to handle. Any dependencies will be resolved by any of the other cases } } + + // Check and assign capabilities: + // Reminder: schemadiff assumes a MySQL flavor, so we only check for MySQL capabilities. + if capableOf := capabilities.MySQLVersionCapableOf(hints.MySQLServerVersion); capableOf != nil { + for _, diff := range schemaDiff.UnorderedDiffs() { + switch diff := diff.(type) { + case *AlterTableEntityDiff: + instantDDLCapable, err := AlterTableCapableOfInstantDDL(diff.AlterTable(), diff.from.CreateTable, capableOf) + if err != nil { + return nil, err + } + if instantDDLCapable { + diff.instantDDLCapability = InstantDDLCapabilityPossible + } else { + diff.instantDDLCapability = InstantDDLCapabilityImpossible + } + } + } + } return schemaDiff, nil } diff --git a/go/vt/schemadiff/schema_diff.go b/go/vt/schemadiff/schema_diff.go index bce0b2ef0e5..d2f5e012220 100644 --- a/go/vt/schemadiff/schema_diff.go +++ b/go/vt/schemadiff/schema_diff.go @@ -18,12 +18,10 @@ package schemadiff import ( "context" - "errors" "fmt" "sort" "vitess.io/vitess/go/mathutil" - "vitess.io/vitess/go/mysql/capabilities" ) type DiffDependencyType int @@ -346,23 +344,24 @@ func (d *SchemaDiff) OrderedDiffs(ctx context.Context) ([]EntityDiff, error) { return orderedDiffs, nil } -// CapableOfInstantDDL returns `true` if all diffs are capable of instant DDL, or are otherwise trivially -// instantaneously applicable (such as `CREATE TABLE` or `ALTER VIEW`). The answer essentially indicates whether -// the entire set of changes can be applied as an immediate operation. -func (d *SchemaDiff) CapableOfInstantDDL(ctx context.Context, capableOf capabilities.CapableOf) (bool, error) { - if capableOf == nil { - return false, nil - } - var errs error - allCapable := true +// InstantDDLCapability returns an overall summary of the ability of the diffs to run with ALGORITHM=INSTANT. +// It is a convenience method, whose logic anyone can reimplement. +func (d *SchemaDiff) InstantDDLCapability() InstantDDLCapability { + // The general logic: we return "InstantDDLCapabilityPossible" if there is one or more diffs that is capable of + // ALGORITHM=INSTANT, and zero or more diffs that are irrelevant, and no diffs that are impossible to run with + // ALGORITHM=INSTANT. + capability := InstantDDLCapabilityIrrelevant for _, diff := range d.UnorderedDiffs() { - capable, err := diffCapableOfInstantDDL(diff, capableOf) - if err != nil { - errs = errors.Join(errs, err) - } - if !capable { - allCapable = false + switch diff.InstantDDLCapability() { + case InstantDDLCapabilityUnknown: + return InstantDDLCapabilityUnknown // Early break + case InstantDDLCapabilityImpossible: + return InstantDDLCapabilityImpossible // Early break + case InstantDDLCapabilityPossible: + capability = InstantDDLCapabilityPossible + case InstantDDLCapabilityIrrelevant: + // do nothing } } - return allCapable, errs + return capability } diff --git a/go/vt/schemadiff/schema_diff_test.go b/go/vt/schemadiff/schema_diff_test.go index da2e1206d36..3b59166c05c 100644 --- a/go/vt/schemadiff/schema_diff_test.go +++ b/go/vt/schemadiff/schema_diff_test.go @@ -23,8 +23,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - - "vitess.io/vitess/go/mysql/capabilities" ) func TestPermutations(t *testing.T) { @@ -253,19 +251,6 @@ func TestPermutationsContext(t *testing.T) { func TestSchemaDiff(t *testing.T) { ctx := context.Background() - capableOf := func(capability capabilities.FlavorCapability) (bool, error) { - switch capability { - case - capabilities.InstantDDLFlavorCapability, - capabilities.InstantAddLastColumnFlavorCapability, - capabilities.InstantAddDropVirtualColumnFlavorCapability, - capabilities.InstantAddDropColumnFlavorCapability, - capabilities.InstantChangeColumnDefaultFlavorCapability, - capabilities.InstantExpandEnumCapability: - return true, nil - } - return false, nil - } var ( createQueries = []string{ "create table t1 (id int primary key, info int not null);", @@ -274,21 +259,22 @@ func TestSchemaDiff(t *testing.T) { } ) tt := []struct { - name string - fromQueries []string - toQueries []string - expectDiffs int - expectDeps int - sequential bool - conflictingDiffs int - entityOrder []string // names of tables/views in expected diff order - instantCapable bool + name string + fromQueries []string + toQueries []string + expectDiffs int + expectDeps int + sequential bool + conflictingDiffs int + entityOrder []string // names of tables/views in expected diff order + mysqlServerVersion string + instantCapability InstantDDLCapability }{ { - name: "no change", - toQueries: createQueries, - entityOrder: []string{}, - instantCapable: true, + name: "no change", + toQueries: createQueries, + entityOrder: []string{}, + instantCapability: InstantDDLCapabilityIrrelevant, }, { name: "three unrelated changes", @@ -298,9 +284,32 @@ func TestSchemaDiff(t *testing.T) { "create view v1 as select id from t1", "create view v2 as select 1 from dual", }, - expectDiffs: 3, - entityOrder: []string{"t1", "t2", "v2"}, - instantCapable: true, + expectDiffs: 3, + entityOrder: []string{"t1", "t2", "v2"}, + instantCapability: InstantDDLCapabilityPossible, + }, + { + name: "instant DDL possible on 8.0.32", + toQueries: []string{ + "create table t1 (id int primary key, ts timestamp, info int not null);", + "create table t2 (id int primary key, ts timestamp);", + "create view v1 as select id from t1", + }, + expectDiffs: 1, + entityOrder: []string{"t1"}, + instantCapability: InstantDDLCapabilityPossible, + }, + { + name: "instant DDL impossible on 8.0.17", + toQueries: []string{ + "create table t1 (id int primary key, ts timestamp, info int not null);", + "create table t2 (id int primary key, ts timestamp);", + "create view v1 as select id from t1", + }, + mysqlServerVersion: "8.0.17", + expectDiffs: 1, + entityOrder: []string{"t1"}, + instantCapability: InstantDDLCapabilityImpossible, }, { name: "three unrelated changes 2", @@ -309,9 +318,9 @@ func TestSchemaDiff(t *testing.T) { "create table t2 (id int primary key, ts timestamp, v varchar);", "create view v2 as select 1 from dual", }, - expectDiffs: 3, - entityOrder: []string{"v1", "t2", "v2"}, - instantCapable: true, + expectDiffs: 3, + entityOrder: []string{"v1", "t2", "v2"}, + instantCapability: InstantDDLCapabilityPossible, }, // Subsequent { @@ -321,9 +330,10 @@ func TestSchemaDiff(t *testing.T) { "create table t2 (id int primary key, ts timestamp, v varchar, fulltext key ftk1 (v));", "create view v1 as select id from t1", }, - expectDiffs: 1, - expectDeps: 0, - entityOrder: []string{"t2"}, + expectDiffs: 1, + expectDeps: 0, + entityOrder: []string{"t2"}, + instantCapability: InstantDDLCapabilityImpossible, }, { // MySQL limitation: you cannot add two FULLTEXT keys in a single statement. `schemadiff` complies @@ -334,10 +344,11 @@ func TestSchemaDiff(t *testing.T) { "create table t2 (id int primary key, ts timestamp, v varchar, fulltext key ftk1 (v), fulltext key ftk2 (v));", "create view v1 as select id from t1", }, - expectDiffs: 2, - expectDeps: 1, - sequential: true, - entityOrder: []string{"t2", "t2"}, + expectDiffs: 2, + expectDeps: 1, + sequential: true, + entityOrder: []string{"t2", "t2"}, + instantCapability: InstantDDLCapabilityImpossible, }, { name: "add partition", @@ -351,9 +362,10 @@ func TestSchemaDiff(t *testing.T) { "create table t2 (id int primary key, ts timestamp) partition by range (id) (partition p0 values less than (0), partition p1 values less than (1), partition p2 values less than (2));", "create view v1 as select id from t1", }, - expectDiffs: 1, - expectDeps: 0, - entityOrder: []string{"t2"}, + expectDiffs: 1, + expectDeps: 0, + entityOrder: []string{"t2"}, + instantCapability: InstantDDLCapabilityImpossible, }, { // In MySQL, you cannot ALTER TABLE ADD COLUMN ..., ADD PARTITION in a single statement @@ -368,10 +380,11 @@ func TestSchemaDiff(t *testing.T) { "create table t2 (id int primary key, ts timestamp, v varchar) partition by range (id) (partition p0 values less than (0), partition p1 values less than (1), partition p2 values less than (2));", "create view v1 as select id from t1", }, - expectDiffs: 2, - expectDeps: 1, - sequential: true, - entityOrder: []string{"t2", "t2"}, + expectDiffs: 2, + expectDeps: 1, + sequential: true, + entityOrder: []string{"t2", "t2"}, + instantCapability: InstantDDLCapabilityImpossible, }, { name: "add view", @@ -379,9 +392,9 @@ func TestSchemaDiff(t *testing.T) { createQueries, "create view v2 as select id from t2", ), - expectDiffs: 1, - entityOrder: []string{"v2"}, - instantCapable: true, + expectDiffs: 1, + entityOrder: []string{"v2"}, + instantCapability: InstantDDLCapabilityIrrelevant, }, { name: "add view, alter table", @@ -391,10 +404,10 @@ func TestSchemaDiff(t *testing.T) { "create view v1 as select id from t1", "create view v2 as select id from t2", }, - expectDiffs: 2, - expectDeps: 1, - entityOrder: []string{"t2", "v2"}, - instantCapable: true, + expectDiffs: 2, + expectDeps: 1, + entityOrder: []string{"t2", "v2"}, + instantCapability: InstantDDLCapabilityPossible, }, { name: "alter view, alter table", @@ -403,10 +416,11 @@ func TestSchemaDiff(t *testing.T) { "create table t2 (id int primary key, ts timestamp);", "create view v1 as select the_id from t1", }, - expectDiffs: 2, - expectDeps: 1, - entityOrder: []string{"t1", "v1"}, - conflictingDiffs: 2, + expectDiffs: 2, + expectDeps: 1, + entityOrder: []string{"t1", "v1"}, + conflictingDiffs: 2, + instantCapability: InstantDDLCapabilityImpossible, }, { name: "alter table, add view", @@ -416,10 +430,10 @@ func TestSchemaDiff(t *testing.T) { "create view v1 as select id from t1", "create view v2 as select id, v from t2", }, - expectDiffs: 2, - expectDeps: 1, - entityOrder: []string{"t2", "v2"}, - instantCapable: true, + expectDiffs: 2, + expectDeps: 1, + entityOrder: []string{"t2", "v2"}, + instantCapability: InstantDDLCapabilityPossible, }, { name: "create view depending on 2 tables, alter table", @@ -429,10 +443,10 @@ func TestSchemaDiff(t *testing.T) { "create view v1 as select id from t1", "create view v2 as select info, v from t1, t2", }, - expectDiffs: 2, - expectDeps: 1, - entityOrder: []string{"t2", "v2"}, - instantCapable: true, + expectDiffs: 2, + expectDeps: 1, + entityOrder: []string{"t2", "v2"}, + instantCapability: InstantDDLCapabilityPossible, }, { name: "create view depending on 2 tables, alter other table", @@ -444,10 +458,10 @@ func TestSchemaDiff(t *testing.T) { "create view v2 as select info, ts from t1, t2", // "create view v2 as select info, ts from t1, t2", }, - expectDiffs: 2, - expectDeps: 1, - entityOrder: []string{"t1", "v2"}, - instantCapable: true, + expectDiffs: 2, + expectDeps: 1, + entityOrder: []string{"t1", "v2"}, + instantCapability: InstantDDLCapabilityPossible, }, { name: "create view depending on 2 tables, alter both tables", @@ -457,10 +471,10 @@ func TestSchemaDiff(t *testing.T) { "create view v1 as select id from t1", "create view v2 as select info, ts from t1, t2", }, - expectDiffs: 3, - expectDeps: 2, - entityOrder: []string{"t1", "t2", "v2"}, - instantCapable: true, + expectDiffs: 3, + expectDeps: 2, + entityOrder: []string{"t1", "t2", "v2"}, + instantCapability: InstantDDLCapabilityPossible, }, { name: "alter view depending on 2 tables, uses new column, alter tables", @@ -470,10 +484,10 @@ func TestSchemaDiff(t *testing.T) { "create view v1 as select id from t1", "create view v2 as select info, v from t1, t2", }, - expectDiffs: 3, - expectDeps: 2, - entityOrder: []string{"t1", "t2", "v2"}, - instantCapable: true, + expectDiffs: 3, + expectDeps: 2, + entityOrder: []string{"t1", "t2", "v2"}, + instantCapability: InstantDDLCapabilityPossible, }, { name: "drop view", @@ -481,10 +495,10 @@ func TestSchemaDiff(t *testing.T) { "create table t1 (id int primary key, info int not null);", "create table t2 (id int primary key, ts timestamp);", }, - expectDiffs: 1, - expectDeps: 0, - entityOrder: []string{"v1"}, - instantCapable: true, + expectDiffs: 1, + expectDeps: 0, + entityOrder: []string{"v1"}, + instantCapability: InstantDDLCapabilityIrrelevant, }, { name: "drop view, alter dependent table", @@ -492,30 +506,30 @@ func TestSchemaDiff(t *testing.T) { "create table t1 (id int primary key, info int not null, dt datetime);", "create table t2 (id int primary key, ts timestamp);", }, - expectDiffs: 2, - expectDeps: 1, - entityOrder: []string{"v1", "t1"}, - instantCapable: true, + expectDiffs: 2, + expectDeps: 1, + entityOrder: []string{"v1", "t1"}, + instantCapability: InstantDDLCapabilityPossible, }, { name: "drop view, drop dependent table", toQueries: []string{ "create table t2 (id int primary key, ts timestamp);", }, - expectDiffs: 2, - expectDeps: 1, - entityOrder: []string{"v1", "t1"}, - instantCapable: true, + expectDiffs: 2, + expectDeps: 1, + entityOrder: []string{"v1", "t1"}, + instantCapability: InstantDDLCapabilityIrrelevant, }, { name: "drop view, drop unrelated table", toQueries: []string{ "create table t1 (id int primary key, info int not null);", }, - expectDiffs: 2, - expectDeps: 0, - entityOrder: []string{"v1", "t2"}, - instantCapable: true, + expectDiffs: 2, + expectDeps: 0, + entityOrder: []string{"v1", "t2"}, + instantCapability: InstantDDLCapabilityIrrelevant, }, { name: "alter view, drop table", @@ -523,10 +537,10 @@ func TestSchemaDiff(t *testing.T) { "create table t2 (id int primary key, ts timestamp);", "create view v1 as select id from t2", }, - expectDiffs: 2, - expectDeps: 1, - entityOrder: []string{"v1", "t1"}, - instantCapable: true, + expectDiffs: 2, + expectDeps: 1, + entityOrder: []string{"v1", "t1"}, + instantCapability: InstantDDLCapabilityIrrelevant, }, { name: "alter view, add view", @@ -536,10 +550,10 @@ func TestSchemaDiff(t *testing.T) { "create view v1 as select id, info from t1", "create view v2 as select info from v1", }, - expectDiffs: 2, - expectDeps: 1, - entityOrder: []string{"v1", "v2"}, - instantCapable: true, + expectDiffs: 2, + expectDeps: 1, + entityOrder: []string{"v1", "v2"}, + instantCapability: InstantDDLCapabilityIrrelevant, }, { name: "alter view, add view, 2", @@ -549,10 +563,10 @@ func TestSchemaDiff(t *testing.T) { "create view v1 as select id, ts from v2", "create view v2 as select id, ts from t2", }, - expectDiffs: 2, - expectDeps: 1, - entityOrder: []string{"v2", "v1"}, - instantCapable: true, + expectDiffs: 2, + expectDeps: 1, + entityOrder: []string{"v2", "v1"}, + instantCapability: InstantDDLCapabilityIrrelevant, }, { name: "alter table, alter view, add view", @@ -562,10 +576,10 @@ func TestSchemaDiff(t *testing.T) { "create view v1 as select ts from t2", "create view v2 as select v from t2", }, - expectDiffs: 3, - expectDeps: 2, - entityOrder: []string{"t2", "v1", "v2"}, - instantCapable: true, + expectDiffs: 3, + expectDeps: 2, + entityOrder: []string{"t2", "v1", "v2"}, + instantCapability: InstantDDLCapabilityPossible, }, { name: "alter table, alter view, impossible sequence", @@ -577,10 +591,10 @@ func TestSchemaDiff(t *testing.T) { "create table t1 (id int primary key, newcol int not null);", "create view v1 as select id, newcol from t1", }, - expectDiffs: 2, - expectDeps: 1, - conflictingDiffs: 2, - instantCapable: true, + expectDiffs: 2, + expectDeps: 1, + conflictingDiffs: 2, + instantCapability: InstantDDLCapabilityPossible, }, // FKs @@ -590,9 +604,9 @@ func TestSchemaDiff(t *testing.T) { createQueries, "create table t3 (id int primary key, ts timestamp, t1_id int, foreign key (t1_id) references t1 (id) on delete no action);", ), - expectDiffs: 1, - entityOrder: []string{"t3"}, - instantCapable: true, + expectDiffs: 1, + entityOrder: []string{"t3"}, + instantCapability: InstantDDLCapabilityIrrelevant, }, { name: "create two tables with fk", @@ -601,11 +615,11 @@ func TestSchemaDiff(t *testing.T) { "create table tp (id int primary key, info int not null);", "create table t3 (id int primary key, ts timestamp, tp_id int, foreign key (tp_id) references tp (id) on delete no action);", ), - expectDiffs: 2, - expectDeps: 1, - entityOrder: []string{"tp", "t3"}, - sequential: true, - instantCapable: true, + expectDiffs: 2, + expectDeps: 1, + entityOrder: []string{"tp", "t3"}, + sequential: true, + instantCapability: InstantDDLCapabilityIrrelevant, }, { name: "add FK", @@ -614,9 +628,10 @@ func TestSchemaDiff(t *testing.T) { "create table t2 (id int primary key, ts timestamp, t1_id int, foreign key (t1_id) references t1 (id) on delete no action);", "create view v1 as select id from t1", }, - expectDiffs: 1, - expectDeps: 0, - entityOrder: []string{"t2"}, + expectDiffs: 1, + expectDeps: 0, + entityOrder: []string{"t2"}, + instantCapability: InstantDDLCapabilityImpossible, }, { name: "add FK pointing to new table", @@ -626,10 +641,11 @@ func TestSchemaDiff(t *testing.T) { "create table tp (id int primary key, info int not null);", "create view v1 as select id from t1", }, - expectDiffs: 2, - expectDeps: 1, - sequential: true, - entityOrder: []string{"tp", "t2"}, + expectDiffs: 2, + expectDeps: 1, + sequential: true, + entityOrder: []string{"tp", "t2"}, + instantCapability: InstantDDLCapabilityImpossible, }, { name: "add FK, unrelated alter", @@ -638,9 +654,10 @@ func TestSchemaDiff(t *testing.T) { "create table t2 (id int primary key, ts timestamp, t1_id int, foreign key (t1_id) references t1 (id) on delete no action);", "create view v1 as select id from t1", }, - expectDiffs: 2, - expectDeps: 1, - entityOrder: []string{"t1", "t2"}, + expectDiffs: 2, + expectDeps: 1, + entityOrder: []string{"t1", "t2"}, + instantCapability: InstantDDLCapabilityImpossible, }, { name: "add FK, add unrelated column", @@ -649,9 +666,10 @@ func TestSchemaDiff(t *testing.T) { "create table t2 (id int primary key, ts timestamp, t1_id int, foreign key (t1_id) references t1 (id) on delete no action);", "create view v1 as select id from t1", }, - expectDiffs: 2, - expectDeps: 1, - entityOrder: []string{"t1", "t2"}, + expectDiffs: 2, + expectDeps: 1, + entityOrder: []string{"t1", "t2"}, + instantCapability: InstantDDLCapabilityImpossible, }, { name: "add FK, alter unrelated column", @@ -660,9 +678,10 @@ func TestSchemaDiff(t *testing.T) { "create table t2 (id int primary key, ts timestamp, t1_id int, foreign key (t1_id) references t1 (id) on delete no action);", "create view v1 as select id from t1", }, - expectDiffs: 2, - expectDeps: 1, - entityOrder: []string{"t1", "t2"}, + expectDiffs: 2, + expectDeps: 1, + entityOrder: []string{"t1", "t2"}, + instantCapability: InstantDDLCapabilityImpossible, }, { name: "add FK, alter referenced column", @@ -671,10 +690,11 @@ func TestSchemaDiff(t *testing.T) { "create table t2 (id int primary key, ts timestamp, t1_id bigint, foreign key (t1_id) references t1 (id) on delete no action);", "create view v1 as select id from t1", }, - expectDiffs: 2, - expectDeps: 1, - sequential: true, - entityOrder: []string{"t1", "t2"}, + expectDiffs: 2, + expectDeps: 1, + sequential: true, + entityOrder: []string{"t1", "t2"}, + instantCapability: InstantDDLCapabilityImpossible, }, { name: "add column. create FK table referencing new column", @@ -684,10 +704,11 @@ func TestSchemaDiff(t *testing.T) { "create view v1 as select id from t1", "create table t3 (id int primary key, ts timestamp, t1_p int, foreign key (t1_p) references t1 (p) on delete no action);", }, - expectDiffs: 2, - expectDeps: 1, - entityOrder: []string{"t1", "t3"}, - sequential: true, + expectDiffs: 2, + expectDeps: 1, + entityOrder: []string{"t1", "t3"}, + sequential: true, + instantCapability: InstantDDLCapabilityImpossible, }, { name: "add column. add FK referencing new column", @@ -696,10 +717,11 @@ func TestSchemaDiff(t *testing.T) { "create table t2 (id int primary key, ts timestamp, t1_p int, foreign key (t1_p) references t1 (p) on delete no action);", "create view v1 as select id from t1", }, - expectDiffs: 2, - expectDeps: 1, - sequential: true, - entityOrder: []string{"t1", "t2"}, + expectDiffs: 2, + expectDeps: 1, + sequential: true, + entityOrder: []string{"t1", "t2"}, + instantCapability: InstantDDLCapabilityImpossible, }, { name: "add column. add FK referencing new column, alphabetically desc", @@ -708,21 +730,24 @@ func TestSchemaDiff(t *testing.T) { "create table t2 (id int primary key, ts timestamp, p int, key p_idx (p));", "create view v1 as select id from t1", }, - expectDiffs: 2, - expectDeps: 1, - sequential: true, - entityOrder: []string{"t2", "t1"}, - }, { + expectDiffs: 2, + expectDeps: 1, + sequential: true, + entityOrder: []string{"t2", "t1"}, + instantCapability: InstantDDLCapabilityImpossible, + }, + { name: "add index on parent. add FK to index column", toQueries: []string{ "create table t1 (id int primary key, info int not null, key info_idx(info));", "create table t2 (id int primary key, ts timestamp, t1_info int not null, constraint parent_info_fk foreign key (t1_info) references t1 (info));", "create view v1 as select id from t1", }, - expectDiffs: 2, - expectDeps: 1, - sequential: true, - entityOrder: []string{"t1", "t2"}, + expectDiffs: 2, + expectDeps: 1, + sequential: true, + entityOrder: []string{"t1", "t2"}, + instantCapability: InstantDDLCapabilityImpossible, }, { name: "add index on parent with existing index. add FK to index column", @@ -736,10 +761,11 @@ func TestSchemaDiff(t *testing.T) { "create table t2 (id int primary key, ts timestamp, t1_info int not null, constraint parent_info_fk foreign key (t1_info) references t1 (info));", "create view v1 as select id from t1", }, - expectDiffs: 2, - expectDeps: 1, - sequential: false, - entityOrder: []string{"t1", "t2"}, + expectDiffs: 2, + expectDeps: 1, + sequential: false, + entityOrder: []string{"t1", "t2"}, + instantCapability: InstantDDLCapabilityImpossible, }, { name: "modify fk column types, fail", @@ -751,10 +777,11 @@ func TestSchemaDiff(t *testing.T) { "create table t1 (id bigint primary key);", "create table t2 (id int primary key, ts timestamp, t1_id bigint, foreign key (t1_id) references t1 (id) on delete no action);", }, - expectDiffs: 2, - expectDeps: 0, - sequential: false, - conflictingDiffs: 1, + expectDiffs: 2, + expectDeps: 0, + sequential: false, + conflictingDiffs: 1, + instantCapability: InstantDDLCapabilityImpossible, }, { name: "add hierarchical constraints", @@ -772,10 +799,11 @@ func TestSchemaDiff(t *testing.T) { "create table t4 (id int primary key, ref int, key ref_idx (ref), foreign key (ref) references t3 (id) on delete no action);", "create table t5 (id int primary key, ref int, key ref_idx (ref), foreign key (ref) references t1 (id) on delete no action);", }, - expectDiffs: 4, - expectDeps: 2, // t2<->t3, t3<->t4 - sequential: false, - entityOrder: []string{"t2", "t3", "t4", "t5"}, + expectDiffs: 4, + expectDeps: 2, // t2<->t3, t3<->t4 + sequential: false, + entityOrder: []string{"t2", "t3", "t4", "t5"}, + instantCapability: InstantDDLCapabilityImpossible, }, { name: "drop fk", @@ -784,10 +812,11 @@ func TestSchemaDiff(t *testing.T) { "create table t2 (id int primary key, ts timestamp, t1_id int, foreign key (t1_id) references t1 (id) on delete no action);", "create view v1 as select id from t1", }, - toQueries: createQueries, - expectDiffs: 1, - expectDeps: 0, - entityOrder: []string{"t2"}, + toQueries: createQueries, + expectDiffs: 1, + expectDeps: 0, + entityOrder: []string{"t2"}, + instantCapability: InstantDDLCapabilityImpossible, }, { name: "drop fk, drop table", @@ -798,9 +827,10 @@ func TestSchemaDiff(t *testing.T) { toQueries: []string{ "create table t2 (id int primary key, ts timestamp, t1_id int);", }, - expectDiffs: 2, - expectDeps: 1, - entityOrder: []string{"t2", "t1"}, + expectDiffs: 2, + expectDeps: 1, + entityOrder: []string{"t2", "t1"}, + instantCapability: InstantDDLCapabilityImpossible, }, { name: "drop fk, drop column", @@ -812,9 +842,10 @@ func TestSchemaDiff(t *testing.T) { "create table t1 (id int primary key, info int not null);", "create table t2 (id int primary key, ts timestamp, t1_p int);", }, - expectDiffs: 2, - expectDeps: 1, - entityOrder: []string{"t2", "t1"}, + expectDiffs: 2, + expectDeps: 1, + entityOrder: []string{"t2", "t1"}, + instantCapability: InstantDDLCapabilityImpossible, }, { name: "reverse fk", @@ -826,9 +857,10 @@ func TestSchemaDiff(t *testing.T) { "create table t1 (id int primary key, p int, key p_idx (p), foreign key (p) references t2 (p) on delete no action);", "create table t2 (id int primary key, p int, key p_idx (p));", }, - expectDiffs: 2, - expectDeps: 2, - entityOrder: []string{"t2", "t1"}, + expectDiffs: 2, + expectDeps: 2, + entityOrder: []string{"t2", "t1"}, + instantCapability: InstantDDLCapabilityImpossible, }, { name: "add and drop FK, add and drop column, impossible order", @@ -840,10 +872,11 @@ func TestSchemaDiff(t *testing.T) { "create table t1 (id int primary key, q int, key q_idx (q));", "create table t2 (id int primary key, q int, key q_idx (q), foreign key (q) references t1 (q) on delete no action);", }, - expectDiffs: 2, - expectDeps: 1, - sequential: true, - conflictingDiffs: 2, + expectDiffs: 2, + expectDeps: 1, + sequential: true, + conflictingDiffs: 2, + instantCapability: InstantDDLCapabilityImpossible, }, { name: "two identical foreign keys in table, drop one", @@ -855,9 +888,10 @@ func TestSchemaDiff(t *testing.T) { "create table parent (id int primary key)", "create table t1 (id int primary key, i int, key i_idex (i), constraint f1 foreign key (i) references parent(id))", }, - expectDiffs: 1, - expectDeps: 0, - entityOrder: []string{"t1"}, + expectDiffs: 1, + expectDeps: 0, + entityOrder: []string{"t1"}, + instantCapability: InstantDDLCapabilityImpossible, }, { name: "test", @@ -868,15 +902,18 @@ func TestSchemaDiff(t *testing.T) { "CREATE TABLE t1 (id bigint NOT NULL, name varchar(255), PRIMARY KEY (id), KEY idx_name (name))", "CREATE TABLE t3 (id bigint NOT NULL, name varchar(255), t1_id bigint, PRIMARY KEY (id), KEY t1_id (t1_id), KEY nameidx (name), CONSTRAINT t3_ibfk_1 FOREIGN KEY (t1_id) REFERENCES t1 (id) ON DELETE CASCADE ON UPDATE CASCADE, CONSTRAINT t3_ibfk_2 FOREIGN KEY (name) REFERENCES t1 (name) ON DELETE CASCADE ON UPDATE CASCADE)", }, - expectDiffs: 2, - expectDeps: 1, - sequential: true, - entityOrder: []string{"t1", "t3"}, + expectDiffs: 2, + expectDeps: 1, + sequential: true, + entityOrder: []string{"t1", "t3"}, + instantCapability: InstantDDLCapabilityImpossible, }, } - hints := &DiffHints{RangeRotationStrategy: RangeRotationDistinctStatements} + baseHints := &DiffHints{ + RangeRotationStrategy: RangeRotationDistinctStatements, + MySQLServerVersion: "8.0.32", + } env := NewTestEnv() - for _, tc := range tt { t.Run(tc.name, func(t *testing.T) { if tc.fromQueries == nil { @@ -890,7 +927,11 @@ func TestSchemaDiff(t *testing.T) { require.NoError(t, err) require.NotNil(t, toSchema) - schemaDiff, err := fromSchema.SchemaDiff(toSchema, hints) + hints := *baseHints + if tc.mysqlServerVersion != "" { + hints.MySQLServerVersion = tc.mysqlServerVersion + } + schemaDiff, err := fromSchema.SchemaDiff(toSchema, &hints) require.NoError(t, err) allDiffs := schemaDiff.UnorderedDiffs() @@ -942,9 +983,8 @@ func TestSchemaDiff(t *testing.T) { _, err := schemaDiff.r.ElementClass(s) require.NoError(t, err) } - capableOfInstantDDL, err := schemaDiff.CapableOfInstantDDL(ctx, capableOf) - require.NoError(t, err) - assert.Equal(t, tc.instantCapable, capableOfInstantDDL) + instantCapability := schemaDiff.InstantDDLCapability() + assert.Equal(t, tc.instantCapability, instantCapability) }) } diff --git a/go/vt/schemadiff/table.go b/go/vt/schemadiff/table.go index bcbe97a7740..3288ae24152 100644 --- a/go/vt/schemadiff/table.go +++ b/go/vt/schemadiff/table.go @@ -40,6 +40,7 @@ type AlterTableEntityDiff struct { canonicalStatementString string subsequentDiff *AlterTableEntityDiff + instantDDLCapability InstantDDLCapability } // IsEmpty implements EntityDiff @@ -123,6 +124,14 @@ func (d *AlterTableEntityDiff) addSubsequentDiff(diff *AlterTableEntityDiff) { } } +// InstantDDLCapability implements EntityDiff +func (d *AlterTableEntityDiff) InstantDDLCapability() InstantDDLCapability { + if d == nil { + return InstantDDLCapabilityUnknown + } + return d.instantDDLCapability +} + type CreateTableEntityDiff struct { to *CreateTableEntity createTable *sqlparser.CreateTable @@ -191,6 +200,11 @@ func (d *CreateTableEntityDiff) SubsequentDiff() EntityDiff { func (d *CreateTableEntityDiff) SetSubsequentDiff(EntityDiff) { } +// InstantDDLCapability implements EntityDiff +func (d *CreateTableEntityDiff) InstantDDLCapability() InstantDDLCapability { + return InstantDDLCapabilityIrrelevant +} + type DropTableEntityDiff struct { from *CreateTableEntity dropTable *sqlparser.DropTable @@ -259,6 +273,11 @@ func (d *DropTableEntityDiff) SubsequentDiff() EntityDiff { func (d *DropTableEntityDiff) SetSubsequentDiff(EntityDiff) { } +// InstantDDLCapability implements EntityDiff +func (d *DropTableEntityDiff) InstantDDLCapability() InstantDDLCapability { + return InstantDDLCapabilityIrrelevant +} + type RenameTableEntityDiff struct { from *CreateTableEntity to *CreateTableEntity @@ -328,6 +347,11 @@ func (d *RenameTableEntityDiff) SubsequentDiff() EntityDiff { func (d *RenameTableEntityDiff) SetSubsequentDiff(EntityDiff) { } +// InstantDDLCapability implements EntityDiff +func (d *RenameTableEntityDiff) InstantDDLCapability() InstantDDLCapability { + return InstantDDLCapabilityIrrelevant +} + // CreateTableEntity stands for a TABLE construct. It contains the table's CREATE statement. type CreateTableEntity struct { *sqlparser.CreateTable diff --git a/go/vt/schemadiff/types.go b/go/vt/schemadiff/types.go index 86e5a8d06bf..76dded320d1 100644 --- a/go/vt/schemadiff/types.go +++ b/go/vt/schemadiff/types.go @@ -20,6 +20,15 @@ import ( "vitess.io/vitess/go/vt/sqlparser" ) +type InstantDDLCapability int + +const ( + InstantDDLCapabilityUnknown InstantDDLCapability = iota + InstantDDLCapabilityIrrelevant + InstantDDLCapabilityImpossible + InstantDDLCapabilityPossible +) + // Entity stands for a database object we can diff: // - A table // - A view @@ -55,6 +64,8 @@ type EntityDiff interface { SubsequentDiff() EntityDiff // SetSubsequentDiff updates the existing subsequent diff to the given one SetSubsequentDiff(EntityDiff) + // InstantDDLCapability returns the ability of this diff to run with ALGORITHM=INSTANT + InstantDDLCapability() InstantDDLCapability } const ( @@ -120,6 +131,8 @@ type DiffHints struct { TableCharsetCollateStrategy int TableQualifierHint int AlterTableAlgorithmStrategy int + + MySQLServerVersion string // Used to determine specific capabilities such as INSTANT DDL support } const ( diff --git a/go/vt/schemadiff/view.go b/go/vt/schemadiff/view.go index 0810b2a3395..c28be710116 100644 --- a/go/vt/schemadiff/view.go +++ b/go/vt/schemadiff/view.go @@ -91,6 +91,11 @@ func (d *AlterViewEntityDiff) SubsequentDiff() EntityDiff { func (d *AlterViewEntityDiff) SetSubsequentDiff(EntityDiff) { } +// InstantDDLCapability implements EntityDiff +func (d *AlterViewEntityDiff) InstantDDLCapability() InstantDDLCapability { + return InstantDDLCapabilityIrrelevant +} + type CreateViewEntityDiff struct { createView *sqlparser.CreateView @@ -159,6 +164,11 @@ func (d *CreateViewEntityDiff) SubsequentDiff() EntityDiff { func (d *CreateViewEntityDiff) SetSubsequentDiff(EntityDiff) { } +// InstantDDLCapability implements EntityDiff +func (d *CreateViewEntityDiff) InstantDDLCapability() InstantDDLCapability { + return InstantDDLCapabilityIrrelevant +} + type DropViewEntityDiff struct { from *CreateViewEntity dropView *sqlparser.DropView @@ -227,6 +237,11 @@ func (d *DropViewEntityDiff) SubsequentDiff() EntityDiff { func (d *DropViewEntityDiff) SetSubsequentDiff(EntityDiff) { } +// InstantDDLCapability implements EntityDiff +func (d *DropViewEntityDiff) InstantDDLCapability() InstantDDLCapability { + return InstantDDLCapabilityIrrelevant +} + // CreateViewEntity stands for a VIEW construct. It contains the view's CREATE statement. type CreateViewEntity struct { *sqlparser.CreateView