Move getting current vschema to ApplyVSchemaDDL

Signed-off-by: Matt Lord <[email protected]>
planetscale · Jan 22, 2025 · 60e9de3 · 60e9de3
1 parent 30b7e2f
commit 60e9de3
Show file tree

Hide file tree

Showing 5 changed files with 22 additions and 38 deletions.
diff --git a/go/test/endtoend/vtgate/keyspace_watches/keyspace_watch_test.go b/go/test/endtoend/vtgate/keyspace_watches/keyspace_watch_test.go
@@ -46,8 +46,7 @@ var (
 		PRIMARY KEY (id)
 	) Engine=InnoDB;`
 	vschemaDDL      = "alter vschema create vindex test_vdx using hash"
-	vschemaDDLError = fmt.Sprintf("Error 1105 (HY000): cannot perform Update on keyspaces/%s/VSchema as the topology server connection is read-only",
-		keyspaceUnshardedName)
+	vschemaDDLError = "Error 1105 (HY000): cannot update VSchema as the topology server connection is read-only"
 )
 
 // createConfig creates a config file in TmpDir in vtdataroot and writes the given data.

diff --git a/go/vt/topotools/vschema_ddl.go b/go/vt/topotools/vschema_ddl.go
@@ -17,6 +17,7 @@ limitations under the License.
 package topotools
 
 import (
+	"context"
 	"reflect"
 
 	"vitess.io/vitess/go/vt/sqlparser"
@@ -29,7 +30,23 @@ import (
 
 // ApplyVSchemaDDL applies the given DDL statement to the vschema
 // keyspace definition and returns the modified keyspace object.
-func ApplyVSchemaDDL(ksName string, ksvs *topo.KeyspaceVSchemaInfo, alterVschema *sqlparser.AlterVschema) (*topo.KeyspaceVSchemaInfo, error) {
+func ApplyVSchemaDDL(ctx context.Context, ksName string, topoServer *topo.Server, alterVschema *sqlparser.AlterVschema) (*topo.KeyspaceVSchemaInfo, error) {
+	if topoServer == nil {
+		return nil, vterrors.Errorf(vtrpcpb.Code_FAILED_PRECONDITION, "cannot update VSchema as the topology server connection is read-only")
+	}
+	// Get the most recent version, which we'll then update.
+	ksvs, err := topoServer.GetVSchema(ctx, ksName)
+	if err != nil {
+		if topo.IsErrType(err, topo.NoNode) {
+			ksvs = &topo.KeyspaceVSchemaInfo{
+				Name:     ksName,
+				Keyspace: &vschemapb.Keyspace{},
+			}
+		} else {
+			return nil, vterrors.Wrapf(err, "failed to get the current VSchema for the %s keyspace", ksName)
+		}
+	}
+
 	if ksvs.Tables == nil {
 		ksvs.Tables = map[string]*vschemapb.Table{}
 	}

diff --git a/go/vt/vtctl/grpcvtctldserver/server.go b/go/vt/vtctl/grpcvtctldserver/server.go
@@ -358,13 +358,7 @@ func (s *VtctldServer) ApplyVSchema(ctx context.Context, req *vtctldatapb.ApplyV
 			return nil, err
 		}
 
-		ksvs, err = s.ts.GetVSchema(ctx, req.Keyspace)
-		if err != nil && !topo.IsErrType(err, topo.NoNode) {
-			err = vterrors.Wrapf(err, "GetVSchema(%s)", req.Keyspace)
-			return nil, err
-		} // otherwise, we keep the empty vschema object from above
-
-		ksvs, err = topotools.ApplyVSchemaDDL(req.Keyspace, ksvs, ddl)
+		ksvs, err = topotools.ApplyVSchemaDDL(ctx, req.Keyspace, s.ts, ddl)
 		if err != nil {
 			err = vterrors.Wrapf(err, "ApplyVSchemaDDL(%s,%v,%v)", req.Keyspace, ksvs, ddl)
 			return nil, err

diff --git a/go/vt/vtctl/vtctl.go b/go/vt/vtctl/vtctl.go
@@ -3380,23 +3380,10 @@ func commandApplyVSchema(ctx context.Context, wr *wrangler.Wrangler, subFlags *p
 			return fmt.Errorf("error parsing vschema statement `%s`: not a ddl statement", *sql)
 		}
 
-		ksvs, err = wr.TopoServer().GetVSchema(ctx, keyspace)
-		if err != nil {
-			if topo.IsErrType(err, topo.NoNode) {
-				ksvs = &topo.KeyspaceVSchemaInfo{
-					Name:     keyspace,
-					Keyspace: &vschemapb.Keyspace{},
-				}
-			} else {
-				return err
-			}
-		}
-
-		ksvs, err = topotools.ApplyVSchemaDDL(keyspace, ksvs, ddl)
+		ksvs, err = topotools.ApplyVSchemaDDL(ctx, keyspace, wr.TopoServer(), ddl)
 		if err != nil {
 			return err
 		}
-
 	} else {
 		// json mode
 		var schema []byte

diff --git a/go/vt/vtgate/executorcontext/vcursor_impl.go b/go/vt/vtgate/executorcontext/vcursor_impl.go
@@ -1415,20 +1415,7 @@ func (vc *VCursorImpl) ExecuteVSchema(ctx context.Context, keyspace string, vsch
 		return ErrNoKeyspace
 	}
 
-	ksvs := &topo.KeyspaceVSchemaInfo{}
-	if vc.topoServer != nil {
-		// Get the most recent version if we can.
-		ksvs, err = vc.topoServer.GetVSchema(ctx, ksName)
-		if err != nil {
-			return err
-		}
-	} else {
-		// Use the cached version as we are in read-only mode
-		// and any writes would fail.
-		ksvs.Name = ksName
-		ksvs.Keyspace = srvVschema.Keyspaces[ksName]
-	}
-	ksvs, err = topotools.ApplyVSchemaDDL(ksName, ksvs, vschemaDDL)
+	ksvs, err := topotools.ApplyVSchemaDDL(ctx, ksName, vc.topoServer, vschemaDDL)
 	if err != nil {
 		return err
 	}