From ba2e7d0fd6255195cfe3d3c67f8f6a798ebd47be Mon Sep 17 00:00:00 2001 From: GuptaManan100 Date: Mon, 14 Dec 2020 15:23:35 +0530 Subject: [PATCH 1/3] refactored AlterVschema to be its own struct Signed-off-by: GuptaManan100 --- go/vt/sqlparser/analyzer.go | 12 ----- go/vt/sqlparser/ast.go | 63 ++++++------------------ go/vt/sqlparser/rewriter.go | 59 ++++++++++++---------- go/vt/sqlparser/sql.go | 16 +++--- go/vt/sqlparser/sql.y | 16 +++--- go/vt/topotools/vschema_ddl.go | 38 +++++++------- go/vt/vtctl/vtctl.go | 2 +- go/vt/vtgate/engine/fake_vcursor_test.go | 4 +- go/vt/vtgate/engine/primitive.go | 2 +- go/vt/vtgate/engine/vschema_ddl.go | 8 +-- go/vt/vtgate/planbuilder/builder.go | 5 +- go/vt/vtgate/planbuilder/ddl.go | 10 ++-- go/vt/vtgate/vcursor_impl.go | 6 +-- 13 files changed, 99 insertions(+), 142 deletions(-) diff --git a/go/vt/sqlparser/analyzer.go b/go/vt/sqlparser/analyzer.go index db028f5d99..fde2d7e162 100644 --- a/go/vt/sqlparser/analyzer.go +++ b/go/vt/sqlparser/analyzer.go @@ -269,18 +269,6 @@ func IsDMLStatement(stmt Statement) bool { return false } -//IsVschemaDDL returns true if the query is an Vschema alter ddl. -func IsVschemaDDL(ddl DDLStatement) bool { - switch ddlStatement := ddl.(type) { - case *DDL: - switch ddlStatement.Action { - case CreateVindexDDLAction, DropVindexDDLAction, AddVschemaTableDDLAction, DropVschemaTableDDLAction, AddColVindexDDLAction, DropColVindexDDLAction, AddSequenceDDLAction, AddAutoIncDDLAction: - return true - } - } - return false -} - // SplitAndExpression breaks up the Expr into AND-separated conditions // and appends them to filters. Outer parenthesis are removed. Precedence // should be taken into account if expressions are recombined. diff --git a/go/vt/sqlparser/ast.go b/go/vt/sqlparser/ast.go index 6b74868c65..ab30848f77 100644 --- a/go/vt/sqlparser/ast.go +++ b/go/vt/sqlparser/ast.go @@ -63,11 +63,8 @@ type ( GetAction() DDLAction GetOptLike() *OptLike GetTableSpec() *TableSpec - GetVindexSpec() *VindexSpec GetFromTables() TableNames GetToTables() TableNames - GetAutoIncSpec() *AutoIncSpec - GetVindexCols() []ColIdent AffectedTables() TableNames SetTable(qualifier string, name string) Statement @@ -275,6 +272,12 @@ type ( TableSpec *TableSpec OptLike *OptLike PartitionSpec *PartitionSpec + } + + // AlterVschema represents a ALTER VSCHEMA statement. + AlterVschema struct { + Action DDLAction + Table TableName // VindexSpec is set for CreateVindexDDLAction, DropVindexDDLAction, AddColVindexDDLAction, DropColVindexDDLAction. VindexSpec *VindexSpec @@ -425,6 +428,7 @@ func (*AlterDatabase) iStatement() {} func (*CreateTable) iStatement() {} func (*LockTables) iStatement() {} func (*UnlockTables) iStatement() {} +func (*AlterVschema) iStatement() {} func (*DDL) iDDLStatement() {} func (*CreateIndex) iDDLStatement() {} @@ -500,21 +504,6 @@ func (node *CreateIndex) GetTableSpec() *TableSpec { return nil } -// GetVindexSpec implements the DDLStatement interface -func (node *DDL) GetVindexSpec() *VindexSpec { - return node.VindexSpec -} - -// GetVindexSpec implements the DDLStatement interface -func (node *CreateIndex) GetVindexSpec() *VindexSpec { - return nil -} - -// GetVindexSpec implements the DDLStatement interface -func (node *CreateTable) GetVindexSpec() *VindexSpec { - return nil -} - // GetFromTables implements the DDLStatement interface func (node *DDL) GetFromTables() TableNames { return node.FromTables @@ -545,36 +534,6 @@ func (node *CreateTable) GetToTables() TableNames { return nil } -// GetAutoIncSpec implements the DDLStatement interface -func (node *DDL) GetAutoIncSpec() *AutoIncSpec { - return node.AutoIncSpec -} - -// GetAutoIncSpec implements the DDLStatement interface -func (node *CreateIndex) GetAutoIncSpec() *AutoIncSpec { - return nil -} - -// GetAutoIncSpec implements the DDLStatement interface -func (node *CreateTable) GetAutoIncSpec() *AutoIncSpec { - return nil -} - -// GetVindexCols implements the DDLStatement interface -func (node *DDL) GetVindexCols() []ColIdent { - return node.VindexCols -} - -// GetVindexCols implements the DDLStatement interface -func (node *CreateIndex) GetVindexCols() []ColIdent { - return nil -} - -// GetVindexCols implements the DDLStatement interface -func (node *CreateTable) GetVindexCols() []ColIdent { - return nil -} - // GetAction implements the DDLStatement interface func (node *DDL) GetAction() DDLAction { return node.Action @@ -1482,6 +1441,14 @@ func (node *DDL) Format(buf *TrackedBuffer) { } case FlushDDLAction: buf.astPrintf(node, "%s", FlushStr) + default: + buf.astPrintf(node, "%s table %v", node.Action.ToString(), node.Table) + } +} + +// Format formats the node. +func (node *AlterVschema) Format(buf *TrackedBuffer) { + switch node.Action { case CreateVindexDDLAction: buf.astPrintf(node, "alter vschema create vindex %v %v", node.Table, node.VindexSpec) case DropVindexDDLAction: diff --git a/go/vt/sqlparser/rewriter.go b/go/vt/sqlparser/rewriter.go index 9c94028f66..f00b8eb80c 100644 --- a/go/vt/sqlparser/rewriter.go +++ b/go/vt/sqlparser/rewriter.go @@ -40,6 +40,28 @@ func replaceAliasedTableExprPartitions(newNode, parent SQLNode) { parent.(*AliasedTableExpr).Partitions = newNode.(Partitions) } +func replaceAlterVschemaAutoIncSpec(newNode, parent SQLNode) { + parent.(*AlterVschema).AutoIncSpec = newNode.(*AutoIncSpec) +} + +func replaceAlterVschemaTable(newNode, parent SQLNode) { + parent.(*AlterVschema).Table = newNode.(TableName) +} + +type replaceAlterVschemaVindexCols int + +func (r *replaceAlterVschemaVindexCols) replace(newNode, container SQLNode) { + container.(*AlterVschema).VindexCols[int(*r)] = newNode.(ColIdent) +} + +func (r *replaceAlterVschemaVindexCols) inc() { + *r++ +} + +func replaceAlterVschemaVindexSpec(newNode, parent SQLNode) { + parent.(*AlterVschema).VindexSpec = newNode.(*VindexSpec) +} + func replaceAndExprLeft(newNode, parent SQLNode) { parent.(*AndExpr).Left = newNode.(Expr) } @@ -196,10 +218,6 @@ func replaceCurTimeFuncExprName(newNode, parent SQLNode) { parent.(*CurTimeFuncExpr).Name = newNode.(ColIdent) } -func replaceDDLAutoIncSpec(newNode, parent SQLNode) { - parent.(*DDL).AutoIncSpec = newNode.(*AutoIncSpec) -} - func replaceDDLFromTables(newNode, parent SQLNode) { parent.(*DDL).FromTables = newNode.(TableNames) } @@ -224,20 +242,6 @@ func replaceDDLToTables(newNode, parent SQLNode) { parent.(*DDL).ToTables = newNode.(TableNames) } -type replaceDDLVindexCols int - -func (r *replaceDDLVindexCols) replace(newNode, container SQLNode) { - container.(*DDL).VindexCols[int(*r)] = newNode.(ColIdent) -} - -func (r *replaceDDLVindexCols) inc() { - *r++ -} - -func replaceDDLVindexSpec(newNode, parent SQLNode) { - parent.(*DDL).VindexSpec = newNode.(*VindexSpec) -} - func replaceDeleteComments(newNode, parent SQLNode) { parent.(*Delete).Comments = newNode.(Comments) } @@ -960,6 +964,17 @@ func (a *application) apply(parent, node SQLNode, replacer replacerFunc) { case *AlterDatabase: + case *AlterVschema: + a.apply(node, n.AutoIncSpec, replaceAlterVschemaAutoIncSpec) + a.apply(node, n.Table, replaceAlterVschemaTable) + replacerVindexCols := replaceAlterVschemaVindexCols(0) + replacerVindexColsB := &replacerVindexCols + for _, item := range n.VindexCols { + a.apply(node, item, replacerVindexColsB.replace) + replacerVindexColsB.inc() + } + a.apply(node, n.VindexSpec, replaceAlterVschemaVindexSpec) + case *AndExpr: a.apply(node, n.Left, replaceAndExprLeft) a.apply(node, n.Right, replaceAndExprRight) @@ -1057,20 +1072,12 @@ func (a *application) apply(parent, node SQLNode, replacer replacerFunc) { a.apply(node, n.Name, replaceCurTimeFuncExprName) case *DDL: - a.apply(node, n.AutoIncSpec, replaceDDLAutoIncSpec) a.apply(node, n.FromTables, replaceDDLFromTables) a.apply(node, n.OptLike, replaceDDLOptLike) a.apply(node, n.PartitionSpec, replaceDDLPartitionSpec) a.apply(node, n.Table, replaceDDLTable) a.apply(node, n.TableSpec, replaceDDLTableSpec) a.apply(node, n.ToTables, replaceDDLToTables) - replacerVindexCols := replaceDDLVindexCols(0) - replacerVindexColsB := &replacerVindexCols - for _, item := range n.VindexCols { - a.apply(node, item, replacerVindexColsB.replace) - replacerVindexColsB.inc() - } - a.apply(node, n.VindexSpec, replaceDDLVindexSpec) case *Default: diff --git a/go/vt/sqlparser/sql.go b/go/vt/sqlparser/sql.go index b336002747..011f3ab3d7 100644 --- a/go/vt/sqlparser/sql.go +++ b/go/vt/sqlparser/sql.go @@ -5730,7 +5730,7 @@ yydefault: yyDollar = yyS[yypt-7 : yypt+1] //line sql.y:1635 { - yyVAL.statement = &DDL{ + yyVAL.statement = &AlterVschema{ Action: CreateVindexDDLAction, Table: yyDollar[5].tableName, VindexSpec: &VindexSpec{ @@ -5744,7 +5744,7 @@ yydefault: yyDollar = yyS[yypt-5 : yypt+1] //line sql.y:1647 { - yyVAL.statement = &DDL{ + yyVAL.statement = &AlterVschema{ Action: DropVindexDDLAction, Table: yyDollar[5].tableName, VindexSpec: &VindexSpec{ @@ -5756,19 +5756,19 @@ yydefault: yyDollar = yyS[yypt-5 : yypt+1] //line sql.y:1657 { - yyVAL.statement = &DDL{Action: AddVschemaTableDDLAction, Table: yyDollar[5].tableName} + yyVAL.statement = &AlterVschema{Action: AddVschemaTableDDLAction, Table: yyDollar[5].tableName} } case 277: yyDollar = yyS[yypt-5 : yypt+1] //line sql.y:1661 { - yyVAL.statement = &DDL{Action: DropVschemaTableDDLAction, Table: yyDollar[5].tableName} + yyVAL.statement = &AlterVschema{Action: DropVschemaTableDDLAction, Table: yyDollar[5].tableName} } case 278: yyDollar = yyS[yypt-12 : yypt+1] //line sql.y:1665 { - yyVAL.statement = &DDL{ + yyVAL.statement = &AlterVschema{ Action: AddColVindexDDLAction, Table: yyDollar[4].tableName, VindexSpec: &VindexSpec{ @@ -5783,7 +5783,7 @@ yydefault: yyDollar = yyS[yypt-7 : yypt+1] //line sql.y:1678 { - yyVAL.statement = &DDL{ + yyVAL.statement = &AlterVschema{ Action: DropColVindexDDLAction, Table: yyDollar[4].tableName, VindexSpec: &VindexSpec{ @@ -5795,13 +5795,13 @@ yydefault: yyDollar = yyS[yypt-5 : yypt+1] //line sql.y:1688 { - yyVAL.statement = &DDL{Action: AddSequenceDDLAction, Table: yyDollar[5].tableName} + yyVAL.statement = &AlterVschema{Action: AddSequenceDDLAction, Table: yyDollar[5].tableName} } case 281: yyDollar = yyS[yypt-9 : yypt+1] //line sql.y:1692 { - yyVAL.statement = &DDL{ + yyVAL.statement = &AlterVschema{ Action: AddAutoIncDDLAction, Table: yyDollar[4].tableName, AutoIncSpec: &AutoIncSpec{ diff --git a/go/vt/sqlparser/sql.y b/go/vt/sqlparser/sql.y index 2399deff70..addc65c177 100644 --- a/go/vt/sqlparser/sql.y +++ b/go/vt/sqlparser/sql.y @@ -1633,7 +1633,7 @@ alter_statement: } | ALTER VSCHEMA CREATE VINDEX table_name vindex_type_opt vindex_params_opt { - $$ = &DDL{ + $$ = &AlterVschema{ Action: CreateVindexDDLAction, Table: $5, VindexSpec: &VindexSpec{ @@ -1645,7 +1645,7 @@ alter_statement: } | ALTER VSCHEMA DROP VINDEX table_name { - $$ = &DDL{ + $$ = &AlterVschema{ Action: DropVindexDDLAction, Table: $5, VindexSpec: &VindexSpec{ @@ -1655,15 +1655,15 @@ alter_statement: } | ALTER VSCHEMA ADD TABLE table_name { - $$ = &DDL{Action: AddVschemaTableDDLAction, Table: $5} + $$ = &AlterVschema{Action: AddVschemaTableDDLAction, Table: $5} } | ALTER VSCHEMA DROP TABLE table_name { - $$ = &DDL{Action: DropVschemaTableDDLAction, Table: $5} + $$ = &AlterVschema{Action: DropVschemaTableDDLAction, Table: $5} } | ALTER VSCHEMA ON table_name ADD VINDEX sql_id '(' column_list ')' vindex_type_opt vindex_params_opt { - $$ = &DDL{ + $$ = &AlterVschema{ Action: AddColVindexDDLAction, Table: $4, VindexSpec: &VindexSpec{ @@ -1676,7 +1676,7 @@ alter_statement: } | ALTER VSCHEMA ON table_name DROP VINDEX sql_id { - $$ = &DDL{ + $$ = &AlterVschema{ Action: DropColVindexDDLAction, Table: $4, VindexSpec: &VindexSpec{ @@ -1686,11 +1686,11 @@ alter_statement: } | ALTER VSCHEMA ADD SEQUENCE table_name { - $$ = &DDL{Action: AddSequenceDDLAction, Table: $5} + $$ = &AlterVschema{Action: AddSequenceDDLAction, Table: $5} } | ALTER VSCHEMA ON table_name ADD AUTO_INCREMENT sql_id USING table_name { - $$ = &DDL{ + $$ = &AlterVschema{ Action: AddAutoIncDDLAction, Table: $4, AutoIncSpec: &AutoIncSpec{ diff --git a/go/vt/topotools/vschema_ddl.go b/go/vt/topotools/vschema_ddl.go index c6745b0124..30cce3bd5d 100644 --- a/go/vt/topotools/vschema_ddl.go +++ b/go/vt/topotools/vschema_ddl.go @@ -29,7 +29,7 @@ import ( // ApplyVSchemaDDL applies the given DDL statement to the vschema // keyspace definition and returns the modified keyspace object. -func ApplyVSchemaDDL(ksName string, ks *vschemapb.Keyspace, ddl sqlparser.DDLStatement) (*vschemapb.Keyspace, error) { +func ApplyVSchemaDDL(ksName string, ks *vschemapb.Keyspace, alterVschema *sqlparser.AlterVschema) (*vschemapb.Keyspace, error) { if ks == nil { ks = new(vschemapb.Keyspace) } @@ -44,14 +44,14 @@ func ApplyVSchemaDDL(ksName string, ks *vschemapb.Keyspace, ddl sqlparser.DDLSta var tableName string var table *vschemapb.Table - if !ddl.GetTable().IsEmpty() { - tableName = ddl.GetTable().Name.String() + if !alterVschema.Table.IsEmpty() { + tableName = alterVschema.Table.Name.String() table = ks.Tables[tableName] } - switch ddl.GetAction() { + switch alterVschema.Action { case sqlparser.CreateVindexDDLAction: - name := ddl.GetVindexSpec().Name.String() + name := alterVschema.VindexSpec.Name.String() if _, ok := ks.Vindexes[name]; ok { return nil, vterrors.Errorf(vtrpcpb.Code_INTERNAL, "vindex %s already exists in keyspace %s", name, ksName) } @@ -62,9 +62,9 @@ func ApplyVSchemaDDL(ksName string, ks *vschemapb.Keyspace, ddl sqlparser.DDLSta ks.Sharded = true } - owner, params := ddl.GetVindexSpec().ParseParams() + owner, params := alterVschema.VindexSpec.ParseParams() ks.Vindexes[name] = &vschemapb.Vindex{ - Type: ddl.GetVindexSpec().Type.String(), + Type: alterVschema.VindexSpec.Type.String(), Params: params, Owner: owner, } @@ -72,7 +72,7 @@ func ApplyVSchemaDDL(ksName string, ks *vschemapb.Keyspace, ddl sqlparser.DDLSta return ks, nil case sqlparser.DropVindexDDLAction: - name := ddl.GetVindexSpec().Name.String() + name := alterVschema.VindexSpec.Name.String() if _, ok := ks.Vindexes[name]; !ok { return nil, vterrors.Errorf(vtrpcpb.Code_INTERNAL, "vindex %s does not exists in keyspace %s", name, ksName) } @@ -95,7 +95,7 @@ func ApplyVSchemaDDL(ksName string, ks *vschemapb.Keyspace, ddl sqlparser.DDLSta return nil, vterrors.Errorf(vtrpcpb.Code_INTERNAL, "add vschema table: unsupported on sharded keyspace %s", ksName) } - name := ddl.GetTable().Name.String() + name := alterVschema.Table.Name.String() if _, ok := ks.Tables[name]; ok { return nil, vterrors.Errorf(vtrpcpb.Code_INTERNAL, "vschema already contains table %s in keyspace %s", name, ksName) } @@ -105,7 +105,7 @@ func ApplyVSchemaDDL(ksName string, ks *vschemapb.Keyspace, ddl sqlparser.DDLSta return ks, nil case sqlparser.DropVschemaTableDDLAction: - name := ddl.GetTable().Name.String() + name := alterVschema.Table.Name.String() if _, ok := ks.Tables[name]; !ok { return nil, vterrors.Errorf(vtrpcpb.Code_INTERNAL, "vschema does not contain table %s in keyspace %s", name, ksName) } @@ -123,7 +123,7 @@ func ApplyVSchemaDDL(ksName string, ks *vschemapb.Keyspace, ddl sqlparser.DDLSta // // 2. The vindex type is not specified. Make sure the vindex // already exists. - spec := ddl.GetVindexSpec() + spec := alterVschema.VindexSpec name := spec.Name.String() if !spec.Type.IsEmpty() { owner, params := spec.ParseParams() @@ -171,8 +171,8 @@ func ApplyVSchemaDDL(ksName string, ks *vschemapb.Keyspace, ddl sqlparser.DDLSta } } - columns := make([]string, len(ddl.GetVindexCols())) - for i, col := range ddl.GetVindexCols() { + columns := make([]string, len(alterVschema.VindexCols)) + for i, col := range alterVschema.VindexCols { columns[i] = col.String() } table.ColumnVindexes = append(table.ColumnVindexes, &vschemapb.ColumnVindex{ @@ -184,7 +184,7 @@ func ApplyVSchemaDDL(ksName string, ks *vschemapb.Keyspace, ddl sqlparser.DDLSta return ks, nil case sqlparser.DropColVindexDDLAction: - spec := ddl.GetVindexSpec() + spec := alterVschema.VindexSpec name := spec.Name.String() if table == nil { return nil, vterrors.Errorf(vtrpcpb.Code_INTERNAL, "table %s.%s not defined in vschema", ksName, tableName) @@ -206,7 +206,7 @@ func ApplyVSchemaDDL(ksName string, ks *vschemapb.Keyspace, ddl sqlparser.DDLSta return nil, vterrors.Errorf(vtrpcpb.Code_INTERNAL, "add sequence table: unsupported on sharded keyspace %s", ksName) } - name := ddl.GetTable().Name.String() + name := alterVschema.Table.Name.String() if _, ok := ks.Tables[name]; ok { return nil, vterrors.Errorf(vtrpcpb.Code_INTERNAL, "vschema already contains sequence %s in keyspace %s", name, ksName) } @@ -216,7 +216,7 @@ func ApplyVSchemaDDL(ksName string, ks *vschemapb.Keyspace, ddl sqlparser.DDLSta return ks, nil case sqlparser.AddAutoIncDDLAction: - name := ddl.GetTable().Name.String() + name := alterVschema.Table.Name.String() table := ks.Tables[name] if table == nil { return nil, vterrors.Errorf(vtrpcpb.Code_INTERNAL, "vschema does not contain table %s in keyspace %s", name, ksName) @@ -226,19 +226,19 @@ func ApplyVSchemaDDL(ksName string, ks *vschemapb.Keyspace, ddl sqlparser.DDLSta return nil, vterrors.Errorf(vtrpcpb.Code_INTERNAL, "vschema already contains auto inc %v on table %s in keyspace %s", table.AutoIncrement, name, ksName) } - sequence := ddl.GetAutoIncSpec().Sequence + sequence := alterVschema.AutoIncSpec.Sequence sequenceFqn := sequence.Name.String() if sequence.Qualifier.String() != "" { sequenceFqn = fmt.Sprintf("%s.%s", sequence.Qualifier.String(), sequenceFqn) } table.AutoIncrement = &vschemapb.AutoIncrement{ - Column: ddl.GetAutoIncSpec().Column.String(), + Column: alterVschema.AutoIncSpec.Column.String(), Sequence: sequenceFqn, } return ks, nil } - return nil, vterrors.Errorf(vtrpcpb.Code_INTERNAL, "unexpected vindex ddl operation %s", ddl.GetAction().ToString()) + return nil, vterrors.Errorf(vtrpcpb.Code_INTERNAL, "unexpected vindex ddl operation %s", alterVschema.Action.ToString()) } diff --git a/go/vt/vtctl/vtctl.go b/go/vt/vtctl/vtctl.go index 8e6cc44ec6..d96fee4864 100644 --- a/go/vt/vtctl/vtctl.go +++ b/go/vt/vtctl/vtctl.go @@ -2772,7 +2772,7 @@ func commandApplyVSchema(ctx context.Context, wr *wrangler.Wrangler, subFlags *f if err != nil { return fmt.Errorf("error parsing vschema statement `%s`: %v", *sql, err) } - ddl, ok := stmt.(sqlparser.DDLStatement) + ddl, ok := stmt.(*sqlparser.AlterVschema) if !ok { return fmt.Errorf("error parsing vschema statement `%s`: not a ddl statement", *sql) } diff --git a/go/vt/vtgate/engine/fake_vcursor_test.go b/go/vt/vtgate/engine/fake_vcursor_test.go index 09ac0bfd2c..25af8de7e8 100644 --- a/go/vt/vtgate/engine/fake_vcursor_test.go +++ b/go/vt/vtgate/engine/fake_vcursor_test.go @@ -116,7 +116,7 @@ func (t noopVCursor) ShardSession() []*srvtopo.ResolvedShard { panic("implement me") } -func (t noopVCursor) ExecuteVSchema(keyspace string, vschemaDDL sqlparser.DDLStatement) error { +func (t noopVCursor) ExecuteVSchema(keyspace string, vschemaDDL *sqlparser.AlterVschema) error { panic("implement me") } @@ -281,7 +281,7 @@ func (f *loggingVCursor) ShardSession() []*srvtopo.ResolvedShard { return nil } -func (f *loggingVCursor) ExecuteVSchema(string, sqlparser.DDLStatement) error { +func (f *loggingVCursor) ExecuteVSchema(string, *sqlparser.AlterVschema) error { panic("implement me") } diff --git a/go/vt/vtgate/engine/primitive.go b/go/vt/vtgate/engine/primitive.go index 587aa9bc43..fd8be8044e 100644 --- a/go/vt/vtgate/engine/primitive.go +++ b/go/vt/vtgate/engine/primitive.go @@ -84,7 +84,7 @@ type ( // Will replace all of the Topo functions. ResolveDestinations(keyspace string, ids []*querypb.Value, destinations []key.Destination) ([]*srvtopo.ResolvedShard, [][]*querypb.Value, error) - ExecuteVSchema(keyspace string, vschemaDDL sqlparser.DDLStatement) error + ExecuteVSchema(keyspace string, vschemaDDL *sqlparser.AlterVschema) error SubmitOnlineDDL(onlineDDl *schema.OnlineDDL) error diff --git a/go/vt/vtgate/engine/vschema_ddl.go b/go/vt/vtgate/engine/vschema_ddl.go index 3fe128ff32..5eca45e566 100644 --- a/go/vt/vtgate/engine/vschema_ddl.go +++ b/go/vt/vtgate/engine/vschema_ddl.go @@ -31,7 +31,7 @@ var _ Primitive = (*AlterVSchema)(nil) type AlterVSchema struct { Keyspace *vindexes.Keyspace - DDL sqlparser.DDLStatement + AlterVschemaDDL *sqlparser.AlterVschema noTxNeeded @@ -43,7 +43,7 @@ func (v *AlterVSchema) description() PrimitiveDescription { OperatorType: "AlterVSchema", Keyspace: v.Keyspace, Other: map[string]interface{}{ - "query": sqlparser.String(v.DDL), + "query": sqlparser.String(v.AlterVschemaDDL), }, } } @@ -60,12 +60,12 @@ func (v *AlterVSchema) GetKeyspaceName() string { //GetTableName implements the Primitive interface func (v *AlterVSchema) GetTableName() string { - return v.DDL.GetTable().Name.String() + return v.AlterVschemaDDL.Table.Name.String() } //Execute implements the Primitive interface func (v *AlterVSchema) Execute(vcursor VCursor, bindVars map[string]*query.BindVariable, wantfields bool) (*sqltypes.Result, error) { - err := vcursor.ExecuteVSchema(v.Keyspace.Name, v.DDL) + err := vcursor.ExecuteVSchema(v.Keyspace.Name, v.AlterVschemaDDL) if err != nil { return nil, err } diff --git a/go/vt/vtgate/planbuilder/builder.go b/go/vt/vtgate/planbuilder/builder.go index 4f4922573f..6ec2baf307 100644 --- a/go/vt/vtgate/planbuilder/builder.go +++ b/go/vt/vtgate/planbuilder/builder.go @@ -106,10 +106,9 @@ func createInstructionFor(query string, stmt sqlparser.Statement, vschema Contex case *sqlparser.Union: return buildRoutePlan(stmt, vschema, buildUnionPlan) case sqlparser.DDLStatement: - if sqlparser.IsVschemaDDL(stmt) { - return buildVSchemaDDLPlan(stmt, vschema) - } return buildGeneralDDLPlan(query, stmt, vschema) + case *sqlparser.AlterVschema: + return buildVSchemaDDLPlan(stmt, vschema) case *sqlparser.Use: return buildUsePlan(stmt, vschema) case *sqlparser.Explain: diff --git a/go/vt/vtgate/planbuilder/ddl.go b/go/vt/vtgate/planbuilder/ddl.go index 219f41775c..70a3c72928 100644 --- a/go/vt/vtgate/planbuilder/ddl.go +++ b/go/vt/vtgate/planbuilder/ddl.go @@ -108,17 +108,13 @@ func buildOnlineDDLPlan(query string, ddlStatement sqlparser.DDLStatement, vsche }, nil } -func buildVSchemaDDLPlan(ddlStmt sqlparser.DDLStatement, vschema ContextVSchema) (engine.Primitive, error) { - stmt, ok := ddlStmt.(*sqlparser.DDL) - if !ok { - return nil, vterrors.Errorf(vtrpc.Code_INTERNAL, "Incorrect type %T", ddlStmt) - } +func buildVSchemaDDLPlan(stmt *sqlparser.AlterVschema, vschema ContextVSchema) (engine.Primitive, error) { _, keyspace, _, err := vschema.TargetDestination(stmt.Table.Qualifier.String()) if err != nil { return nil, err } return &engine.AlterVSchema{ - Keyspace: keyspace, - DDL: stmt, + Keyspace: keyspace, + AlterVschemaDDL: stmt, }, nil } diff --git a/go/vt/vtgate/vcursor_impl.go b/go/vt/vtgate/vcursor_impl.go index 9eb612d634..662559b8b9 100644 --- a/go/vt/vtgate/vcursor_impl.go +++ b/go/vt/vtgate/vcursor_impl.go @@ -103,7 +103,7 @@ func (vc *vcursorImpl) GetKeyspace() string { return vc.keyspace } -func (vc *vcursorImpl) ExecuteVSchema(keyspace string, vschemaDDL sqlparser.DDLStatement) error { +func (vc *vcursorImpl) ExecuteVSchema(keyspace string, vschemaDDL *sqlparser.AlterVschema) error { srvVschema := vc.vm.GetCurrentSrvVschema() if srvVschema == nil { return vterrors.Errorf(vtrpcpb.Code_INTERNAL, "vschema not loaded") @@ -117,8 +117,8 @@ func (vc *vcursorImpl) ExecuteVSchema(keyspace string, vschemaDDL sqlparser.DDLS // Resolve the keyspace either from the table qualifier or the target keyspace var ksName string - if !vschemaDDL.GetTable().IsEmpty() { - ksName = vschemaDDL.GetTable().Qualifier.String() + if !vschemaDDL.Table.IsEmpty() { + ksName = vschemaDDL.Table.Qualifier.String() } if ksName == "" { ksName = keyspace From 2d1b007cf269dcfd962c57fdd29fbcd09b11221d Mon Sep 17 00:00:00 2001 From: GuptaManan100 Date: Mon, 14 Dec 2020 16:37:49 +0530 Subject: [PATCH 2/3] refactored code Signed-off-by: GuptaManan100 --- go/vt/vtgate/planbuilder/builder.go | 11 +++++++++++ go/vt/vtgate/planbuilder/ddl.go | 11 ----------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/go/vt/vtgate/planbuilder/builder.go b/go/vt/vtgate/planbuilder/builder.go index 6ec2baf307..23cd4ccda2 100644 --- a/go/vt/vtgate/planbuilder/builder.go +++ b/go/vt/vtgate/planbuilder/builder.go @@ -204,3 +204,14 @@ func buildLoadPlan(query string, vschema ContextVSchema) (engine.Primitive, erro SingleShardOnly: true, }, nil } + +func buildVSchemaDDLPlan(stmt *sqlparser.AlterVschema, vschema ContextVSchema) (engine.Primitive, error) { + _, keyspace, _, err := vschema.TargetDestination(stmt.Table.Qualifier.String()) + if err != nil { + return nil, err + } + return &engine.AlterVSchema{ + Keyspace: keyspace, + AlterVschemaDDL: stmt, + }, nil +} diff --git a/go/vt/vtgate/planbuilder/ddl.go b/go/vt/vtgate/planbuilder/ddl.go index 907f35272f..a8921e3f36 100644 --- a/go/vt/vtgate/planbuilder/ddl.go +++ b/go/vt/vtgate/planbuilder/ddl.go @@ -132,14 +132,3 @@ func buildDDLPlans(sql string, ddlStatement sqlparser.DDLStatement, vschema Cont SQL: query, }, nil } - -func buildVSchemaDDLPlan(stmt *sqlparser.AlterVschema, vschema ContextVSchema) (engine.Primitive, error) { - _, keyspace, _, err := vschema.TargetDestination(stmt.Table.Qualifier.String()) - if err != nil { - return nil, err - } - return &engine.AlterVSchema{ - Keyspace: keyspace, - AlterVschemaDDL: stmt, - }, nil -} From e1a35c6860a7a59e5d3d6a34de911348d2b5e010 Mon Sep 17 00:00:00 2001 From: GuptaManan100 Date: Mon, 14 Dec 2020 18:28:39 +0530 Subject: [PATCH 3/3] Added planning tests for Alter Vschema Signed-off-by: GuptaManan100 --- go/vt/sqlparser/analyzer.go | 2 +- go/vt/vtgate/planbuilder/plan_test.go | 1 + .../testdata/alterVschema_cases.txt | 134 ++++++++++++++++++ 3 files changed, 136 insertions(+), 1 deletion(-) create mode 100644 go/vt/vtgate/planbuilder/testdata/alterVschema_cases.txt diff --git a/go/vt/sqlparser/analyzer.go b/go/vt/sqlparser/analyzer.go index fde2d7e162..827ffd2439 100644 --- a/go/vt/sqlparser/analyzer.go +++ b/go/vt/sqlparser/analyzer.go @@ -76,7 +76,7 @@ func ASTToStatementType(stmt Statement) StatementType { return StmtSet case *Show: return StmtShow - case DDLStatement, DBDDLStatement: + case DDLStatement, DBDDLStatement, *AlterVschema: return StmtDDL case *Use: return StmtUse diff --git a/go/vt/vtgate/planbuilder/plan_test.go b/go/vt/vtgate/planbuilder/plan_test.go index 51d22e5077..1f2455938f 100644 --- a/go/vt/vtgate/planbuilder/plan_test.go +++ b/go/vt/vtgate/planbuilder/plan_test.go @@ -233,6 +233,7 @@ func TestWithDefaultKeyspaceFromFile(t *testing.T) { tabletType: topodatapb.TabletType_MASTER, } + testFile(t, "alterVschema_cases.txt", testOutputTempDir, vschema) testFile(t, "ddl_cases.txt", testOutputTempDir, vschema) testFile(t, "show_cases.txt", testOutputTempDir, vschema) } diff --git a/go/vt/vtgate/planbuilder/testdata/alterVschema_cases.txt b/go/vt/vtgate/planbuilder/testdata/alterVschema_cases.txt new file mode 100644 index 0000000000..91e071130d --- /dev/null +++ b/go/vt/vtgate/planbuilder/testdata/alterVschema_cases.txt @@ -0,0 +1,134 @@ +# Create vindex +"alter vschema create vindex hash_vdx using hash" +{ + "QueryType": "DDL", + "Original": "alter vschema create vindex hash_vdx using hash", + "Instructions": { + "OperatorType": "AlterVSchema", + "Keyspace": { + "Name": "main", + "Sharded": false + }, + "query": "alter vschema create vindex hash_vdx using hash" + } +} + +# Create vindex with qualifier +"alter vschema create vindex user.hash_vdx using hash" +{ + "QueryType": "DDL", + "Original": "alter vschema create vindex user.hash_vdx using hash", + "Instructions": { + "OperatorType": "AlterVSchema", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "query": "alter vschema create vindex user.hash_vdx using hash" + } +} + +# Drop vindex +"alter vschema drop vindex hash_vdx" +{ + "QueryType": "DDL", + "Original": "alter vschema drop vindex hash_vdx", + "Instructions": { + "OperatorType": "AlterVSchema", + "Keyspace": { + "Name": "main", + "Sharded": false + }, + "query": "alter vschema drop vindex hash_vdx" + } +} + +# Add table +"alter vschema add table a" +{ + "QueryType": "DDL", + "Original": "alter vschema add table a", + "Instructions": { + "OperatorType": "AlterVSchema", + "Keyspace": { + "Name": "main", + "Sharded": false + }, + "query": "alter vschema add table a" + } +} + +# Add sequence +"alter vschema add sequence a_seq" +{ + "QueryType": "DDL", + "Original": "alter vschema add sequence a_seq", + "Instructions": { + "OperatorType": "AlterVSchema", + "Keyspace": { + "Name": "main", + "Sharded": false + }, + "query": "alter vschema add sequence a_seq" + } +} + +# Add auto_increment with qualifier +"alter vschema on user.a add auto_increment id using a_seq" +{ + "QueryType": "DDL", + "Original": "alter vschema on user.a add auto_increment id using a_seq", + "Instructions": { + "OperatorType": "AlterVSchema", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "query": "alter vschema on user.a add auto_increment id using a_seq" + } +} + +# Drop table +"alter vschema drop table a" +{ + "QueryType": "DDL", + "Original": "alter vschema drop table a", + "Instructions": { + "OperatorType": "AlterVSchema", + "Keyspace": { + "Name": "main", + "Sharded": false + }, + "query": "alter vschema drop table a" + } +} + +# Add Vindex +"alter vschema on a add vindex hash (id)" +{ + "QueryType": "DDL", + "Original": "alter vschema on a add vindex hash (id)", + "Instructions": { + "OperatorType": "AlterVSchema", + "Keyspace": { + "Name": "main", + "Sharded": false + }, + "query": "alter vschema on a add vindex hash (id)" + } +} + +# Drop Vindex +"alter vschema on a drop vindex hash" +{ + "QueryType": "DDL", + "Original": "alter vschema on a drop vindex hash", + "Instructions": { + "OperatorType": "AlterVSchema", + "Keyspace": { + "Name": "main", + "Sharded": false + }, + "query": "alter vschema on a drop vindex hash" + } +}