From 6c09953f33a6c13180d6f39a9642010c6afb211d Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Tue, 21 Mar 2023 11:00:53 +0200 Subject: [PATCH 1/6] formalize ShowMigrations command Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/vt/sqlparser/ast.go | 7 ++++ go/vt/sqlparser/ast_clone.go | 15 ++++++++ go/vt/sqlparser/ast_copy_on_rewrite.go | 28 +++++++++++++++ go/vt/sqlparser/ast_equals.go | 24 +++++++++++++ go/vt/sqlparser/ast_format.go | 9 +++++ go/vt/sqlparser/ast_format_fast.go | 10 ++++++ go/vt/sqlparser/ast_rewrite.go | 36 +++++++++++++++++++ go/vt/sqlparser/ast_visit.go | 19 ++++++++++ go/vt/sqlparser/cached_size.go | 14 ++++++++ go/vt/sqlparser/sql.go | 2 +- go/vt/sqlparser/sql.y | 2 +- go/vt/vtgate/planbuilder/builder.go | 2 ++ go/vt/vtgate/planbuilder/migration.go | 50 ++++++++++++++++++++++++++ go/vt/vtgate/planbuilder/show.go | 48 ------------------------- 14 files changed, 216 insertions(+), 50 deletions(-) diff --git a/go/vt/sqlparser/ast.go b/go/vt/sqlparser/ast.go index 9a1f717b1af..e313085b574 100644 --- a/go/vt/sqlparser/ast.go +++ b/go/vt/sqlparser/ast.go @@ -443,6 +443,12 @@ type ( AutoIncSpec *AutoIncSpec } + // ShowMigrations represents a SHOW VITESS_MIGRATIONS statement + ShowMigrations struct { + DbName IdentifierCS + Filter *ShowFilter + } + // ShowMigrationLogs represents a SHOW VITESS_MIGRATION '' LOGS statement ShowMigrationLogs struct { UUID string @@ -731,6 +737,7 @@ func (*AlterTable) iStatement() {} func (*AlterVschema) iStatement() {} func (*AlterMigration) iStatement() {} func (*RevertMigration) iStatement() {} +func (*ShowMigrations) iStatement() {} func (*ShowMigrationLogs) iStatement() {} func (*ShowThrottledApps) iStatement() {} func (*ShowThrottlerStatus) iStatement() {} diff --git a/go/vt/sqlparser/ast_clone.go b/go/vt/sqlparser/ast_clone.go index 2473fe66511..d82fb3fb7e3 100644 --- a/go/vt/sqlparser/ast_clone.go +++ b/go/vt/sqlparser/ast_clone.go @@ -419,6 +419,8 @@ func CloneSQLNode(in SQLNode) SQLNode { return CloneRefOfShowFilter(in) case *ShowMigrationLogs: return CloneRefOfShowMigrationLogs(in) + case *ShowMigrations: + return CloneRefOfShowMigrations(in) case *ShowOther: return CloneRefOfShowOther(in) case *ShowThrottledApps: @@ -2628,6 +2630,17 @@ func CloneRefOfShowMigrationLogs(n *ShowMigrationLogs) *ShowMigrationLogs { return &out } +// CloneRefOfShowMigrations creates a deep clone of the input. +func CloneRefOfShowMigrations(n *ShowMigrations) *ShowMigrations { + if n == nil { + return nil + } + out := *n + out.DbName = CloneIdentifierCS(n.DbName) + out.Filter = CloneRefOfShowFilter(n.Filter) + return &out +} + // CloneRefOfShowOther creates a deep clone of the input. func CloneRefOfShowOther(n *ShowOther) *ShowOther { if n == nil { @@ -3923,6 +3936,8 @@ func CloneStatement(in Statement) Statement { return CloneRefOfShow(in) case *ShowMigrationLogs: return CloneRefOfShowMigrationLogs(in) + case *ShowMigrations: + return CloneRefOfShowMigrations(in) case *ShowThrottledApps: return CloneRefOfShowThrottledApps(in) case *ShowThrottlerStatus: diff --git a/go/vt/sqlparser/ast_copy_on_rewrite.go b/go/vt/sqlparser/ast_copy_on_rewrite.go index 58a6545b24b..b48d870aea6 100644 --- a/go/vt/sqlparser/ast_copy_on_rewrite.go +++ b/go/vt/sqlparser/ast_copy_on_rewrite.go @@ -418,6 +418,8 @@ func (c *cow) copyOnRewriteSQLNode(n SQLNode, parent SQLNode) (out SQLNode, chan return c.copyOnRewriteRefOfShowFilter(n, parent) case *ShowMigrationLogs: return c.copyOnRewriteRefOfShowMigrationLogs(n, parent) + case *ShowMigrations: + return c.copyOnRewriteRefOfShowMigrations(n, parent) case *ShowOther: return c.copyOnRewriteRefOfShowOther(n, parent) case *ShowThrottledApps: @@ -4980,6 +4982,30 @@ func (c *cow) copyOnRewriteRefOfShowMigrationLogs(n *ShowMigrationLogs, parent S } return } +func (c *cow) copyOnRewriteRefOfShowMigrations(n *ShowMigrations, parent SQLNode) (out SQLNode, changed bool) { + if n == nil || c.cursor.stop { + return n, false + } + out = n + if c.pre == nil || c.pre(n, parent) { + _DbName, changedDbName := c.copyOnRewriteIdentifierCS(n.DbName, n) + _Filter, changedFilter := c.copyOnRewriteRefOfShowFilter(n.Filter, n) + if changedDbName || changedFilter { + res := *n + res.DbName, _ = _DbName.(IdentifierCS) + res.Filter, _ = _Filter.(*ShowFilter) + out = &res + if c.cloned != nil { + c.cloned(n, out) + } + changed = true + } + } + if c.post != nil { + out, changed = c.postVisit(out, parent, changed) + } + return +} func (c *cow) copyOnRewriteRefOfShowOther(n *ShowOther, parent SQLNode) (out SQLNode, changed bool) { if n == nil || c.cursor.stop { return n, false @@ -6895,6 +6921,8 @@ func (c *cow) copyOnRewriteStatement(n Statement, parent SQLNode) (out SQLNode, return c.copyOnRewriteRefOfShow(n, parent) case *ShowMigrationLogs: return c.copyOnRewriteRefOfShowMigrationLogs(n, parent) + case *ShowMigrations: + return c.copyOnRewriteRefOfShowMigrations(n, parent) case *ShowThrottledApps: return c.copyOnRewriteRefOfShowThrottledApps(n, parent) case *ShowThrottlerStatus: diff --git a/go/vt/sqlparser/ast_equals.go b/go/vt/sqlparser/ast_equals.go index 44241e8388b..c66858d49f0 100644 --- a/go/vt/sqlparser/ast_equals.go +++ b/go/vt/sqlparser/ast_equals.go @@ -1214,6 +1214,12 @@ func (cmp *Comparator) SQLNode(inA, inB SQLNode) bool { return false } return cmp.RefOfShowMigrationLogs(a, b) + case *ShowMigrations: + b, ok := inB.(*ShowMigrations) + if !ok { + return false + } + return cmp.RefOfShowMigrations(a, b) case *ShowOther: b, ok := inB.(*ShowOther) if !ok { @@ -3977,6 +3983,18 @@ func (cmp *Comparator) RefOfShowMigrationLogs(a, b *ShowMigrationLogs) bool { cmp.RefOfParsedComments(a.Comments, b.Comments) } +// RefOfShowMigrations does deep equals between the two objects. +func (cmp *Comparator) RefOfShowMigrations(a, b *ShowMigrations) bool { + if a == b { + return true + } + if a == nil || b == nil { + return false + } + return cmp.IdentifierCS(a.DbName, b.DbName) && + cmp.RefOfShowFilter(a.Filter, b.Filter) +} + // RefOfShowOther does deep equals between the two objects. func (cmp *Comparator) RefOfShowOther(a, b *ShowOther) bool { if a == b { @@ -6480,6 +6498,12 @@ func (cmp *Comparator) Statement(inA, inB Statement) bool { return false } return cmp.RefOfShowMigrationLogs(a, b) + case *ShowMigrations: + b, ok := inB.(*ShowMigrations) + if !ok { + return false + } + return cmp.RefOfShowMigrations(a, b) case *ShowThrottledApps: b, ok := inB.(*ShowThrottledApps) if !ok { diff --git a/go/vt/sqlparser/ast_format.go b/go/vt/sqlparser/ast_format.go index e867bf0f235..38d10c9b555 100644 --- a/go/vt/sqlparser/ast_format.go +++ b/go/vt/sqlparser/ast_format.go @@ -300,6 +300,15 @@ func (node *RevertMigration) Format(buf *TrackedBuffer) { buf.astPrintf(node, "revert %vvitess_migration '%#s'", node.Comments, node.UUID) } +// Format formats the node. +func (node *ShowMigrations) Format(buf *TrackedBuffer) { + buf.astPrintf(node, "show vitess_migrations") + if !node.DbName.IsEmpty() { + buf.astPrintf(node, " from %v", node.DbName) + } + buf.astPrintf(node, "%v", node.Filter) +} + // Format formats the node. func (node *ShowMigrationLogs) Format(buf *TrackedBuffer) { buf.astPrintf(node, "show vitess_migration '%#s' logs", node.UUID) diff --git a/go/vt/sqlparser/ast_format_fast.go b/go/vt/sqlparser/ast_format_fast.go index 3b2a3ce6762..c4b898efeaf 100644 --- a/go/vt/sqlparser/ast_format_fast.go +++ b/go/vt/sqlparser/ast_format_fast.go @@ -433,6 +433,16 @@ func (node *RevertMigration) formatFast(buf *TrackedBuffer) { buf.WriteByte('\'') } +// formatFast formats the node. +func (node *ShowMigrations) formatFast(buf *TrackedBuffer) { + buf.WriteString("show vitess_migrations") + if !node.DbName.IsEmpty() { + buf.WriteString(" from ") + node.DbName.formatFast(buf) + } + node.Filter.formatFast(buf) +} + // formatFast formats the node. func (node *ShowMigrationLogs) formatFast(buf *TrackedBuffer) { buf.WriteString("show vitess_migration '") diff --git a/go/vt/sqlparser/ast_rewrite.go b/go/vt/sqlparser/ast_rewrite.go index 404db8301a9..20d4efac7c0 100644 --- a/go/vt/sqlparser/ast_rewrite.go +++ b/go/vt/sqlparser/ast_rewrite.go @@ -418,6 +418,8 @@ func (a *application) rewriteSQLNode(parent SQLNode, node SQLNode, replacer repl return a.rewriteRefOfShowFilter(parent, node, replacer) case *ShowMigrationLogs: return a.rewriteRefOfShowMigrationLogs(parent, node, replacer) + case *ShowMigrations: + return a.rewriteRefOfShowMigrations(parent, node, replacer) case *ShowOther: return a.rewriteRefOfShowOther(parent, node, replacer) case *ShowThrottledApps: @@ -6713,6 +6715,38 @@ func (a *application) rewriteRefOfShowMigrationLogs(parent SQLNode, node *ShowMi } return true } +func (a *application) rewriteRefOfShowMigrations(parent SQLNode, node *ShowMigrations, replacer replacerFunc) bool { + if node == nil { + return true + } + if a.pre != nil { + a.cur.replacer = replacer + a.cur.parent = parent + a.cur.node = node + if !a.pre(&a.cur) { + return true + } + } + if !a.rewriteIdentifierCS(node, node.DbName, func(newNode, parent SQLNode) { + parent.(*ShowMigrations).DbName = newNode.(IdentifierCS) + }) { + return false + } + if !a.rewriteRefOfShowFilter(node, node.Filter, func(newNode, parent SQLNode) { + parent.(*ShowMigrations).Filter = newNode.(*ShowFilter) + }) { + return false + } + if a.post != nil { + a.cur.replacer = replacer + a.cur.parent = parent + a.cur.node = node + if !a.post(&a.cur) { + return false + } + } + return true +} func (a *application) rewriteRefOfShowOther(parent SQLNode, node *ShowOther, replacer replacerFunc) bool { if node == nil { return true @@ -9122,6 +9156,8 @@ func (a *application) rewriteStatement(parent SQLNode, node Statement, replacer return a.rewriteRefOfShow(parent, node, replacer) case *ShowMigrationLogs: return a.rewriteRefOfShowMigrationLogs(parent, node, replacer) + case *ShowMigrations: + return a.rewriteRefOfShowMigrations(parent, node, replacer) case *ShowThrottledApps: return a.rewriteRefOfShowThrottledApps(parent, node, replacer) case *ShowThrottlerStatus: diff --git a/go/vt/sqlparser/ast_visit.go b/go/vt/sqlparser/ast_visit.go index 5a15a5650af..fee737b9fe7 100644 --- a/go/vt/sqlparser/ast_visit.go +++ b/go/vt/sqlparser/ast_visit.go @@ -418,6 +418,8 @@ func VisitSQLNode(in SQLNode, f Visit) error { return VisitRefOfShowFilter(in, f) case *ShowMigrationLogs: return VisitRefOfShowMigrationLogs(in, f) + case *ShowMigrations: + return VisitRefOfShowMigrations(in, f) case *ShowOther: return VisitRefOfShowOther(in, f) case *ShowThrottledApps: @@ -3349,6 +3351,21 @@ func VisitRefOfShowMigrationLogs(in *ShowMigrationLogs, f Visit) error { } return nil } +func VisitRefOfShowMigrations(in *ShowMigrations, f Visit) error { + if in == nil { + return nil + } + if cont, err := f(in); err != nil || !cont { + return err + } + if err := VisitIdentifierCS(in.DbName, f); err != nil { + return err + } + if err := VisitRefOfShowFilter(in.Filter, f); err != nil { + return err + } + return nil +} func VisitRefOfShowOther(in *ShowOther, f Visit) error { if in == nil { return nil @@ -4798,6 +4815,8 @@ func VisitStatement(in Statement, f Visit) error { return VisitRefOfShow(in, f) case *ShowMigrationLogs: return VisitRefOfShowMigrationLogs(in, f) + case *ShowMigrations: + return VisitRefOfShowMigrations(in, f) case *ShowThrottledApps: return VisitRefOfShowThrottledApps(in, f) case *ShowThrottlerStatus: diff --git a/go/vt/sqlparser/cached_size.go b/go/vt/sqlparser/cached_size.go index 6a9ad76728f..d46337112aa 100644 --- a/go/vt/sqlparser/cached_size.go +++ b/go/vt/sqlparser/cached_size.go @@ -3483,6 +3483,20 @@ func (cached *ShowMigrationLogs) CachedSize(alloc bool) int64 { size += cached.Comments.CachedSize(true) return size } +func (cached *ShowMigrations) CachedSize(alloc bool) int64 { + if cached == nil { + return int64(0) + } + size := int64(0) + if alloc { + size += int64(24) + } + // field DbName vitess.io/vitess/go/vt/sqlparser.IdentifierCS + size += cached.DbName.CachedSize(false) + // field Filter *vitess.io/vitess/go/vt/sqlparser.ShowFilter + size += cached.Filter.CachedSize(true) + return size +} func (cached *ShowOther) CachedSize(alloc bool) int64 { if cached == nil { return int64(0) diff --git a/go/vt/sqlparser/sql.go b/go/vt/sqlparser/sql.go index ab945928e4e..a8b2db14861 100644 --- a/go/vt/sqlparser/sql.go +++ b/go/vt/sqlparser/sql.go @@ -14469,7 +14469,7 @@ yydefault: var yyLOCAL Statement //line sql.y:4095 { - yyLOCAL = &Show{&ShowBasic{Command: VitessMigrations, Filter: yyDollar[4].showFilterUnion(), DbName: yyDollar[3].identifierCS}} + yyLOCAL = &ShowMigrations{Filter: yyDollar[4].showFilterUnion(), DbName: yyDollar[3].identifierCS} } yyVAL.union = yyLOCAL case 749: diff --git a/go/vt/sqlparser/sql.y b/go/vt/sqlparser/sql.y index 1d0f5ac57fd..168fd0327bf 100644 --- a/go/vt/sqlparser/sql.y +++ b/go/vt/sqlparser/sql.y @@ -4093,7 +4093,7 @@ show_statement: } | SHOW VITESS_MIGRATIONS from_database_opt like_or_where_opt { - $$ = &Show{&ShowBasic{Command: VitessMigrations, Filter: $4, DbName: $3}} + $$ = &ShowMigrations{Filter: $4, DbName: $3} } | SHOW VITESS_MIGRATION STRING LOGS { diff --git a/go/vt/vtgate/planbuilder/builder.go b/go/vt/vtgate/planbuilder/builder.go index 1181c9b2c8b..a46cbf7c6e3 100644 --- a/go/vt/vtgate/planbuilder/builder.go +++ b/go/vt/vtgate/planbuilder/builder.go @@ -236,6 +236,8 @@ func createInstructionFor(query string, stmt sqlparser.Statement, reservedVars * return buildAlterMigrationPlan(query, vschema, enableOnlineDDL) case *sqlparser.RevertMigration: return buildRevertMigrationPlan(query, stmt, vschema, enableOnlineDDL) + case *sqlparser.ShowMigrations: + return buildShowVMigrationsPlan(stmt, vschema) case *sqlparser.ShowMigrationLogs: return buildShowMigrationLogsPlan(query, vschema, enableOnlineDDL) case *sqlparser.ShowThrottledApps: diff --git a/go/vt/vtgate/planbuilder/migration.go b/go/vt/vtgate/planbuilder/migration.go index 468c86d3ffb..236eedc44d4 100644 --- a/go/vt/vtgate/planbuilder/migration.go +++ b/go/vt/vtgate/planbuilder/migration.go @@ -110,3 +110,53 @@ func buildShowMigrationLogsPlan(query string, vschema plancontext.VSchema, enabl } return newPlanResult(send), nil } + +// buildShowVMigrationsPlan serves `SHOW VITESS_MIGRATIONS ...` queries. +// It invokes queries on the sidecar database's schema_migrations table +// on all PRIMARY tablets in the keyspace's shards. +func buildShowVMigrationsPlan(show *sqlparser.ShowMigrations, vschema plancontext.VSchema) (*planResult, error) { + dest, ks, tabletType, err := vschema.TargetDestination(show.DbName.String()) + if err != nil { + return nil, err + } + if ks == nil { + return nil, vterrors.VT09005() + } + + if tabletType != topodatapb.TabletType_PRIMARY { + return nil, vterrors.VT09006("SHOW") + } + + if dest == nil { + dest = key.DestinationAllShards{} + } + + send := &engine.Send{ + Keyspace: ks, + TargetDestination: dest, + Query: sqlparser.String(show), + } + return newPlanResult(send), nil + + // sidecarDBID, err := sidecardb.GetIdentifierForKeyspace(ks.Name) + // if err != nil { + // log.Errorf("Failed to read sidecar database identifier for keyspace %q from the cache: %v", ks.Name, err) + // return nil, vterrors.VT14005(ks.Name) + // } + + // sql := sqlparser.BuildParsedQuery("SELECT * FROM %s.schema_migrations", sidecarDBID).Query + + // if show.Filter != nil { + // if show.Filter.Filter != nil { + // sql += fmt.Sprintf(" where %s", sqlparser.String(show.Filter.Filter)) + // } else if show.Filter.Like != "" { + // lit := sqlparser.String(sqlparser.NewStrLiteral(show.Filter.Like)) + // sql += fmt.Sprintf(" where migration_uuid LIKE %s OR migration_context LIKE %s OR migration_status LIKE %s", lit, lit, lit) + // } + // } + // return &engine.Send{ + // Keyspace: ks, + // TargetDestination: dest, + // Query: sql, + // }, nil +} diff --git a/go/vt/vtgate/planbuilder/show.go b/go/vt/vtgate/planbuilder/show.go index 28e7e707848..4242c62baa3 100644 --- a/go/vt/vtgate/planbuilder/show.go +++ b/go/vt/vtgate/planbuilder/show.go @@ -25,11 +25,8 @@ import ( "vitess.io/vitess/go/mysql/collations" "vitess.io/vitess/go/sqltypes" "vitess.io/vitess/go/vt/key" - "vitess.io/vitess/go/vt/log" querypb "vitess.io/vitess/go/vt/proto/query" - topodatapb "vitess.io/vitess/go/vt/proto/topodata" vschemapb "vitess.io/vitess/go/vt/proto/vschema" - "vitess.io/vitess/go/vt/sidecardb" "vitess.io/vitess/go/vt/sqlparser" "vitess.io/vitess/go/vt/vterrors" "vitess.io/vitess/go/vt/vtgate/engine" @@ -98,8 +95,6 @@ func buildShowBasicPlan(show *sqlparser.ShowBasic, vschema plancontext.VSchema) return buildPlanWithDB(show, vschema) case sqlparser.StatusGlobal, sqlparser.StatusSession: return buildSendAnywherePlan(show, vschema) - case sqlparser.VitessMigrations: - return buildShowVMigrationsPlan(show, vschema) case sqlparser.VGtidExecGlobal: return buildShowVGtidPlan(show, vschema) case sqlparser.GtidExecGlobal: @@ -247,49 +242,6 @@ func buildDBPlan(show *sqlparser.ShowBasic, vschema plancontext.VSchema) (engine return engine.NewRowsPrimitive(rows, buildVarCharFields("Database")), nil } -// buildShowVMigrationsPlan serves `SHOW VITESS_MIGRATIONS ...` queries. -// It invokes queries on the sidecar database's schema_migrations table -// on all PRIMARY tablets in the keyspace's shards. -func buildShowVMigrationsPlan(show *sqlparser.ShowBasic, vschema plancontext.VSchema) (engine.Primitive, error) { - dest, ks, tabletType, err := vschema.TargetDestination(show.DbName.String()) - if err != nil { - return nil, err - } - if ks == nil { - return nil, vterrors.VT09005() - } - - if tabletType != topodatapb.TabletType_PRIMARY { - return nil, vterrors.VT09006("SHOW") - } - - if dest == nil { - dest = key.DestinationAllShards{} - } - - sidecarDBID, err := sidecardb.GetIdentifierForKeyspace(ks.Name) - if err != nil { - log.Errorf("Failed to read sidecar database identifier for keyspace %q from the cache: %v", ks.Name, err) - return nil, vterrors.VT14005(ks.Name) - } - - sql := sqlparser.BuildParsedQuery("SELECT * FROM %s.schema_migrations", sidecarDBID).Query - - if show.Filter != nil { - if show.Filter.Filter != nil { - sql += fmt.Sprintf(" where %s", sqlparser.String(show.Filter.Filter)) - } else if show.Filter.Like != "" { - lit := sqlparser.String(sqlparser.NewStrLiteral(show.Filter.Like)) - sql += fmt.Sprintf(" where migration_uuid LIKE %s OR migration_context LIKE %s OR migration_status LIKE %s", lit, lit, lit) - } - } - return &engine.Send{ - Keyspace: ks, - TargetDestination: dest, - Query: sql, - }, nil -} - func buildPlanWithDB(show *sqlparser.ShowBasic, vschema plancontext.VSchema) (engine.Primitive, error) { dbName := show.DbName dbDestination := show.DbName.String() From ec5e8714908f2ad09cb12ec176d25dad4215de9c Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Tue, 21 Mar 2023 12:29:39 +0200 Subject: [PATCH 2/6] vtgate supports both new and old execution methods for ShowMigrations Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/vt/vtgate/engine/show_migrations.go | 136 +++++++++++++++++++++++++ go/vt/vtgate/planbuilder/migration.go | 31 ++---- 2 files changed, 143 insertions(+), 24 deletions(-) create mode 100644 go/vt/vtgate/engine/show_migrations.go diff --git a/go/vt/vtgate/engine/show_migrations.go b/go/vt/vtgate/engine/show_migrations.go new file mode 100644 index 00000000000..1b80efc23ab --- /dev/null +++ b/go/vt/vtgate/engine/show_migrations.go @@ -0,0 +1,136 @@ +/* +Copyright 2023 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 engine + +import ( + "context" + "fmt" + + "vitess.io/vitess/go/sqltypes" + "vitess.io/vitess/go/vt/key" + "vitess.io/vitess/go/vt/log" + querypb "vitess.io/vitess/go/vt/proto/query" + vtrpcpb "vitess.io/vitess/go/vt/proto/vtrpc" + "vitess.io/vitess/go/vt/sidecardb" + "vitess.io/vitess/go/vt/sqlparser" + "vitess.io/vitess/go/vt/vterrors" + "vitess.io/vitess/go/vt/vtgate/vindexes" +) + +var _ Primitive = (*ShowMigrations)(nil) + +// TODO: (shlomi) remove in v18 +// Backwards compatibility: this is introduced in v17, But vttablets may still be on v16 and +// are not aware of this new statement. The logic in this primitive supports both new and old options. +// In v18 this struct should be removed and replaced with a simple Send (see buildShowVMigrationsPlan()) + +// ShowMigrations represents the instructions to perform an online schema change via vtctld +type ShowMigrations struct { + Keyspace *vindexes.Keyspace + Stmt *sqlparser.ShowMigrations + Query string + TargetDestination key.Destination + + noTxNeeded + + noInputs +} + +func (v *ShowMigrations) description() PrimitiveDescription { + return PrimitiveDescription{ + OperatorType: "ShowMigrations", + Keyspace: v.Keyspace, + Other: map[string]any{ + "query": v.Query, + }, + } +} + +// RouteType implements the Primitive interface +func (v *ShowMigrations) RouteType() string { + return "ShowMigrations" +} + +// GetKeyspaceName implements the Primitive interface +func (v *ShowMigrations) GetKeyspaceName() string { + return v.Keyspace.Name +} + +// GetTableName implements the Primitive interface +func (v *ShowMigrations) GetTableName() string { + return "" +} + +// TryExecute implements the Primitive interface +func (v *ShowMigrations) TryExecute(ctx context.Context, vcursor VCursor, bindVars map[string]*querypb.BindVariable, wantfields bool) (result *sqltypes.Result, err error) { + s := Send{ + Keyspace: v.Keyspace, + TargetDestination: v.TargetDestination, + Query: v.Query, + IsDML: false, + SingleShardOnly: false, + } + result, err = vcursor.ExecutePrimitive(ctx, &s, bindVars, wantfields) + if err == nil { + return result, nil + } + // TODO: (shlomi) remove in v18 + // Backwards compatibility: in v17 we introduce ShowMigrations ast statement. But vttablets may still be on v16 and + // are not aware of this new statement. In such case they return a vtrpcpb.Code_INVALID_ARGUMENT. + // If we intercept this return code, we fall back to a simple Send() with a generated query. + if vterrors.Code(err) != vtrpcpb.Code_INVALID_ARGUMENT { + return result, err + } + // Old method: construct a query here. Remove in v18. + sidecarDBID, err := sidecardb.GetIdentifierForKeyspace(v.GetKeyspaceName()) + if err != nil { + log.Errorf("Failed to read sidecar database identifier for keyspace %q from the cache: %v", v.GetKeyspaceName(), err) + return nil, vterrors.VT14005(v.GetKeyspaceName()) + } + + sql := sqlparser.BuildParsedQuery("SELECT * FROM %s.schema_migrations", sidecarDBID).Query + + if v.Stmt.Filter != nil { + if v.Stmt.Filter.Filter != nil { + sql += fmt.Sprintf(" where %s", sqlparser.String(v.Stmt.Filter.Filter)) + } else if v.Stmt.Filter.Like != "" { + lit := sqlparser.String(sqlparser.NewStrLiteral(v.Stmt.Filter.Like)) + sql += fmt.Sprintf(" where migration_uuid LIKE %s OR migration_context LIKE %s OR migration_status LIKE %s", lit, lit, lit) + } + } + s = Send{ + Keyspace: v.Keyspace, + TargetDestination: v.TargetDestination, + Query: sql, + } + result, err = vcursor.ExecutePrimitive(ctx, &s, bindVars, wantfields) + return result, err +} + +// TryStreamExecute implements the Primitive interface +func (v *ShowMigrations) TryStreamExecute(ctx context.Context, vcursor VCursor, bindVars map[string]*querypb.BindVariable, wantfields bool, callback func(*sqltypes.Result) error) error { + results, err := v.TryExecute(ctx, vcursor, bindVars, wantfields) + if err != nil { + return err + } + return callback(results) +} + +// GetFields implements the Primitive interface +func (v *ShowMigrations) GetFields(ctx context.Context, vcursor VCursor, bindVars map[string]*querypb.BindVariable) (*sqltypes.Result, error) { + return nil, vterrors.Errorf(vtrpcpb.Code_INTERNAL, "[BUG] GetFields is not reachable") +} diff --git a/go/vt/vtgate/planbuilder/migration.go b/go/vt/vtgate/planbuilder/migration.go index 236eedc44d4..339cb3f0eac 100644 --- a/go/vt/vtgate/planbuilder/migration.go +++ b/go/vt/vtgate/planbuilder/migration.go @@ -131,32 +131,15 @@ func buildShowVMigrationsPlan(show *sqlparser.ShowMigrations, vschema plancontex dest = key.DestinationAllShards{} } - send := &engine.Send{ + // v17 introduces ShowMigrations ast, but vttablets may still be on v16 and not know this statement. + // For this reason, we create a new primitive, `engine.ShowMigrations` that supports both new and + // old methods. + // TODO: (shlomi) in v18, remove engine.ShowMigrations and use a simple Send. + prim := &engine.ShowMigrations{ Keyspace: ks, TargetDestination: dest, + Stmt: show, Query: sqlparser.String(show), } - return newPlanResult(send), nil - - // sidecarDBID, err := sidecardb.GetIdentifierForKeyspace(ks.Name) - // if err != nil { - // log.Errorf("Failed to read sidecar database identifier for keyspace %q from the cache: %v", ks.Name, err) - // return nil, vterrors.VT14005(ks.Name) - // } - - // sql := sqlparser.BuildParsedQuery("SELECT * FROM %s.schema_migrations", sidecarDBID).Query - - // if show.Filter != nil { - // if show.Filter.Filter != nil { - // sql += fmt.Sprintf(" where %s", sqlparser.String(show.Filter.Filter)) - // } else if show.Filter.Like != "" { - // lit := sqlparser.String(sqlparser.NewStrLiteral(show.Filter.Like)) - // sql += fmt.Sprintf(" where migration_uuid LIKE %s OR migration_context LIKE %s OR migration_status LIKE %s", lit, lit, lit) - // } - // } - // return &engine.Send{ - // Keyspace: ks, - // TargetDestination: dest, - // Query: sql, - // }, nil + return newPlanResult(prim), nil } From a6547115c4f49594bd942102a99b9f9a981793a6 Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Tue, 21 Mar 2023 12:48:56 +0200 Subject: [PATCH 3/6] support ShowMigrations in query executor and OnlineDDL executor Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/vt/vttablet/onlineddl/executor.go | 23 +++++++++++++++++++ go/vt/vttablet/onlineddl/schema.go | 4 ++++ .../tabletserver/planbuilder/permission.go | 1 + .../vttablet/tabletserver/planbuilder/plan.go | 4 ++++ go/vt/vttablet/tabletserver/query_executor.go | 9 ++++++++ 5 files changed, 41 insertions(+) diff --git a/go/vt/vttablet/onlineddl/executor.go b/go/vt/vttablet/onlineddl/executor.go index 048b4e7ce62..eef8578a721 100644 --- a/go/vt/vttablet/onlineddl/executor.go +++ b/go/vt/vttablet/onlineddl/executor.go @@ -4693,6 +4693,29 @@ func (e *Executor) SubmitMigration( return result, nil } +// ShowMigrations shows migrations, optionally filtered by a condition +func (e *Executor) ShowMigrations(ctx context.Context, stmt *sqlparser.ShowMigrations) (result *sqltypes.Result, err error) { + if atomic.LoadInt64(&e.isOpen) == 0 { + return nil, vterrors.New(vtrpcpb.Code_FAILED_PRECONDITION, "online ddl is disabled") + } + + whereExpr := "" + if stmt.Filter != nil { + if stmt.Filter.Filter != nil { + whereExpr = fmt.Sprintf(" where %s", sqlparser.String(stmt.Filter.Filter)) + } else if stmt.Filter.Like != "" { + lit := sqlparser.String(sqlparser.NewStrLiteral(stmt.Filter.Like)) + whereExpr = fmt.Sprintf(" where migration_uuid LIKE %s OR migration_context LIKE %s OR migration_status LIKE %s", lit, lit, lit) + } + } + + e.migrationMutex.Lock() + defer e.migrationMutex.Unlock() + query := sqlparser.BuildParsedQuery(sqlShowMigrationsWhere, whereExpr).Query + result, err = e.execQuery(ctx, query) + return result, err +} + // ShowMigrationLogs reads the migration log for a given migration func (e *Executor) ShowMigrationLogs(ctx context.Context, stmt *sqlparser.ShowMigrationLogs) (result *sqltypes.Result, err error) { if atomic.LoadInt64(&e.isOpen) == 0 { diff --git a/go/vt/vttablet/onlineddl/schema.go b/go/vt/vttablet/onlineddl/schema.go index 90babe05b20..c35591fd5a9 100644 --- a/go/vt/vttablet/onlineddl/schema.go +++ b/go/vt/vttablet/onlineddl/schema.go @@ -359,6 +359,10 @@ const ( AND cleanup_timestamp IS NULL AND completed_timestamp IS NULL ` + sqlShowMigrationsWhere = `SELECT * + FROM _vt.schema_migrations + %s + ` sqlSelectMigration = `SELECT id, migration_uuid, diff --git a/go/vt/vttablet/tabletserver/planbuilder/permission.go b/go/vt/vttablet/tabletserver/planbuilder/permission.go index d68ff43a152..d37de27c13e 100644 --- a/go/vt/vttablet/tabletserver/planbuilder/permission.go +++ b/go/vt/vttablet/tabletserver/planbuilder/permission.go @@ -54,6 +54,7 @@ func BuildPermissions(stmt sqlparser.Statement) []Permission { case *sqlparser.AlterMigration, *sqlparser.RevertMigration, + *sqlparser.ShowMigrations, *sqlparser.ShowMigrationLogs, *sqlparser.ShowThrottledApps, *sqlparser.ShowThrottlerStatus: diff --git a/go/vt/vttablet/tabletserver/planbuilder/plan.go b/go/vt/vttablet/tabletserver/planbuilder/plan.go index 603ea455ac7..c2bad33a5c5 100644 --- a/go/vt/vttablet/tabletserver/planbuilder/plan.go +++ b/go/vt/vttablet/tabletserver/planbuilder/plan.go @@ -76,6 +76,7 @@ const ( PlanCallProc PlanAlterMigration PlanRevertMigration + PlanShowMigrations PlanShowMigrationLogs PlanShowThrottledApps PlanShowThrottlerStatus @@ -112,6 +113,7 @@ var planName = []string{ "CallProcedure", "AlterMigration", "RevertMigration", + "ShowMigrations", "ShowMigrationLogs", "ShowThrottledApps", "ShowThrottlerStatus", @@ -225,6 +227,8 @@ func Build(statement sqlparser.Statement, tables map[string]*schema.Table, dbNam plan, err = &Plan{PlanID: PlanAlterMigration, FullStmt: stmt}, nil case *sqlparser.RevertMigration: plan, err = &Plan{PlanID: PlanRevertMigration, FullStmt: stmt}, nil + case *sqlparser.ShowMigrations: + plan, err = &Plan{PlanID: PlanShowMigrations, FullStmt: stmt}, nil case *sqlparser.ShowMigrationLogs: plan, err = &Plan{PlanID: PlanShowMigrationLogs, FullStmt: stmt}, nil case *sqlparser.ShowThrottledApps: diff --git a/go/vt/vttablet/tabletserver/query_executor.go b/go/vt/vttablet/tabletserver/query_executor.go index e342b2b6b18..3161ab6534c 100644 --- a/go/vt/vttablet/tabletserver/query_executor.go +++ b/go/vt/vttablet/tabletserver/query_executor.go @@ -204,6 +204,8 @@ func (qre *QueryExecutor) Execute() (reply *sqltypes.Result, err error) { return qre.execAlterMigration() case p.PlanRevertMigration: return qre.execRevertMigration() + case p.PlanShowMigrations: + return qre.execShowMigrations() case p.PlanShowMigrationLogs: return qre.execShowMigrationLogs() case p.PlanShowThrottledApps: @@ -1057,6 +1059,13 @@ func (qre *QueryExecutor) execRevertMigration() (*sqltypes.Result, error) { return qre.tsv.onlineDDLExecutor.SubmitMigration(qre.ctx, qre.plan.FullStmt) } +func (qre *QueryExecutor) execShowMigrations() (*sqltypes.Result, error) { + if showMigrationStmt, ok := qre.plan.FullStmt.(*sqlparser.ShowMigrations); ok { + return qre.tsv.onlineDDLExecutor.ShowMigrations(qre.ctx, showMigrationStmt) + } + return nil, vterrors.New(vtrpcpb.Code_INTERNAL, "Expecting SHOW VITESS_MIGRATIONS plan") +} + func (qre *QueryExecutor) execShowMigrationLogs() (*sqltypes.Result, error) { if showMigrationLogsStmt, ok := qre.plan.FullStmt.(*sqlparser.ShowMigrationLogs); ok { return qre.tsv.onlineDDLExecutor.ShowMigrationLogs(qre.ctx, showMigrationLogsStmt) From 703c532547f348889ed6870df4cc74f6e47fe98d Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Wed, 22 Mar 2023 10:15:11 +0200 Subject: [PATCH 4/6] Simplified: vtgate and ast restored from main; query executor generates a ShowMigrationsPlan based on ShowBasic, in preparation for ShowBasic statement coming up in next releases Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/vt/sqlparser/ast.go | 7 - go/vt/sqlparser/ast_clone.go | 15 -- go/vt/sqlparser/ast_copy_on_rewrite.go | 28 ---- go/vt/sqlparser/ast_equals.go | 24 ---- go/vt/sqlparser/ast_format.go | 9 -- go/vt/sqlparser/ast_format_fast.go | 10 -- go/vt/sqlparser/ast_rewrite.go | 36 ----- go/vt/sqlparser/ast_visit.go | 19 --- go/vt/sqlparser/cached_size.go | 14 -- go/vt/sqlparser/sql.go | 2 +- go/vt/sqlparser/sql.y | 2 +- go/vt/vtgate/engine/show_migrations.go | 136 ------------------ go/vt/vtgate/planbuilder/builder.go | 2 - go/vt/vtgate/planbuilder/migration.go | 33 ----- go/vt/vtgate/planbuilder/show.go | 48 +++++++ go/vt/vttablet/onlineddl/executor.go | 26 ++-- .../tabletserver/planbuilder/builder.go | 5 +- .../tabletserver/planbuilder/permission.go | 1 - .../vttablet/tabletserver/planbuilder/plan.go | 2 - go/vt/vttablet/tabletserver/query_executor.go | 4 +- 20 files changed, 70 insertions(+), 353 deletions(-) delete mode 100644 go/vt/vtgate/engine/show_migrations.go diff --git a/go/vt/sqlparser/ast.go b/go/vt/sqlparser/ast.go index e313085b574..9a1f717b1af 100644 --- a/go/vt/sqlparser/ast.go +++ b/go/vt/sqlparser/ast.go @@ -443,12 +443,6 @@ type ( AutoIncSpec *AutoIncSpec } - // ShowMigrations represents a SHOW VITESS_MIGRATIONS statement - ShowMigrations struct { - DbName IdentifierCS - Filter *ShowFilter - } - // ShowMigrationLogs represents a SHOW VITESS_MIGRATION '' LOGS statement ShowMigrationLogs struct { UUID string @@ -737,7 +731,6 @@ func (*AlterTable) iStatement() {} func (*AlterVschema) iStatement() {} func (*AlterMigration) iStatement() {} func (*RevertMigration) iStatement() {} -func (*ShowMigrations) iStatement() {} func (*ShowMigrationLogs) iStatement() {} func (*ShowThrottledApps) iStatement() {} func (*ShowThrottlerStatus) iStatement() {} diff --git a/go/vt/sqlparser/ast_clone.go b/go/vt/sqlparser/ast_clone.go index d82fb3fb7e3..2473fe66511 100644 --- a/go/vt/sqlparser/ast_clone.go +++ b/go/vt/sqlparser/ast_clone.go @@ -419,8 +419,6 @@ func CloneSQLNode(in SQLNode) SQLNode { return CloneRefOfShowFilter(in) case *ShowMigrationLogs: return CloneRefOfShowMigrationLogs(in) - case *ShowMigrations: - return CloneRefOfShowMigrations(in) case *ShowOther: return CloneRefOfShowOther(in) case *ShowThrottledApps: @@ -2630,17 +2628,6 @@ func CloneRefOfShowMigrationLogs(n *ShowMigrationLogs) *ShowMigrationLogs { return &out } -// CloneRefOfShowMigrations creates a deep clone of the input. -func CloneRefOfShowMigrations(n *ShowMigrations) *ShowMigrations { - if n == nil { - return nil - } - out := *n - out.DbName = CloneIdentifierCS(n.DbName) - out.Filter = CloneRefOfShowFilter(n.Filter) - return &out -} - // CloneRefOfShowOther creates a deep clone of the input. func CloneRefOfShowOther(n *ShowOther) *ShowOther { if n == nil { @@ -3936,8 +3923,6 @@ func CloneStatement(in Statement) Statement { return CloneRefOfShow(in) case *ShowMigrationLogs: return CloneRefOfShowMigrationLogs(in) - case *ShowMigrations: - return CloneRefOfShowMigrations(in) case *ShowThrottledApps: return CloneRefOfShowThrottledApps(in) case *ShowThrottlerStatus: diff --git a/go/vt/sqlparser/ast_copy_on_rewrite.go b/go/vt/sqlparser/ast_copy_on_rewrite.go index b48d870aea6..58a6545b24b 100644 --- a/go/vt/sqlparser/ast_copy_on_rewrite.go +++ b/go/vt/sqlparser/ast_copy_on_rewrite.go @@ -418,8 +418,6 @@ func (c *cow) copyOnRewriteSQLNode(n SQLNode, parent SQLNode) (out SQLNode, chan return c.copyOnRewriteRefOfShowFilter(n, parent) case *ShowMigrationLogs: return c.copyOnRewriteRefOfShowMigrationLogs(n, parent) - case *ShowMigrations: - return c.copyOnRewriteRefOfShowMigrations(n, parent) case *ShowOther: return c.copyOnRewriteRefOfShowOther(n, parent) case *ShowThrottledApps: @@ -4982,30 +4980,6 @@ func (c *cow) copyOnRewriteRefOfShowMigrationLogs(n *ShowMigrationLogs, parent S } return } -func (c *cow) copyOnRewriteRefOfShowMigrations(n *ShowMigrations, parent SQLNode) (out SQLNode, changed bool) { - if n == nil || c.cursor.stop { - return n, false - } - out = n - if c.pre == nil || c.pre(n, parent) { - _DbName, changedDbName := c.copyOnRewriteIdentifierCS(n.DbName, n) - _Filter, changedFilter := c.copyOnRewriteRefOfShowFilter(n.Filter, n) - if changedDbName || changedFilter { - res := *n - res.DbName, _ = _DbName.(IdentifierCS) - res.Filter, _ = _Filter.(*ShowFilter) - out = &res - if c.cloned != nil { - c.cloned(n, out) - } - changed = true - } - } - if c.post != nil { - out, changed = c.postVisit(out, parent, changed) - } - return -} func (c *cow) copyOnRewriteRefOfShowOther(n *ShowOther, parent SQLNode) (out SQLNode, changed bool) { if n == nil || c.cursor.stop { return n, false @@ -6921,8 +6895,6 @@ func (c *cow) copyOnRewriteStatement(n Statement, parent SQLNode) (out SQLNode, return c.copyOnRewriteRefOfShow(n, parent) case *ShowMigrationLogs: return c.copyOnRewriteRefOfShowMigrationLogs(n, parent) - case *ShowMigrations: - return c.copyOnRewriteRefOfShowMigrations(n, parent) case *ShowThrottledApps: return c.copyOnRewriteRefOfShowThrottledApps(n, parent) case *ShowThrottlerStatus: diff --git a/go/vt/sqlparser/ast_equals.go b/go/vt/sqlparser/ast_equals.go index c66858d49f0..44241e8388b 100644 --- a/go/vt/sqlparser/ast_equals.go +++ b/go/vt/sqlparser/ast_equals.go @@ -1214,12 +1214,6 @@ func (cmp *Comparator) SQLNode(inA, inB SQLNode) bool { return false } return cmp.RefOfShowMigrationLogs(a, b) - case *ShowMigrations: - b, ok := inB.(*ShowMigrations) - if !ok { - return false - } - return cmp.RefOfShowMigrations(a, b) case *ShowOther: b, ok := inB.(*ShowOther) if !ok { @@ -3983,18 +3977,6 @@ func (cmp *Comparator) RefOfShowMigrationLogs(a, b *ShowMigrationLogs) bool { cmp.RefOfParsedComments(a.Comments, b.Comments) } -// RefOfShowMigrations does deep equals between the two objects. -func (cmp *Comparator) RefOfShowMigrations(a, b *ShowMigrations) bool { - if a == b { - return true - } - if a == nil || b == nil { - return false - } - return cmp.IdentifierCS(a.DbName, b.DbName) && - cmp.RefOfShowFilter(a.Filter, b.Filter) -} - // RefOfShowOther does deep equals between the two objects. func (cmp *Comparator) RefOfShowOther(a, b *ShowOther) bool { if a == b { @@ -6498,12 +6480,6 @@ func (cmp *Comparator) Statement(inA, inB Statement) bool { return false } return cmp.RefOfShowMigrationLogs(a, b) - case *ShowMigrations: - b, ok := inB.(*ShowMigrations) - if !ok { - return false - } - return cmp.RefOfShowMigrations(a, b) case *ShowThrottledApps: b, ok := inB.(*ShowThrottledApps) if !ok { diff --git a/go/vt/sqlparser/ast_format.go b/go/vt/sqlparser/ast_format.go index 38d10c9b555..e867bf0f235 100644 --- a/go/vt/sqlparser/ast_format.go +++ b/go/vt/sqlparser/ast_format.go @@ -300,15 +300,6 @@ func (node *RevertMigration) Format(buf *TrackedBuffer) { buf.astPrintf(node, "revert %vvitess_migration '%#s'", node.Comments, node.UUID) } -// Format formats the node. -func (node *ShowMigrations) Format(buf *TrackedBuffer) { - buf.astPrintf(node, "show vitess_migrations") - if !node.DbName.IsEmpty() { - buf.astPrintf(node, " from %v", node.DbName) - } - buf.astPrintf(node, "%v", node.Filter) -} - // Format formats the node. func (node *ShowMigrationLogs) Format(buf *TrackedBuffer) { buf.astPrintf(node, "show vitess_migration '%#s' logs", node.UUID) diff --git a/go/vt/sqlparser/ast_format_fast.go b/go/vt/sqlparser/ast_format_fast.go index c4b898efeaf..3b2a3ce6762 100644 --- a/go/vt/sqlparser/ast_format_fast.go +++ b/go/vt/sqlparser/ast_format_fast.go @@ -433,16 +433,6 @@ func (node *RevertMigration) formatFast(buf *TrackedBuffer) { buf.WriteByte('\'') } -// formatFast formats the node. -func (node *ShowMigrations) formatFast(buf *TrackedBuffer) { - buf.WriteString("show vitess_migrations") - if !node.DbName.IsEmpty() { - buf.WriteString(" from ") - node.DbName.formatFast(buf) - } - node.Filter.formatFast(buf) -} - // formatFast formats the node. func (node *ShowMigrationLogs) formatFast(buf *TrackedBuffer) { buf.WriteString("show vitess_migration '") diff --git a/go/vt/sqlparser/ast_rewrite.go b/go/vt/sqlparser/ast_rewrite.go index 20d4efac7c0..404db8301a9 100644 --- a/go/vt/sqlparser/ast_rewrite.go +++ b/go/vt/sqlparser/ast_rewrite.go @@ -418,8 +418,6 @@ func (a *application) rewriteSQLNode(parent SQLNode, node SQLNode, replacer repl return a.rewriteRefOfShowFilter(parent, node, replacer) case *ShowMigrationLogs: return a.rewriteRefOfShowMigrationLogs(parent, node, replacer) - case *ShowMigrations: - return a.rewriteRefOfShowMigrations(parent, node, replacer) case *ShowOther: return a.rewriteRefOfShowOther(parent, node, replacer) case *ShowThrottledApps: @@ -6715,38 +6713,6 @@ func (a *application) rewriteRefOfShowMigrationLogs(parent SQLNode, node *ShowMi } return true } -func (a *application) rewriteRefOfShowMigrations(parent SQLNode, node *ShowMigrations, replacer replacerFunc) bool { - if node == nil { - return true - } - if a.pre != nil { - a.cur.replacer = replacer - a.cur.parent = parent - a.cur.node = node - if !a.pre(&a.cur) { - return true - } - } - if !a.rewriteIdentifierCS(node, node.DbName, func(newNode, parent SQLNode) { - parent.(*ShowMigrations).DbName = newNode.(IdentifierCS) - }) { - return false - } - if !a.rewriteRefOfShowFilter(node, node.Filter, func(newNode, parent SQLNode) { - parent.(*ShowMigrations).Filter = newNode.(*ShowFilter) - }) { - return false - } - if a.post != nil { - a.cur.replacer = replacer - a.cur.parent = parent - a.cur.node = node - if !a.post(&a.cur) { - return false - } - } - return true -} func (a *application) rewriteRefOfShowOther(parent SQLNode, node *ShowOther, replacer replacerFunc) bool { if node == nil { return true @@ -9156,8 +9122,6 @@ func (a *application) rewriteStatement(parent SQLNode, node Statement, replacer return a.rewriteRefOfShow(parent, node, replacer) case *ShowMigrationLogs: return a.rewriteRefOfShowMigrationLogs(parent, node, replacer) - case *ShowMigrations: - return a.rewriteRefOfShowMigrations(parent, node, replacer) case *ShowThrottledApps: return a.rewriteRefOfShowThrottledApps(parent, node, replacer) case *ShowThrottlerStatus: diff --git a/go/vt/sqlparser/ast_visit.go b/go/vt/sqlparser/ast_visit.go index fee737b9fe7..5a15a5650af 100644 --- a/go/vt/sqlparser/ast_visit.go +++ b/go/vt/sqlparser/ast_visit.go @@ -418,8 +418,6 @@ func VisitSQLNode(in SQLNode, f Visit) error { return VisitRefOfShowFilter(in, f) case *ShowMigrationLogs: return VisitRefOfShowMigrationLogs(in, f) - case *ShowMigrations: - return VisitRefOfShowMigrations(in, f) case *ShowOther: return VisitRefOfShowOther(in, f) case *ShowThrottledApps: @@ -3351,21 +3349,6 @@ func VisitRefOfShowMigrationLogs(in *ShowMigrationLogs, f Visit) error { } return nil } -func VisitRefOfShowMigrations(in *ShowMigrations, f Visit) error { - if in == nil { - return nil - } - if cont, err := f(in); err != nil || !cont { - return err - } - if err := VisitIdentifierCS(in.DbName, f); err != nil { - return err - } - if err := VisitRefOfShowFilter(in.Filter, f); err != nil { - return err - } - return nil -} func VisitRefOfShowOther(in *ShowOther, f Visit) error { if in == nil { return nil @@ -4815,8 +4798,6 @@ func VisitStatement(in Statement, f Visit) error { return VisitRefOfShow(in, f) case *ShowMigrationLogs: return VisitRefOfShowMigrationLogs(in, f) - case *ShowMigrations: - return VisitRefOfShowMigrations(in, f) case *ShowThrottledApps: return VisitRefOfShowThrottledApps(in, f) case *ShowThrottlerStatus: diff --git a/go/vt/sqlparser/cached_size.go b/go/vt/sqlparser/cached_size.go index d46337112aa..6a9ad76728f 100644 --- a/go/vt/sqlparser/cached_size.go +++ b/go/vt/sqlparser/cached_size.go @@ -3483,20 +3483,6 @@ func (cached *ShowMigrationLogs) CachedSize(alloc bool) int64 { size += cached.Comments.CachedSize(true) return size } -func (cached *ShowMigrations) CachedSize(alloc bool) int64 { - if cached == nil { - return int64(0) - } - size := int64(0) - if alloc { - size += int64(24) - } - // field DbName vitess.io/vitess/go/vt/sqlparser.IdentifierCS - size += cached.DbName.CachedSize(false) - // field Filter *vitess.io/vitess/go/vt/sqlparser.ShowFilter - size += cached.Filter.CachedSize(true) - return size -} func (cached *ShowOther) CachedSize(alloc bool) int64 { if cached == nil { return int64(0) diff --git a/go/vt/sqlparser/sql.go b/go/vt/sqlparser/sql.go index a8b2db14861..ab945928e4e 100644 --- a/go/vt/sqlparser/sql.go +++ b/go/vt/sqlparser/sql.go @@ -14469,7 +14469,7 @@ yydefault: var yyLOCAL Statement //line sql.y:4095 { - yyLOCAL = &ShowMigrations{Filter: yyDollar[4].showFilterUnion(), DbName: yyDollar[3].identifierCS} + yyLOCAL = &Show{&ShowBasic{Command: VitessMigrations, Filter: yyDollar[4].showFilterUnion(), DbName: yyDollar[3].identifierCS}} } yyVAL.union = yyLOCAL case 749: diff --git a/go/vt/sqlparser/sql.y b/go/vt/sqlparser/sql.y index 168fd0327bf..1d0f5ac57fd 100644 --- a/go/vt/sqlparser/sql.y +++ b/go/vt/sqlparser/sql.y @@ -4093,7 +4093,7 @@ show_statement: } | SHOW VITESS_MIGRATIONS from_database_opt like_or_where_opt { - $$ = &ShowMigrations{Filter: $4, DbName: $3} + $$ = &Show{&ShowBasic{Command: VitessMigrations, Filter: $4, DbName: $3}} } | SHOW VITESS_MIGRATION STRING LOGS { diff --git a/go/vt/vtgate/engine/show_migrations.go b/go/vt/vtgate/engine/show_migrations.go deleted file mode 100644 index 1b80efc23ab..00000000000 --- a/go/vt/vtgate/engine/show_migrations.go +++ /dev/null @@ -1,136 +0,0 @@ -/* -Copyright 2023 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 engine - -import ( - "context" - "fmt" - - "vitess.io/vitess/go/sqltypes" - "vitess.io/vitess/go/vt/key" - "vitess.io/vitess/go/vt/log" - querypb "vitess.io/vitess/go/vt/proto/query" - vtrpcpb "vitess.io/vitess/go/vt/proto/vtrpc" - "vitess.io/vitess/go/vt/sidecardb" - "vitess.io/vitess/go/vt/sqlparser" - "vitess.io/vitess/go/vt/vterrors" - "vitess.io/vitess/go/vt/vtgate/vindexes" -) - -var _ Primitive = (*ShowMigrations)(nil) - -// TODO: (shlomi) remove in v18 -// Backwards compatibility: this is introduced in v17, But vttablets may still be on v16 and -// are not aware of this new statement. The logic in this primitive supports both new and old options. -// In v18 this struct should be removed and replaced with a simple Send (see buildShowVMigrationsPlan()) - -// ShowMigrations represents the instructions to perform an online schema change via vtctld -type ShowMigrations struct { - Keyspace *vindexes.Keyspace - Stmt *sqlparser.ShowMigrations - Query string - TargetDestination key.Destination - - noTxNeeded - - noInputs -} - -func (v *ShowMigrations) description() PrimitiveDescription { - return PrimitiveDescription{ - OperatorType: "ShowMigrations", - Keyspace: v.Keyspace, - Other: map[string]any{ - "query": v.Query, - }, - } -} - -// RouteType implements the Primitive interface -func (v *ShowMigrations) RouteType() string { - return "ShowMigrations" -} - -// GetKeyspaceName implements the Primitive interface -func (v *ShowMigrations) GetKeyspaceName() string { - return v.Keyspace.Name -} - -// GetTableName implements the Primitive interface -func (v *ShowMigrations) GetTableName() string { - return "" -} - -// TryExecute implements the Primitive interface -func (v *ShowMigrations) TryExecute(ctx context.Context, vcursor VCursor, bindVars map[string]*querypb.BindVariable, wantfields bool) (result *sqltypes.Result, err error) { - s := Send{ - Keyspace: v.Keyspace, - TargetDestination: v.TargetDestination, - Query: v.Query, - IsDML: false, - SingleShardOnly: false, - } - result, err = vcursor.ExecutePrimitive(ctx, &s, bindVars, wantfields) - if err == nil { - return result, nil - } - // TODO: (shlomi) remove in v18 - // Backwards compatibility: in v17 we introduce ShowMigrations ast statement. But vttablets may still be on v16 and - // are not aware of this new statement. In such case they return a vtrpcpb.Code_INVALID_ARGUMENT. - // If we intercept this return code, we fall back to a simple Send() with a generated query. - if vterrors.Code(err) != vtrpcpb.Code_INVALID_ARGUMENT { - return result, err - } - // Old method: construct a query here. Remove in v18. - sidecarDBID, err := sidecardb.GetIdentifierForKeyspace(v.GetKeyspaceName()) - if err != nil { - log.Errorf("Failed to read sidecar database identifier for keyspace %q from the cache: %v", v.GetKeyspaceName(), err) - return nil, vterrors.VT14005(v.GetKeyspaceName()) - } - - sql := sqlparser.BuildParsedQuery("SELECT * FROM %s.schema_migrations", sidecarDBID).Query - - if v.Stmt.Filter != nil { - if v.Stmt.Filter.Filter != nil { - sql += fmt.Sprintf(" where %s", sqlparser.String(v.Stmt.Filter.Filter)) - } else if v.Stmt.Filter.Like != "" { - lit := sqlparser.String(sqlparser.NewStrLiteral(v.Stmt.Filter.Like)) - sql += fmt.Sprintf(" where migration_uuid LIKE %s OR migration_context LIKE %s OR migration_status LIKE %s", lit, lit, lit) - } - } - s = Send{ - Keyspace: v.Keyspace, - TargetDestination: v.TargetDestination, - Query: sql, - } - result, err = vcursor.ExecutePrimitive(ctx, &s, bindVars, wantfields) - return result, err -} - -// TryStreamExecute implements the Primitive interface -func (v *ShowMigrations) TryStreamExecute(ctx context.Context, vcursor VCursor, bindVars map[string]*querypb.BindVariable, wantfields bool, callback func(*sqltypes.Result) error) error { - results, err := v.TryExecute(ctx, vcursor, bindVars, wantfields) - if err != nil { - return err - } - return callback(results) -} - -// GetFields implements the Primitive interface -func (v *ShowMigrations) GetFields(ctx context.Context, vcursor VCursor, bindVars map[string]*querypb.BindVariable) (*sqltypes.Result, error) { - return nil, vterrors.Errorf(vtrpcpb.Code_INTERNAL, "[BUG] GetFields is not reachable") -} diff --git a/go/vt/vtgate/planbuilder/builder.go b/go/vt/vtgate/planbuilder/builder.go index a46cbf7c6e3..1181c9b2c8b 100644 --- a/go/vt/vtgate/planbuilder/builder.go +++ b/go/vt/vtgate/planbuilder/builder.go @@ -236,8 +236,6 @@ func createInstructionFor(query string, stmt sqlparser.Statement, reservedVars * return buildAlterMigrationPlan(query, vschema, enableOnlineDDL) case *sqlparser.RevertMigration: return buildRevertMigrationPlan(query, stmt, vschema, enableOnlineDDL) - case *sqlparser.ShowMigrations: - return buildShowVMigrationsPlan(stmt, vschema) case *sqlparser.ShowMigrationLogs: return buildShowMigrationLogsPlan(query, vschema, enableOnlineDDL) case *sqlparser.ShowThrottledApps: diff --git a/go/vt/vtgate/planbuilder/migration.go b/go/vt/vtgate/planbuilder/migration.go index 339cb3f0eac..468c86d3ffb 100644 --- a/go/vt/vtgate/planbuilder/migration.go +++ b/go/vt/vtgate/planbuilder/migration.go @@ -110,36 +110,3 @@ func buildShowMigrationLogsPlan(query string, vschema plancontext.VSchema, enabl } return newPlanResult(send), nil } - -// buildShowVMigrationsPlan serves `SHOW VITESS_MIGRATIONS ...` queries. -// It invokes queries on the sidecar database's schema_migrations table -// on all PRIMARY tablets in the keyspace's shards. -func buildShowVMigrationsPlan(show *sqlparser.ShowMigrations, vschema plancontext.VSchema) (*planResult, error) { - dest, ks, tabletType, err := vschema.TargetDestination(show.DbName.String()) - if err != nil { - return nil, err - } - if ks == nil { - return nil, vterrors.VT09005() - } - - if tabletType != topodatapb.TabletType_PRIMARY { - return nil, vterrors.VT09006("SHOW") - } - - if dest == nil { - dest = key.DestinationAllShards{} - } - - // v17 introduces ShowMigrations ast, but vttablets may still be on v16 and not know this statement. - // For this reason, we create a new primitive, `engine.ShowMigrations` that supports both new and - // old methods. - // TODO: (shlomi) in v18, remove engine.ShowMigrations and use a simple Send. - prim := &engine.ShowMigrations{ - Keyspace: ks, - TargetDestination: dest, - Stmt: show, - Query: sqlparser.String(show), - } - return newPlanResult(prim), nil -} diff --git a/go/vt/vtgate/planbuilder/show.go b/go/vt/vtgate/planbuilder/show.go index 4242c62baa3..28e7e707848 100644 --- a/go/vt/vtgate/planbuilder/show.go +++ b/go/vt/vtgate/planbuilder/show.go @@ -25,8 +25,11 @@ import ( "vitess.io/vitess/go/mysql/collations" "vitess.io/vitess/go/sqltypes" "vitess.io/vitess/go/vt/key" + "vitess.io/vitess/go/vt/log" querypb "vitess.io/vitess/go/vt/proto/query" + topodatapb "vitess.io/vitess/go/vt/proto/topodata" vschemapb "vitess.io/vitess/go/vt/proto/vschema" + "vitess.io/vitess/go/vt/sidecardb" "vitess.io/vitess/go/vt/sqlparser" "vitess.io/vitess/go/vt/vterrors" "vitess.io/vitess/go/vt/vtgate/engine" @@ -95,6 +98,8 @@ func buildShowBasicPlan(show *sqlparser.ShowBasic, vschema plancontext.VSchema) return buildPlanWithDB(show, vschema) case sqlparser.StatusGlobal, sqlparser.StatusSession: return buildSendAnywherePlan(show, vschema) + case sqlparser.VitessMigrations: + return buildShowVMigrationsPlan(show, vschema) case sqlparser.VGtidExecGlobal: return buildShowVGtidPlan(show, vschema) case sqlparser.GtidExecGlobal: @@ -242,6 +247,49 @@ func buildDBPlan(show *sqlparser.ShowBasic, vschema plancontext.VSchema) (engine return engine.NewRowsPrimitive(rows, buildVarCharFields("Database")), nil } +// buildShowVMigrationsPlan serves `SHOW VITESS_MIGRATIONS ...` queries. +// It invokes queries on the sidecar database's schema_migrations table +// on all PRIMARY tablets in the keyspace's shards. +func buildShowVMigrationsPlan(show *sqlparser.ShowBasic, vschema plancontext.VSchema) (engine.Primitive, error) { + dest, ks, tabletType, err := vschema.TargetDestination(show.DbName.String()) + if err != nil { + return nil, err + } + if ks == nil { + return nil, vterrors.VT09005() + } + + if tabletType != topodatapb.TabletType_PRIMARY { + return nil, vterrors.VT09006("SHOW") + } + + if dest == nil { + dest = key.DestinationAllShards{} + } + + sidecarDBID, err := sidecardb.GetIdentifierForKeyspace(ks.Name) + if err != nil { + log.Errorf("Failed to read sidecar database identifier for keyspace %q from the cache: %v", ks.Name, err) + return nil, vterrors.VT14005(ks.Name) + } + + sql := sqlparser.BuildParsedQuery("SELECT * FROM %s.schema_migrations", sidecarDBID).Query + + if show.Filter != nil { + if show.Filter.Filter != nil { + sql += fmt.Sprintf(" where %s", sqlparser.String(show.Filter.Filter)) + } else if show.Filter.Like != "" { + lit := sqlparser.String(sqlparser.NewStrLiteral(show.Filter.Like)) + sql += fmt.Sprintf(" where migration_uuid LIKE %s OR migration_context LIKE %s OR migration_status LIKE %s", lit, lit, lit) + } + } + return &engine.Send{ + Keyspace: ks, + TargetDestination: dest, + Query: sql, + }, nil +} + func buildPlanWithDB(show *sqlparser.ShowBasic, vschema plancontext.VSchema) (engine.Primitive, error) { dbName := show.DbName dbDestination := show.DbName.String() diff --git a/go/vt/vttablet/onlineddl/executor.go b/go/vt/vttablet/onlineddl/executor.go index eef8578a721..b497be9d283 100644 --- a/go/vt/vttablet/onlineddl/executor.go +++ b/go/vt/vttablet/onlineddl/executor.go @@ -4694,26 +4694,28 @@ func (e *Executor) SubmitMigration( } // ShowMigrations shows migrations, optionally filtered by a condition -func (e *Executor) ShowMigrations(ctx context.Context, stmt *sqlparser.ShowMigrations) (result *sqltypes.Result, err error) { +func (e *Executor) ShowMigrations(ctx context.Context, show *sqlparser.Show) (result *sqltypes.Result, err error) { if atomic.LoadInt64(&e.isOpen) == 0 { return nil, vterrors.New(vtrpcpb.Code_FAILED_PRECONDITION, "online ddl is disabled") } - + showBasic, ok := show.Internal.(*sqlparser.ShowBasic) + if !ok { + return nil, vterrors.Errorf(vtrpcpb.Code_INTERNAL, "[BUG] ShowMigrations expects a ShowBasic statement. Got: %s", sqlparser.String(show)) + } + if showBasic.Command != sqlparser.VitessMigrations { + return nil, vterrors.Errorf(vtrpcpb.Code_INTERNAL, "[BUG] ShowMigrations expects a VitessMigrations command, got %v. Statement: %s", sqlparser.VitessMigrations, sqlparser.String(show)) + } whereExpr := "" - if stmt.Filter != nil { - if stmt.Filter.Filter != nil { - whereExpr = fmt.Sprintf(" where %s", sqlparser.String(stmt.Filter.Filter)) - } else if stmt.Filter.Like != "" { - lit := sqlparser.String(sqlparser.NewStrLiteral(stmt.Filter.Like)) + if showBasic.Filter != nil { + if showBasic.Filter.Filter != nil { + whereExpr = fmt.Sprintf(" where %s", sqlparser.String(showBasic.Filter.Filter)) + } else if showBasic.Filter.Like != "" { + lit := sqlparser.String(sqlparser.NewStrLiteral(showBasic.Filter.Like)) whereExpr = fmt.Sprintf(" where migration_uuid LIKE %s OR migration_context LIKE %s OR migration_status LIKE %s", lit, lit, lit) } } - - e.migrationMutex.Lock() - defer e.migrationMutex.Unlock() query := sqlparser.BuildParsedQuery(sqlShowMigrationsWhere, whereExpr).Query - result, err = e.execQuery(ctx, query) - return result, err + return e.execQuery(ctx, query) } // ShowMigrationLogs reads the migration log for a given migration diff --git a/go/vt/vttablet/tabletserver/planbuilder/builder.go b/go/vt/vttablet/tabletserver/planbuilder/builder.go index 980adf3efb5..89388812d70 100644 --- a/go/vt/vttablet/tabletserver/planbuilder/builder.go +++ b/go/vt/vttablet/tabletserver/planbuilder/builder.go @@ -135,7 +135,10 @@ func analyzeInsert(ins *sqlparser.Insert, tables map[string]*schema.Table) (plan func analyzeShow(show *sqlparser.Show, dbName string) (plan *Plan, err error) { switch showInternal := show.Internal.(type) { case *sqlparser.ShowBasic: - if showInternal.Command == sqlparser.Table { + switch showInternal.Command { + case sqlparser.VitessMigrations: + return &Plan{PlanID: PlanShowMigrations, FullStmt: show}, nil + case sqlparser.Table: // rewrite WHERE clause if it exists // `where Tables_in_Keyspace` => `where Tables_in_DbName` if showInternal.Filter != nil { diff --git a/go/vt/vttablet/tabletserver/planbuilder/permission.go b/go/vt/vttablet/tabletserver/planbuilder/permission.go index d37de27c13e..d68ff43a152 100644 --- a/go/vt/vttablet/tabletserver/planbuilder/permission.go +++ b/go/vt/vttablet/tabletserver/planbuilder/permission.go @@ -54,7 +54,6 @@ func BuildPermissions(stmt sqlparser.Statement) []Permission { case *sqlparser.AlterMigration, *sqlparser.RevertMigration, - *sqlparser.ShowMigrations, *sqlparser.ShowMigrationLogs, *sqlparser.ShowThrottledApps, *sqlparser.ShowThrottlerStatus: diff --git a/go/vt/vttablet/tabletserver/planbuilder/plan.go b/go/vt/vttablet/tabletserver/planbuilder/plan.go index c2bad33a5c5..842f6349665 100644 --- a/go/vt/vttablet/tabletserver/planbuilder/plan.go +++ b/go/vt/vttablet/tabletserver/planbuilder/plan.go @@ -227,8 +227,6 @@ func Build(statement sqlparser.Statement, tables map[string]*schema.Table, dbNam plan, err = &Plan{PlanID: PlanAlterMigration, FullStmt: stmt}, nil case *sqlparser.RevertMigration: plan, err = &Plan{PlanID: PlanRevertMigration, FullStmt: stmt}, nil - case *sqlparser.ShowMigrations: - plan, err = &Plan{PlanID: PlanShowMigrations, FullStmt: stmt}, nil case *sqlparser.ShowMigrationLogs: plan, err = &Plan{PlanID: PlanShowMigrationLogs, FullStmt: stmt}, nil case *sqlparser.ShowThrottledApps: diff --git a/go/vt/vttablet/tabletserver/query_executor.go b/go/vt/vttablet/tabletserver/query_executor.go index 3161ab6534c..eb8ef76dcd0 100644 --- a/go/vt/vttablet/tabletserver/query_executor.go +++ b/go/vt/vttablet/tabletserver/query_executor.go @@ -1060,8 +1060,8 @@ func (qre *QueryExecutor) execRevertMigration() (*sqltypes.Result, error) { } func (qre *QueryExecutor) execShowMigrations() (*sqltypes.Result, error) { - if showMigrationStmt, ok := qre.plan.FullStmt.(*sqlparser.ShowMigrations); ok { - return qre.tsv.onlineDDLExecutor.ShowMigrations(qre.ctx, showMigrationStmt) + if showStmt, ok := qre.plan.FullStmt.(*sqlparser.Show); ok { + return qre.tsv.onlineDDLExecutor.ShowMigrations(qre.ctx, showStmt) } return nil, vterrors.New(vtrpcpb.Code_INTERNAL, "Expecting SHOW VITESS_MIGRATIONS plan") } From c4be8e297bed6ff08ba760322da64061652d06ff Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Wed, 22 Mar 2023 11:30:03 +0200 Subject: [PATCH 5/6] fix value Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/vt/vttablet/onlineddl/executor.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/vt/vttablet/onlineddl/executor.go b/go/vt/vttablet/onlineddl/executor.go index b497be9d283..bb9acecb3d2 100644 --- a/go/vt/vttablet/onlineddl/executor.go +++ b/go/vt/vttablet/onlineddl/executor.go @@ -4703,7 +4703,7 @@ func (e *Executor) ShowMigrations(ctx context.Context, show *sqlparser.Show) (re return nil, vterrors.Errorf(vtrpcpb.Code_INTERNAL, "[BUG] ShowMigrations expects a ShowBasic statement. Got: %s", sqlparser.String(show)) } if showBasic.Command != sqlparser.VitessMigrations { - return nil, vterrors.Errorf(vtrpcpb.Code_INTERNAL, "[BUG] ShowMigrations expects a VitessMigrations command, got %v. Statement: %s", sqlparser.VitessMigrations, sqlparser.String(show)) + return nil, vterrors.Errorf(vtrpcpb.Code_INTERNAL, "[BUG] ShowMigrations expects a VitessMigrations command, got %v. Statement: %s", showBasic.Command, sqlparser.String(show)) } whereExpr := "" if showBasic.Filter != nil { From d0c46accb81c36c1b5e4be2a64819d4df4cd703e Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Sun, 2 Apr 2023 08:32:47 +0300 Subject: [PATCH 6/6] reuse schema.ErrOnlineDDLDisabled Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/vt/vttablet/onlineddl/executor.go | 24 +++++++++---------- .../tabletserver/query_executor_test.go | 2 +- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/go/vt/vttablet/onlineddl/executor.go b/go/vt/vttablet/onlineddl/executor.go index bb9acecb3d2..a55f690f581 100644 --- a/go/vt/vttablet/onlineddl/executor.go +++ b/go/vt/vttablet/onlineddl/executor.go @@ -1989,7 +1989,7 @@ func (e *Executor) terminateMigration(ctx context.Context, onlineDDL *schema.Onl // CancelMigration attempts to abort a scheduled or a running migration func (e *Executor) CancelMigration(ctx context.Context, uuid string, message string, issuedByUser bool) (result *sqltypes.Result, err error) { if atomic.LoadInt64(&e.isOpen) == 0 { - return nil, vterrors.New(vtrpcpb.Code_FAILED_PRECONDITION, "online ddl is disabled") + return nil, vterrors.New(vtrpcpb.Code_FAILED_PRECONDITION, schema.ErrOnlineDDLDisabled.Error()) } log.Infof("CancelMigration: request to cancel %s with message: %v", uuid, message) @@ -2060,7 +2060,7 @@ func (e *Executor) cancelMigrations(ctx context.Context, cancellable []*cancella // for this keyspace func (e *Executor) CancelPendingMigrations(ctx context.Context, message string, issuedByUser bool) (result *sqltypes.Result, err error) { if atomic.LoadInt64(&e.isOpen) == 0 { - return nil, vterrors.New(vtrpcpb.Code_FAILED_PRECONDITION, "online ddl is disabled") + return nil, vterrors.New(vtrpcpb.Code_FAILED_PRECONDITION, schema.ErrOnlineDDLDisabled.Error()) } uuids, err := e.readPendingMigrationsUUIDs(ctx) @@ -4335,7 +4335,7 @@ func (e *Executor) retryMigrationWhere(ctx context.Context, whereExpr string) (r // RetryMigration marks given migration for retry func (e *Executor) RetryMigration(ctx context.Context, uuid string) (result *sqltypes.Result, err error) { if atomic.LoadInt64(&e.isOpen) == 0 { - return nil, vterrors.New(vtrpcpb.Code_FAILED_PRECONDITION, "online ddl is disabled") + return nil, vterrors.New(vtrpcpb.Code_FAILED_PRECONDITION, schema.ErrOnlineDDLDisabled.Error()) } if !schema.IsOnlineDDLUUID(uuid) { return nil, vterrors.Errorf(vtrpcpb.Code_UNKNOWN, "Not a valid migration ID in RETRY: %s", uuid) @@ -4359,7 +4359,7 @@ func (e *Executor) RetryMigration(ctx context.Context, uuid string) (result *sql // next iteration of gcArtifacts() picks up the migration's artifacts and schedules them for deletion func (e *Executor) CleanupMigration(ctx context.Context, uuid string) (result *sqltypes.Result, err error) { if atomic.LoadInt64(&e.isOpen) == 0 { - return nil, vterrors.New(vtrpcpb.Code_FAILED_PRECONDITION, "online ddl is disabled") + return nil, vterrors.New(vtrpcpb.Code_FAILED_PRECONDITION, schema.ErrOnlineDDLDisabled.Error()) } if !schema.IsOnlineDDLUUID(uuid) { return nil, vterrors.Errorf(vtrpcpb.Code_UNKNOWN, "Not a valid migration ID in CLEANUP: %s", uuid) @@ -4385,7 +4385,7 @@ func (e *Executor) CleanupMigration(ctx context.Context, uuid string) (result *s // CompleteMigration clears the postpone_completion flag for a given migration, assuming it was set in the first place func (e *Executor) CompleteMigration(ctx context.Context, uuid string) (result *sqltypes.Result, err error) { if atomic.LoadInt64(&e.isOpen) == 0 { - return nil, vterrors.New(vtrpcpb.Code_FAILED_PRECONDITION, "online ddl is disabled") + return nil, vterrors.New(vtrpcpb.Code_FAILED_PRECONDITION, schema.ErrOnlineDDLDisabled.Error()) } if !schema.IsOnlineDDLUUID(uuid) { return nil, vterrors.Errorf(vtrpcpb.Code_UNKNOWN, "Not a valid migration ID in COMPLETE: %s", uuid) @@ -4419,7 +4419,7 @@ func (e *Executor) CompleteMigration(ctx context.Context, uuid string) (result * // for this keyspace func (e *Executor) CompletePendingMigrations(ctx context.Context) (result *sqltypes.Result, err error) { if atomic.LoadInt64(&e.isOpen) == 0 { - return nil, vterrors.New(vtrpcpb.Code_FAILED_PRECONDITION, "online ddl is disabled") + return nil, vterrors.New(vtrpcpb.Code_FAILED_PRECONDITION, schema.ErrOnlineDDLDisabled.Error()) } uuids, err := e.readPendingMigrationsUUIDs(ctx) @@ -4444,7 +4444,7 @@ func (e *Executor) CompletePendingMigrations(ctx context.Context) (result *sqlty // LaunchMigration clears the postpone_launch flag for a given migration, assuming it was set in the first place func (e *Executor) LaunchMigration(ctx context.Context, uuid string, shardsArg string) (result *sqltypes.Result, err error) { if atomic.LoadInt64(&e.isOpen) == 0 { - return nil, vterrors.New(vtrpcpb.Code_FAILED_PRECONDITION, "online ddl is disabled") + return nil, vterrors.New(vtrpcpb.Code_FAILED_PRECONDITION, schema.ErrOnlineDDLDisabled.Error()) } if !schema.IsOnlineDDLUUID(uuid) { return nil, vterrors.Errorf(vtrpcpb.Code_UNKNOWN, "Not a valid migration ID in EXECUTE: %s", uuid) @@ -4476,7 +4476,7 @@ func (e *Executor) LaunchMigration(ctx context.Context, uuid string, shardsArg s // LaunchMigrations launches all launch-postponed queued migrations for this keyspace func (e *Executor) LaunchMigrations(ctx context.Context) (result *sqltypes.Result, err error) { if atomic.LoadInt64(&e.isOpen) == 0 { - return nil, vterrors.New(vtrpcpb.Code_FAILED_PRECONDITION, "online ddl is disabled") + return nil, vterrors.New(vtrpcpb.Code_FAILED_PRECONDITION, schema.ErrOnlineDDLDisabled.Error()) } uuids, err := e.readPendingMigrationsUUIDs(ctx) @@ -4598,7 +4598,7 @@ func (e *Executor) SubmitMigration( stmt sqlparser.Statement, ) (*sqltypes.Result, error) { if atomic.LoadInt64(&e.isOpen) == 0 { - return nil, vterrors.New(vtrpcpb.Code_FAILED_PRECONDITION, "online ddl is disabled") + return nil, vterrors.New(vtrpcpb.Code_FAILED_PRECONDITION, schema.ErrOnlineDDLDisabled.Error()) } log.Infof("SubmitMigration: request to submit migration with statement: %0.50s...", sqlparser.CanonicalString(stmt)) @@ -4696,14 +4696,14 @@ func (e *Executor) SubmitMigration( // ShowMigrations shows migrations, optionally filtered by a condition func (e *Executor) ShowMigrations(ctx context.Context, show *sqlparser.Show) (result *sqltypes.Result, err error) { if atomic.LoadInt64(&e.isOpen) == 0 { - return nil, vterrors.New(vtrpcpb.Code_FAILED_PRECONDITION, "online ddl is disabled") + return nil, vterrors.New(vtrpcpb.Code_FAILED_PRECONDITION, schema.ErrOnlineDDLDisabled.Error()) } showBasic, ok := show.Internal.(*sqlparser.ShowBasic) if !ok { return nil, vterrors.Errorf(vtrpcpb.Code_INTERNAL, "[BUG] ShowMigrations expects a ShowBasic statement. Got: %s", sqlparser.String(show)) } if showBasic.Command != sqlparser.VitessMigrations { - return nil, vterrors.Errorf(vtrpcpb.Code_INTERNAL, "[BUG] ShowMigrations expects a VitessMigrations command, got %v. Statement: %s", showBasic.Command, sqlparser.String(show)) + return nil, vterrors.Errorf(vtrpcpb.Code_INTERNAL, "[BUG] ShowMigrations expects a VitessMigrations command, got %+v. Statement: %s", showBasic.Command, sqlparser.String(show)) } whereExpr := "" if showBasic.Filter != nil { @@ -4721,7 +4721,7 @@ func (e *Executor) ShowMigrations(ctx context.Context, show *sqlparser.Show) (re // ShowMigrationLogs reads the migration log for a given migration func (e *Executor) ShowMigrationLogs(ctx context.Context, stmt *sqlparser.ShowMigrationLogs) (result *sqltypes.Result, err error) { if atomic.LoadInt64(&e.isOpen) == 0 { - return nil, vterrors.New(vtrpcpb.Code_FAILED_PRECONDITION, "online ddl is disabled") + return nil, vterrors.New(vtrpcpb.Code_FAILED_PRECONDITION, schema.ErrOnlineDDLDisabled.Error()) } _, row, err := e.readMigration(ctx, stmt.UUID) if err != nil { diff --git a/go/vt/vttablet/tabletserver/query_executor_test.go b/go/vt/vttablet/tabletserver/query_executor_test.go index 10dd261f975..fd824c48940 100644 --- a/go/vt/vttablet/tabletserver/query_executor_test.go +++ b/go/vt/vttablet/tabletserver/query_executor_test.go @@ -489,7 +489,7 @@ func TestDisableOnlineDDL(t *testing.T) { qre = newTestQueryExecutor(ctx, tsv, query, 0) _, err = qre.Execute() - require.EqualError(t, err, "online ddl is disabled") + require.EqualError(t, err, "online DDL is disabled") } func TestQueryExecutorLimitFailure(t *testing.T) {