From 94dfd6433c02d30f10bda237a76bf14ec1b14700 Mon Sep 17 00:00:00 2001 From: Noble Mittal Date: Sat, 21 Dec 2024 23:33:20 +0530 Subject: [PATCH] refac: newLookupVindex func to reduce code duplicacy Signed-off-by: Noble Mittal --- go/vt/vtctl/workflow/lookup_vindex.go | 39 +++++++++++------- go/vt/vtctl/workflow/materializer_test.go | 49 ++++------------------- go/vt/vtctl/workflow/server.go | 8 +--- 3 files changed, 34 insertions(+), 62 deletions(-) diff --git a/go/vt/vtctl/workflow/lookup_vindex.go b/go/vt/vtctl/workflow/lookup_vindex.go index d1de6e23bda..cf9b4833c28 100644 --- a/go/vt/vtctl/workflow/lookup_vindex.go +++ b/go/vt/vtctl/workflow/lookup_vindex.go @@ -50,8 +50,19 @@ type lookupVindex struct { parser *sqlparser.Parser } +// newLookupVindex creates a new lookupVindex instance which is responsible +// for performing actions related to lookup vindexes. +func newLookupVindex(ws *Server) *lookupVindex { + return &lookupVindex{ + ts: ws.ts, + tmc: ws.tmc, + logger: ws.Logger(), + parser: ws.SQLParser(), + } +} + // prepareCreate performs the preparatory steps for creating a Lookup Vindex. -func (l *lookupVindex) prepareCreate(ctx context.Context, workflow, keyspace string, specs *vschemapb.Keyspace, continueAfterCopyWithOwner bool) ( +func (lv *lookupVindex) prepareCreate(ctx context.Context, workflow, keyspace string, specs *vschemapb.Keyspace, continueAfterCopyWithOwner bool) ( ms *vtctldatapb.MaterializeSettings, sourceVSchema, targetVSchema *vschemapb.Keyspace, cancelFunc func() error, err error) { var ( // sourceVSchemaTable is the table info present in the vschema. @@ -65,7 +76,7 @@ func (l *lookupVindex) prepareCreate(ctx context.Context, workflow, keyspace str ) // Validate input vindex. - vindex, vInfo, err := l.validateAndGetVindex(specs) + vindex, vInfo, err := lv.validateAndGetVindex(specs) if err != nil { return nil, nil, nil, nil, err } @@ -80,7 +91,7 @@ func (l *lookupVindex) prepareCreate(ctx context.Context, workflow, keyspace str return nil, nil, nil, nil, err } - sourceVSchema, targetVSchema, err = l.getTargetAndSourceVSchema(ctx, keyspace, vInfo.targetKeyspace) + sourceVSchema, targetVSchema, err = lv.getTargetAndSourceVSchema(ctx, keyspace, vInfo.targetKeyspace) if err != nil { return nil, nil, nil, nil, err } @@ -102,7 +113,7 @@ func (l *lookupVindex) prepareCreate(ctx context.Context, workflow, keyspace str } // Validate against source schema. - sourceShards, err := l.ts.GetServingShards(ctx, keyspace) + sourceShards, err := lv.ts.GetServingShards(ctx, keyspace) if err != nil { return nil, nil, nil, nil, err } @@ -113,7 +124,7 @@ func (l *lookupVindex) prepareCreate(ctx context.Context, workflow, keyspace str } req := &tabletmanagerdatapb.GetSchemaRequest{Tables: []string{vInfo.sourceTableName}} - tableSchema, err := schematools.GetSchema(ctx, l.ts, l.tmc, onesource.PrimaryAlias, req) + tableSchema, err := schematools.GetSchema(ctx, lv.ts, lv.tmc, onesource.PrimaryAlias, req) if err != nil { return nil, nil, nil, nil, err } @@ -124,7 +135,7 @@ func (l *lookupVindex) prepareCreate(ctx context.Context, workflow, keyspace str } // Generate "create table" statement. - createDDL, err = l.generateCreateDDLStatement(tableSchema, sourceVindexColumns, vInfo, vindex) + createDDL, err = lv.generateCreateDDLStatement(tableSchema, sourceVindexColumns, vInfo, vindex) if err != nil { return nil, nil, nil, nil, err } @@ -202,7 +213,7 @@ func (l *lookupVindex) prepareCreate(ctx context.Context, workflow, keyspace str if targetChanged { cancelFunc = func() error { // Restore the original target vschema. - return l.ts.SaveVSchema(ctx, vInfo.targetKeyspace, ogTargetVSchema) + return lv.ts.SaveVSchema(ctx, vInfo.targetKeyspace, ogTargetVSchema) } } @@ -241,7 +252,7 @@ type vindexInfo struct { } // validateAndGetVindex validates and extracts vindex configuration -func (l *lookupVindex) validateAndGetVindex(specs *vschemapb.Keyspace) (*vschemapb.Vindex, *vindexInfo, error) { +func (lv *lookupVindex) validateAndGetVindex(specs *vschemapb.Keyspace) (*vschemapb.Vindex, *vindexInfo, error) { if specs == nil { return nil, nil, vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "no vindex provided") } @@ -256,7 +267,7 @@ func (l *lookupVindex) validateAndGetVindex(specs *vschemapb.Keyspace) (*vschema return nil, nil, vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "vindex %s is not a lookup type", vindex.Type) } - targetKeyspace, targetTableName, err := l.parser.ParseTable(vindex.Params["table"]) + targetKeyspace, targetTableName, err := lv.parser.ParseTable(vindex.Params["table"]) if err != nil || targetKeyspace == "" { return nil, nil, vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "vindex table name (%s) must be in the form .", vindex.Params["table"]) @@ -313,8 +324,8 @@ func (l *lookupVindex) validateAndGetVindex(specs *vschemapb.Keyspace) (*vschema }, nil } -func (l *lookupVindex) getTargetAndSourceVSchema(ctx context.Context, sourceKeyspace string, targetKeyspace string) (sourceVSchema *vschemapb.Keyspace, targetVSchema *vschemapb.Keyspace, err error) { - sourceVSchema, err = l.ts.GetVSchema(ctx, sourceKeyspace) +func (lv *lookupVindex) getTargetAndSourceVSchema(ctx context.Context, sourceKeyspace string, targetKeyspace string) (sourceVSchema *vschemapb.Keyspace, targetVSchema *vschemapb.Keyspace, err error) { + sourceVSchema, err = lv.ts.GetVSchema(ctx, sourceKeyspace) if err != nil { return nil, nil, err } @@ -326,7 +337,7 @@ func (l *lookupVindex) getTargetAndSourceVSchema(ctx context.Context, sourceKeys if sourceKeyspace == targetKeyspace { targetVSchema = sourceVSchema } else { - targetVSchema, err = l.ts.GetVSchema(ctx, targetKeyspace) + targetVSchema, err = lv.ts.GetVSchema(ctx, targetKeyspace) if err != nil { return nil, nil, err } @@ -374,7 +385,7 @@ func getSourceTable(specs *vschemapb.Keyspace, targetTableName string, fromCols return sourceTable, sourceTableName, nil } -func (l *lookupVindex) generateCreateDDLStatement(tableSchema *tabletmanagerdatapb.SchemaDefinition, sourceVindexColumns []string, vInfo *vindexInfo, vindex *vschemapb.Vindex) (string, error) { +func (lv *lookupVindex) generateCreateDDLStatement(tableSchema *tabletmanagerdatapb.SchemaDefinition, sourceVindexColumns []string, vInfo *vindexInfo, vindex *vschemapb.Vindex) (string, error) { lines := strings.Split(tableSchema.TableDefinitions[0].Schema, "\n") if len(lines) < 3 { // Should never happen. @@ -412,7 +423,7 @@ func (l *lookupVindex) generateCreateDDLStatement(tableSchema *tabletmanagerdata createDDL := strings.Join(modified, "\n") // Confirm that our DDL is valid before we create anything. - if _, err := l.parser.ParseStrictDDL(createDDL); err != nil { + if _, err := lv.parser.ParseStrictDDL(createDDL); err != nil { return "", vterrors.Errorf(vtrpcpb.Code_INTERNAL, "error: %v; invalid lookup table definition generated: %s", err, createDDL) } diff --git a/go/vt/vtctl/workflow/materializer_test.go b/go/vt/vtctl/workflow/materializer_test.go index 2b33bfc7afe..e430f740c1f 100644 --- a/go/vt/vtctl/workflow/materializer_test.go +++ b/go/vt/vtctl/workflow/materializer_test.go @@ -1515,12 +1515,7 @@ func TestCreateLookupVindexCreateDDL(t *testing.T) { setStartingVschema() }() } - lv := &lookupVindex{ - ts: env.ws.ts, - tmc: env.ws.tmc, - logger: env.ws.Logger(), - parser: env.ws.SQLParser(), - } + lv := newLookupVindex(env.ws) outms, _, _, cancelFunc, err := lv.prepareCreate(ctx, "workflow", ms.SourceKeyspace, tcase.specs, false) if tcase.err != "" { require.Error(t, err) @@ -1769,12 +1764,7 @@ func TestCreateLookupVindexSourceVSchema(t *testing.T) { t.Fatal(err) } - lv := &lookupVindex{ - ts: env.ws.ts, - tmc: env.ws.tmc, - logger: env.ws.Logger(), - parser: env.ws.SQLParser(), - } + lv := newLookupVindex(env.ws) _, got, _, _, err := lv.prepareCreate(ctx, "workflow", ms.SourceKeyspace, specs, false) require.NoError(t, err) if !proto.Equal(got, tcase.out) { @@ -2011,12 +2001,7 @@ func TestCreateLookupVindexTargetVSchema(t *testing.T) { t.Fatal(err) } - lv := &lookupVindex{ - ts: env.ws.ts, - tmc: env.ws.tmc, - logger: env.ws.Logger(), - parser: env.ws.SQLParser(), - } + lv := newLookupVindex(env.ws) _, _, got, cancelFunc, err := lv.prepareCreate(ctx, "workflow", ms.SourceKeyspace, specs, false) if tcase.err != "" { if err == nil || !strings.Contains(err.Error(), tcase.err) { @@ -2139,12 +2124,7 @@ func TestCreateLookupVindexSameKeyspace(t *testing.T) { t.Fatal(err) } - lv := &lookupVindex{ - ts: env.ws.ts, - tmc: env.ws.tmc, - logger: env.ws.Logger(), - parser: env.ws.SQLParser(), - } + lv := newLookupVindex(env.ws) _, got, _, _, err := lv.prepareCreate(ctx, "keyspace", ms.TargetKeyspace, specs, false) require.NoError(t, err) if !proto.Equal(got, want) { @@ -2271,12 +2251,7 @@ func TestCreateCustomizedVindex(t *testing.T) { t.Fatal(err) } - lv := &lookupVindex{ - ts: env.ws.ts, - tmc: env.ws.tmc, - logger: env.ws.Logger(), - parser: env.ws.SQLParser(), - } + lv := newLookupVindex(env.ws) _, got, _, _, err := lv.prepareCreate(ctx, "workflow", ms.TargetKeyspace, specs, false) require.NoError(t, err) if !proto.Equal(got, want) { @@ -2395,12 +2370,7 @@ func TestCreateLookupVindexIgnoreNulls(t *testing.T) { t.Fatal(err) } - lv := &lookupVindex{ - ts: env.ws.ts, - tmc: env.ws.tmc, - logger: env.ws.Logger(), - parser: env.ws.SQLParser(), - } + lv := newLookupVindex(env.ws) ms, ks, _, _, err := lv.prepareCreate(ctx, "workflow", ms.TargetKeyspace, specs, false) require.NoError(t, err) if !proto.Equal(wantKs, ks) { @@ -2481,12 +2451,7 @@ func TestStopAfterCopyFlag(t *testing.T) { t.Fatal(err) } - lv := &lookupVindex{ - ts: env.ws.ts, - tmc: env.ws.tmc, - logger: env.ws.Logger(), - parser: env.ws.SQLParser(), - } + lv := newLookupVindex(env.ws) ms1, _, _, _, err := lv.prepareCreate(ctx, "workflow", ms.TargetKeyspace, specs, false) require.NoError(t, err) require.Equal(t, ms1.StopAfterCopy, true) diff --git a/go/vt/vtctl/workflow/server.go b/go/vt/vtctl/workflow/server.go index badc4c5917e..8123416eb41 100644 --- a/go/vt/vtctl/workflow/server.go +++ b/go/vt/vtctl/workflow/server.go @@ -567,12 +567,8 @@ func (s *Server) LookupVindexCreate(ctx context.Context, req *vtctldatapb.Lookup span.Annotate("cells", req.Cells) span.Annotate("tablet_types", req.TabletTypes) - lv := &lookupVindex{ - ts: s.ts, - tmc: s.tmc, - logger: s.Logger(), - parser: s.SQLParser(), - } + lv := newLookupVindex(s) + ms, sourceVSchema, targetVSchema, cancelFunc, err := lv.prepareCreate(ctx, req.Workflow, req.Keyspace, req.Vindex, req.ContinueAfterCopyWithOwner) if err != nil { return nil, err