From 26e2359ce30abc38a24690b62f5b14166b6e7367 Mon Sep 17 00:00:00 2001 From: Andres Taylor Date: Thu, 6 Jun 2024 15:12:40 +0200 Subject: [PATCH 1/3] refactor: cleaner cloning API Signed-off-by: Andres Taylor --- go/vt/sqlparser/ast_funcs.go | 5 +++++ go/vt/vtgate/planbuilder/operators/aggregation_pushing.go | 2 +- .../planbuilder/operators/aggregation_pushing_helper.go | 4 ++-- go/vt/vtgate/planbuilder/operators/delete.go | 4 ++-- go/vt/vtgate/planbuilder/operators/horizon.go | 2 +- go/vt/vtgate/planbuilder/operators/insert.go | 2 +- go/vt/vtgate/planbuilder/operators/limit.go | 2 +- go/vt/vtgate/planbuilder/operators/phases.go | 4 ++-- go/vt/vtgate/planbuilder/operators/projection.go | 2 +- go/vt/vtgate/planbuilder/operators/projection_pushing.go | 2 +- go/vt/vtgate/planbuilder/operators/query_planning.go | 4 ++-- go/vt/vtgate/planbuilder/operators/querygraph.go | 4 ++-- go/vt/vtgate/planbuilder/operators/subquery.go | 2 +- go/vt/vtgate/planbuilder/operators/subquery_builder.go | 2 +- go/vt/vtgate/planbuilder/operators/table.go | 2 +- go/vt/vtgate/planbuilder/operators/update.go | 4 ++-- go/vt/vtgate/planbuilder/operators/upsert.go | 2 +- 17 files changed, 27 insertions(+), 22 deletions(-) diff --git a/go/vt/sqlparser/ast_funcs.go b/go/vt/sqlparser/ast_funcs.go index df201676fae..df419e5d7a1 100644 --- a/go/vt/sqlparser/ast_funcs.go +++ b/go/vt/sqlparser/ast_funcs.go @@ -2776,3 +2776,8 @@ func (lock Lock) GetHighestOrderLock(newLock Lock) Lock { } return lock } + +// Clone returns a deep copy of the SQLNode, typed as the original type +func Clone[K SQLNode](x K) K { + return CloneSQLNode(x).(K) +} diff --git a/go/vt/vtgate/planbuilder/operators/aggregation_pushing.go b/go/vt/vtgate/planbuilder/operators/aggregation_pushing.go index 43a88d82871..67dd0c1d306 100644 --- a/go/vt/vtgate/planbuilder/operators/aggregation_pushing.go +++ b/go/vt/vtgate/planbuilder/operators/aggregation_pushing.go @@ -628,7 +628,7 @@ func splitAvgAggregations(ctx *plancontext.PlanningContext, aggr *Aggregator) (O outputColumn := aeWrap(col.Expr) outputColumn.As = sqlparser.NewIdentifierCI(col.ColumnName()) - proj.addUnexploredExpr(sqlparser.CloneRefOfAliasedExpr(col), calcExpr) + proj.addUnexploredExpr(sqlparser.Clone(col), calcExpr) col.Expr = sumExpr found := false for aggrOffset, aggregation := range aggr.Aggregations { diff --git a/go/vt/vtgate/planbuilder/operators/aggregation_pushing_helper.go b/go/vt/vtgate/planbuilder/operators/aggregation_pushing_helper.go index eb14f83b7df..2c47f426695 100644 --- a/go/vt/vtgate/planbuilder/operators/aggregation_pushing_helper.go +++ b/go/vt/vtgate/planbuilder/operators/aggregation_pushing_helper.go @@ -266,7 +266,7 @@ func (p *joinPusher) countStar(ctx *plancontext.PlanningContext) (*sqlparser.Ali // It returns the expression of the aggregation as it should be used in the parent Aggregator. func (p *joinPusher) addAggr(ctx *plancontext.PlanningContext, aggr Aggr) sqlparser.Expr { copyAggr := aggr - expr := sqlparser.CloneExpr(aggr.Original.Expr) + expr := sqlparser.Clone(aggr.Original.Expr) copyAggr.Original = aeWrap(expr) // copy dependencies so we can keep track of which side expressions need to be pushed to ctx.SemTable.Direct[expr] = p.tableID @@ -291,7 +291,7 @@ func (p *joinPusher) pushThroughAggr(aggr Aggr) { // It returns the expression of the GroupBy as it should be used in the parent Aggregator. func (p *joinPusher) addGrouping(ctx *plancontext.PlanningContext, gb GroupBy) sqlparser.Expr { copyGB := gb - expr := sqlparser.CloneExpr(gb.Inner) + expr := sqlparser.Clone(gb.Inner) // copy dependencies so we can keep track of which side expressions need to be pushed to ctx.SemTable.CopyDependencies(gb.Inner, expr) // if the column exists in the selection then copy it down to the pushed aggregator operator. diff --git a/go/vt/vtgate/planbuilder/operators/delete.go b/go/vt/vtgate/planbuilder/operators/delete.go index a3c45e79135..5bbf5218bd7 100644 --- a/go/vt/vtgate/planbuilder/operators/delete.go +++ b/go/vt/vtgate/planbuilder/operators/delete.go @@ -73,7 +73,7 @@ func createOperatorFromDelete(ctx *plancontext.PlanningContext, deleteStmt *sqlp return createDeleteWithInputOp(ctx, deleteStmt) } - delClone := sqlparser.CloneRefOfDelete(deleteStmt) + delClone := sqlparser.Clone(deleteStmt) var vTbl *vindexes.Table op, vTbl = createDeleteOperator(ctx, deleteStmt) @@ -315,7 +315,7 @@ func addOrdering(ctx *plancontext.PlanningContext, orderBy sqlparser.OrderBy, op continue } ordering.Order = append(ordering.Order, OrderBy{ - Inner: sqlparser.CloneRefOfOrder(order), + Inner: sqlparser.Clone(order), SimplifiedExpr: order.Expr, }) } diff --git a/go/vt/vtgate/planbuilder/operators/horizon.go b/go/vt/vtgate/planbuilder/operators/horizon.go index 532441d6a34..c28bc2dd5f0 100644 --- a/go/vt/vtgate/planbuilder/operators/horizon.go +++ b/go/vt/vtgate/planbuilder/operators/horizon.go @@ -59,7 +59,7 @@ func newHorizon(src Operator, query sqlparser.SelectStatement) *Horizon { func (h *Horizon) Clone(inputs []Operator) Operator { klone := *h klone.Source = inputs[0] - klone.ColumnAliases = sqlparser.CloneColumns(h.ColumnAliases) + klone.ColumnAliases = sqlparser.Clone(h.ColumnAliases) klone.Columns = slices.Clone(h.Columns) klone.ColumnsOffset = slices.Clone(h.ColumnsOffset) klone.QP = h.QP diff --git a/go/vt/vtgate/planbuilder/operators/insert.go b/go/vt/vtgate/planbuilder/operators/insert.go index 7c6e242ae9c..a009e14f99c 100644 --- a/go/vt/vtgate/planbuilder/operators/insert.go +++ b/go/vt/vtgate/planbuilder/operators/insert.go @@ -135,7 +135,7 @@ func createOperatorFromInsert(ctx *plancontext.PlanningContext, ins *sqlparser.I delStmt := &sqlparser.Delete{ Comments: ins.Comments, - TableExprs: sqlparser.TableExprs{sqlparser.CloneRefOfAliasedTableExpr(ins.Table)}, + TableExprs: sqlparser.TableExprs{sqlparser.Clone(ins.Table)}, Where: sqlparser.NewWhere(sqlparser.WhereClause, whereExpr), } delOp := createOpFromStmt(ctx, delStmt, false, "") diff --git a/go/vt/vtgate/planbuilder/operators/limit.go b/go/vt/vtgate/planbuilder/operators/limit.go index 0d4857d5aaa..1801e57c1c9 100644 --- a/go/vt/vtgate/planbuilder/operators/limit.go +++ b/go/vt/vtgate/planbuilder/operators/limit.go @@ -36,7 +36,7 @@ type Limit struct { func (l *Limit) Clone(inputs []Operator) Operator { return &Limit{ Source: inputs[0], - AST: sqlparser.CloneRefOfLimit(l.AST), + AST: sqlparser.Clone(l.AST), Top: l.Top, Pushed: l.Pushed, } diff --git a/go/vt/vtgate/planbuilder/operators/phases.go b/go/vt/vtgate/planbuilder/operators/phases.go index 3864b514aa9..9f2178bae05 100644 --- a/go/vt/vtgate/planbuilder/operators/phases.go +++ b/go/vt/vtgate/planbuilder/operators/phases.go @@ -176,8 +176,8 @@ func createDMLWithInput(ctx *plancontext.PlanningContext, op, src Operator, in * targetQT := targetTable.QTable qt := &QueryTable{ ID: targetQT.ID, - Alias: sqlparser.CloneRefOfAliasedTableExpr(targetQT.Alias), - Table: sqlparser.CloneTableName(targetQT.Table), + Alias: sqlparser.Clone(targetQT.Alias), + Table: sqlparser.Clone(targetQT.Table), Predicates: []sqlparser.Expr{compExpr}, } diff --git a/go/vt/vtgate/planbuilder/operators/projection.go b/go/vt/vtgate/planbuilder/operators/projection.go index 41b83c8f7fe..869cacc005a 100644 --- a/go/vt/vtgate/planbuilder/operators/projection.go +++ b/go/vt/vtgate/planbuilder/operators/projection.go @@ -111,7 +111,7 @@ type ( func newProjExpr(ae *sqlparser.AliasedExpr) *ProjExpr { return &ProjExpr{ - Original: sqlparser.CloneRefOfAliasedExpr(ae), + Original: sqlparser.Clone(ae), EvalExpr: ae.Expr, ColExpr: ae.Expr, } diff --git a/go/vt/vtgate/planbuilder/operators/projection_pushing.go b/go/vt/vtgate/planbuilder/operators/projection_pushing.go index 56f829fc7f5..d1baf6def62 100644 --- a/go/vt/vtgate/planbuilder/operators/projection_pushing.go +++ b/go/vt/vtgate/planbuilder/operators/projection_pushing.go @@ -327,7 +327,7 @@ func splitUnexploredExpression( alias string, dt *DerivedTable, ) applyJoinColumn { - original := sqlparser.CloneRefOfAliasedExpr(pe.Original) + original := sqlparser.Clone(pe.Original) expr := pe.ColExpr var colName *sqlparser.ColName diff --git a/go/vt/vtgate/planbuilder/operators/query_planning.go b/go/vt/vtgate/planbuilder/operators/query_planning.go index f2625bcb90b..b20cad7c125 100644 --- a/go/vt/vtgate/planbuilder/operators/query_planning.go +++ b/go/vt/vtgate/planbuilder/operators/query_planning.go @@ -34,7 +34,7 @@ func planQuery(ctx *plancontext.PlanningContext, root Operator) Operator { var selExpr sqlparser.SelectExprs if horizon, isHorizon := root.(*Horizon); isHorizon { sel := sqlparser.GetFirstSelect(horizon.Query) - selExpr = sqlparser.CloneSelectExprs(sel.SelectExprs) + selExpr = sqlparser.Clone(sel.SelectExprs) } output := runPhases(ctx, root) @@ -252,7 +252,7 @@ func tryPushLimit(ctx *plancontext.PlanningContext, in *Limit) (Operator, *Apply } func createPushedLimit(ctx *plancontext.PlanningContext, src Operator, orig *Limit) Operator { - pushedLimit := sqlparser.CloneRefOfLimit(orig.AST) + pushedLimit := sqlparser.Clone(orig.AST) if pushedLimit.Offset != nil { // we can't push down an offset, so we need to convert it to a rowcount // by adding it to the already existing rowcount, and then let the LIMIT running on the vtgate do the rest diff --git a/go/vt/vtgate/planbuilder/operators/querygraph.go b/go/vt/vtgate/planbuilder/operators/querygraph.go index bc731f29df6..8e8572f7dfa 100644 --- a/go/vt/vtgate/planbuilder/operators/querygraph.go +++ b/go/vt/vtgate/planbuilder/operators/querygraph.go @@ -190,8 +190,8 @@ func (qg *QueryGraph) AddPredicate(ctx *plancontext.PlanningContext, expr sqlpar func (qt *QueryTable) Clone() *QueryTable { return &QueryTable{ ID: qt.ID, - Alias: sqlparser.CloneRefOfAliasedTableExpr(qt.Alias), - Table: sqlparser.CloneTableName(qt.Table), + Alias: sqlparser.Clone(qt.Alias), + Table: sqlparser.Clone(qt.Table), Predicates: qt.Predicates, IsInfSchema: qt.IsInfSchema, } diff --git a/go/vt/vtgate/planbuilder/operators/subquery.go b/go/vt/vtgate/planbuilder/operators/subquery.go index 03a482185d8..e2f046f4a4d 100644 --- a/go/vt/vtgate/planbuilder/operators/subquery.go +++ b/go/vt/vtgate/planbuilder/operators/subquery.go @@ -124,7 +124,7 @@ func (sq *SubQuery) Clone(inputs []Operator) Operator { } klone.JoinColumns = slices.Clone(sq.JoinColumns) klone.Vars = maps.Clone(sq.Vars) - klone.Predicates = sqlparser.CloneExprs(sq.Predicates) + klone.Predicates = sqlparser.Clone(sq.Predicates) return &klone } diff --git a/go/vt/vtgate/planbuilder/operators/subquery_builder.go b/go/vt/vtgate/planbuilder/operators/subquery_builder.go index 4caf3530075..f2fdefeb007 100644 --- a/go/vt/vtgate/planbuilder/operators/subquery_builder.go +++ b/go/vt/vtgate/planbuilder/operators/subquery_builder.go @@ -297,7 +297,7 @@ func (sqb *SubQueryBuilder) pullOutValueSubqueries( outerID semantics.TableSet, isDML bool, ) (sqlparser.Expr, []*SubQuery) { - original := sqlparser.CloneExpr(expr) + original := sqlparser.Clone(expr) sqe := extractSubQueries(ctx, expr, isDML) if sqe == nil { return nil, nil diff --git a/go/vt/vtgate/planbuilder/operators/table.go b/go/vt/vtgate/planbuilder/operators/table.go index 14207fe3b3e..3ecd4982ece 100644 --- a/go/vt/vtgate/planbuilder/operators/table.go +++ b/go/vt/vtgate/planbuilder/operators/table.go @@ -45,7 +45,7 @@ type ( func (to *Table) Clone([]Operator) Operator { var columns []*sqlparser.ColName for _, name := range to.Columns { - columns = append(columns, sqlparser.CloneRefOfColName(name)) + columns = append(columns, sqlparser.Clone(name)) } return &Table{ QTable: to.QTable, diff --git a/go/vt/vtgate/planbuilder/operators/update.go b/go/vt/vtgate/planbuilder/operators/update.go index 4abf319ad08..ba83ad7efaf 100644 --- a/go/vt/vtgate/planbuilder/operators/update.go +++ b/go/vt/vtgate/planbuilder/operators/update.go @@ -338,7 +338,7 @@ func createUpdateOperator(ctx *plancontext.PlanningContext, updStmt *sqlparser.U // updClone is used in foreign key planning to create the selection statements to be used for verification and selection. // If we encounter subqueries, we want to fix the updClone to use the replaced expression, so that the pulled out subquery's // result is used everywhere instead of running the subquery multiple times, which is wasteful. - updClone := sqlparser.CloneRefOfUpdate(updStmt) + updClone := sqlparser.Clone(updStmt) var tblInfo semantics.TableInfo var err error for idx, updExpr := range updStmt.Exprs { @@ -346,7 +346,7 @@ func createUpdateOperator(ctx *plancontext.PlanningContext, updStmt *sqlparser.U if len(subqs) == 0 { expr = updExpr.Expr } else { - updClone.Exprs[idx].Expr = sqlparser.CloneExpr(expr) + updClone.Exprs[idx].Expr = sqlparser.Clone(expr) ctx.SemTable.UpdateChildFKExpr(updExpr, expr) } proj := newProjExpr(aeWrap(expr)) diff --git a/go/vt/vtgate/planbuilder/operators/upsert.go b/go/vt/vtgate/planbuilder/operators/upsert.go index 8f028a790b2..bfe2a1cbc52 100644 --- a/go/vt/vtgate/planbuilder/operators/upsert.go +++ b/go/vt/vtgate/planbuilder/operators/upsert.go @@ -129,7 +129,7 @@ func createUpsertOperator(ctx *plancontext.PlanningContext, ins *sqlparser.Inser updOp := createOpFromStmt(ctx, upd, false, "") // replan insert statement without on duplicate key update. - newInsert := sqlparser.CloneRefOfInsert(ins) + newInsert := sqlparser.Clone(ins) newInsert.OnDup = nil newInsert.Rows = sqlparser.Values{row} insOp = createOpFromStmt(ctx, newInsert, false, "") From d0df62186fffc77d93ffe3cfbfe75e5f9f190405 Mon Sep 17 00:00:00 2001 From: Andres Taylor Date: Fri, 7 Jun 2024 07:15:37 +0200 Subject: [PATCH 2/3] refactor: update more Clone uses Signed-off-by: Andres Taylor --- .../vtgate/queries/random/query_gen.go | 2 +- go/test/fuzzing/ast_fuzzer.go | 2 +- go/vt/schema/online_ddl.go | 2 +- go/vt/schemadiff/capability.go | 2 +- go/vt/schemadiff/schema.go | 6 ++-- go/vt/schemadiff/table.go | 14 +++++----- go/vt/schemadiff/view.go | 8 +++--- go/vt/vtgate/executor.go | 2 +- go/vt/vtgate/planbuilder/ddl.go | 2 +- go/vt/vtgate/planbuilder/simplifier_test.go | 4 +-- go/vt/vtgate/semantics/early_rewriter.go | 2 +- go/vt/vtgate/semantics/semantic_state.go | 2 +- .../simplifier/expression_simplifier.go | 8 +++--- go/vt/vtgate/simplifier/simplifier.go | 28 +++++++++---------- go/vt/vtgate/vindexes/vschema.go | 2 +- go/vt/vtgate/vschema_manager.go | 2 +- go/vt/vttablet/onlineddl/executor.go | 6 ++-- 17 files changed, 47 insertions(+), 47 deletions(-) diff --git a/go/test/endtoend/vtgate/queries/random/query_gen.go b/go/test/endtoend/vtgate/queries/random/query_gen.go index b078f1cab8b..ba7c83aaa91 100644 --- a/go/test/endtoend/vtgate/queries/random/query_gen.go +++ b/go/test/endtoend/vtgate/queries/random/query_gen.go @@ -132,7 +132,7 @@ func (t *tableT) addColumns(col ...column) { func (t *tableT) clone() *tableT { return &tableT{ - tableExpr: sqlparser.CloneSimpleTableExpr(t.tableExpr), + tableExpr: sqlparser.Clone(t.tableExpr), alias: t.alias, cols: slices.Clone(t.cols), } diff --git a/go/test/fuzzing/ast_fuzzer.go b/go/test/fuzzing/ast_fuzzer.go index 5951a0da9eb..278b20df588 100644 --- a/go/test/fuzzing/ast_fuzzer.go +++ b/go/test/fuzzing/ast_fuzzer.go @@ -61,7 +61,7 @@ func FuzzEqualsSQLNode(data []byte) int { } // Target 2: - newSQLNode := sqlparser.CloneSQLNode(inA) + newSQLNode := sqlparser.Clone(inA) if !sqlparser.EqualsSQLNode(inA, newSQLNode) { panic("These two nodes should be identical") } diff --git a/go/vt/schema/online_ddl.go b/go/vt/schema/online_ddl.go index 57ed075cf38..7b8647f86b7 100644 --- a/go/vt/schema/online_ddl.go +++ b/go/vt/schema/online_ddl.go @@ -288,7 +288,7 @@ func OnlineDDLFromCommentedStatement(stmt sqlparser.Statement) (onlineDDL *Onlin // We clone the comments because they will end up being cached by the query planner. Then, the Directive() function actually modifies the comments. // If comments are shared in cache, and Directive() modifies it, then we have a concurrency issue when someone else wants to read the comments. // By cloning the comments we remove the concurrency problem. - comments = sqlparser.CloneRefOfParsedComments(comments) + comments = sqlparser.Clone(comments) comments.ResetDirectives() if comments.Length() == 0 { diff --git a/go/vt/schemadiff/capability.go b/go/vt/schemadiff/capability.go index 9ce985c71d9..cde99ac18c3 100644 --- a/go/vt/schemadiff/capability.go +++ b/go/vt/schemadiff/capability.go @@ -74,7 +74,7 @@ func alterOptionCapableOfInstantDDL(alterOption sqlparser.AlterOption, createTab return true, col.Type.Options.Storage } colStringStrippedDown := func(col *sqlparser.ColumnDefinition, stripDefault bool, stripEnum bool) string { - strippedCol := sqlparser.CloneRefOfColumnDefinition(col) + strippedCol := sqlparser.Clone(col) if stripDefault { strippedCol.Type.Options.Default = nil strippedCol.Type.Options.DefaultLiteral = false diff --git a/go/vt/schemadiff/schema.go b/go/vt/schemadiff/schema.go index efad76d9a33..034b784ffb5 100644 --- a/go/vt/schemadiff/schema.go +++ b/go/vt/schemadiff/schema.go @@ -1014,7 +1014,7 @@ func (s *Schema) ValidateViewReferences() error { schemaInformation.addTable("dual") for _, view := range s.Views() { - sel := sqlparser.CloneSelectStatement(view.CreateView.Select) // Analyze(), below, rewrites the select; we don't want to actually modify the schema + sel := sqlparser.Clone(view.CreateView.Select) // Analyze(), below, rewrites the select; we don't want to actually modify the schema _, err := semantics.AnalyzeStrict(sel, semanticKS.Name, schemaInformation) formalizeErr := func(err error) error { if err == nil { @@ -1079,7 +1079,7 @@ func (s *Schema) getViewColumnNames(v *CreateViewEntity, schemaInformation *decl case *sqlparser.StarExpr: if tableName := node.TableName.Name.String(); tableName != "" { for _, col := range schemaInformation.Tables[tableName].Columns { - name := sqlparser.CloneRefOfIdentifierCI(&col.Name) + name := sqlparser.Clone(&col.Name) columnNames = append(columnNames, name) } } else { @@ -1088,7 +1088,7 @@ func (s *Schema) getViewColumnNames(v *CreateViewEntity, schemaInformation *decl for _, entityName := range dependentNames { if schemaInformation.Tables[entityName] != nil { // is nil for dual/DUAL for _, col := range schemaInformation.Tables[entityName].Columns { - name := sqlparser.CloneRefOfIdentifierCI(&col.Name) + name := sqlparser.Clone(&col.Name) columnNames = append(columnNames, name) } } diff --git a/go/vt/schemadiff/table.go b/go/vt/schemadiff/table.go index def83fa7f19..5d21d524704 100644 --- a/go/vt/schemadiff/table.go +++ b/go/vt/schemadiff/table.go @@ -146,7 +146,7 @@ func (d *AlterTableEntityDiff) Clone() EntityDiff { } ann := *d.annotations clone := &AlterTableEntityDiff{ - alterTable: sqlparser.CloneRefOfAlterTable(d.alterTable), + alterTable: sqlparser.Clone(d.alterTable), instantDDLCapability: d.instantDDLCapability, annotations: &ann, } @@ -245,7 +245,7 @@ func (d *CreateTableEntityDiff) Clone() EntityDiff { return nil } clone := &CreateTableEntityDiff{ - createTable: sqlparser.CloneRefOfCreateTable(d.createTable), + createTable: sqlparser.Clone(d.createTable), } if d.to != nil { clone.to = d.to.Clone().(*CreateTableEntity) @@ -336,7 +336,7 @@ func (d *DropTableEntityDiff) Clone() EntityDiff { return nil } clone := &DropTableEntityDiff{ - dropTable: sqlparser.CloneRefOfDropTable(d.dropTable), + dropTable: sqlparser.Clone(d.dropTable), } if d.from != nil { clone.from = d.from.Clone().(*CreateTableEntity) @@ -428,7 +428,7 @@ func (d *RenameTableEntityDiff) Clone() EntityDiff { return nil } clone := &RenameTableEntityDiff{ - renameTable: sqlparser.CloneRefOfRenameTable(d.renameTable), + renameTable: sqlparser.Clone(d.renameTable), } if d.from != nil { clone.from = d.from.Clone().(*CreateTableEntity) @@ -526,7 +526,7 @@ func (c *CreateTableEntity) GetCollation() string { } func (c *CreateTableEntity) Clone() Entity { - return &CreateTableEntity{CreateTable: sqlparser.CloneRefOfCreateTable(c.CreateTable), Env: c.Env} + return &CreateTableEntity{CreateTable: sqlparser.Clone(c.CreateTable), Env: c.Env} } func getTableCharsetCollate(env *Environment, tableOptions *sqlparser.TableOptions) *charsetCollate { @@ -1601,8 +1601,8 @@ func (c *CreateTableEntity) diffKeys(alterTable *sqlparser.AlterTable, // Returns if this is a visibility only change and if true, whether // the new visibility is invisible or not. func indexOnlyVisibilityChange(t1Key, t2Key *sqlparser.IndexDefinition) (bool, bool) { - t1KeyCopy := sqlparser.CloneRefOfIndexDefinition(t1Key) - t2KeyCopy := sqlparser.CloneRefOfIndexDefinition(t2Key) + t1KeyCopy := sqlparser.Clone(t1Key) + t2KeyCopy := sqlparser.Clone(t2Key) t1KeyKeptOptions := make([]*sqlparser.IndexOption, 0, len(t1KeyCopy.Options)) t2KeyInvisible := false for _, opt := range t1KeyCopy.Options { diff --git a/go/vt/schemadiff/view.go b/go/vt/schemadiff/view.go index c4d48ac66cc..d2dc4dfb76f 100644 --- a/go/vt/schemadiff/view.go +++ b/go/vt/schemadiff/view.go @@ -106,7 +106,7 @@ func (d *AlterViewEntityDiff) Clone() EntityDiff { return nil } clone := &AlterViewEntityDiff{ - alterView: sqlparser.CloneRefOfAlterView(d.alterView), + alterView: sqlparser.Clone(d.alterView), } if d.from != nil { clone.from = d.from.Clone().(*CreateViewEntity) @@ -200,7 +200,7 @@ func (d *CreateViewEntityDiff) Clone() EntityDiff { return nil } return &CreateViewEntityDiff{ - createView: sqlparser.CloneRefOfCreateView(d.createView), + createView: sqlparser.Clone(d.createView), } } @@ -287,7 +287,7 @@ func (d *DropViewEntityDiff) Clone() EntityDiff { return nil } clone := &DropViewEntityDiff{ - dropView: sqlparser.CloneRefOfDropView(d.dropView), + dropView: sqlparser.Clone(d.dropView), } if d.from != nil { clone.from = d.from.Clone().(*CreateViewEntity) @@ -402,7 +402,7 @@ func (c *CreateViewEntity) Apply(diff EntityDiff) (Entity, error) { } func (c *CreateViewEntity) Clone() Entity { - return &CreateViewEntity{CreateView: sqlparser.CloneRefOfCreateView(c.CreateView)} + return &CreateViewEntity{CreateView: sqlparser.Clone(c.CreateView)} } func (c *CreateViewEntity) identicalOtherThanName(other *CreateViewEntity) bool { diff --git a/go/vt/vtgate/executor.go b/go/vt/vtgate/executor.go index 2679cf5a2fd..cec0dba8274 100644 --- a/go/vt/vtgate/executor.go +++ b/go/vt/vtgate/executor.go @@ -1592,7 +1592,7 @@ func (e *Executor) planPrepareStmt(ctx context.Context, vcursor *vcursorImpl, qu ctx, vcursor, query, - sqlparser.CloneStatement(stmt), + sqlparser.Clone(stmt), vcursor.marginComments, map[string]*querypb.BindVariable{}, reservedVars, /* normalize */ diff --git a/go/vt/vtgate/planbuilder/ddl.go b/go/vt/vtgate/planbuilder/ddl.go index 351bf42672c..4c4b3791c20 100644 --- a/go/vt/vtgate/planbuilder/ddl.go +++ b/go/vt/vtgate/planbuilder/ddl.go @@ -210,7 +210,7 @@ func buildCreateViewCommon( // because we don't trust the schema tracker to have up-to-date info, we don't want to expand any SELECT * here var expressions []sqlparser.SelectExprs _ = sqlparser.VisitAllSelects(ddlSelect, func(p *sqlparser.Select, idx int) error { - expressions = append(expressions, sqlparser.CloneSelectExprs(p.SelectExprs)) + expressions = append(expressions, sqlparser.Clone(p.SelectExprs)) return nil }) selectPlan, err := createInstructionFor(ctx, sqlparser.String(ddlSelect), ddlSelect, reservedVars, vschema, enableOnlineDDL, enableDirectDDL) diff --git a/go/vt/vtgate/planbuilder/simplifier_test.go b/go/vt/vtgate/planbuilder/simplifier_test.go index 61ed220bd44..305c18896e3 100644 --- a/go/vt/vtgate/planbuilder/simplifier_test.go +++ b/go/vt/vtgate/planbuilder/simplifier_test.go @@ -45,7 +45,7 @@ func TestSimplifyBuggyQuery(t *testing.T) { } stmt, reserved, err := sqlparser.NewTestParser().Parse2(query) require.NoError(t, err) - rewritten, _ := sqlparser.RewriteAST(sqlparser.CloneStatement(stmt), vschema.CurrentDb(), sqlparser.SQLSelectLimitUnset, "", nil, nil, nil) + rewritten, _ := sqlparser.RewriteAST(sqlparser.Clone(stmt), vschema.CurrentDb(), sqlparser.SQLSelectLimitUnset, "", nil, nil, nil) reservedVars := sqlparser.NewReservedVars("vtg", reserved) simplified := simplifier.SimplifyStatement( @@ -68,7 +68,7 @@ func TestSimplifyPanic(t *testing.T) { } stmt, reserved, err := sqlparser.NewTestParser().Parse2(query) require.NoError(t, err) - rewritten, _ := sqlparser.RewriteAST(sqlparser.CloneStatement(stmt), vschema.CurrentDb(), sqlparser.SQLSelectLimitUnset, "", nil, nil, nil) + rewritten, _ := sqlparser.RewriteAST(sqlparser.Clone(stmt), vschema.CurrentDb(), sqlparser.SQLSelectLimitUnset, "", nil, nil, nil) reservedVars := sqlparser.NewReservedVars("vtg", reserved) simplified := simplifier.SimplifyStatement( diff --git a/go/vt/vtgate/semantics/early_rewriter.go b/go/vt/vtgate/semantics/early_rewriter.go index 51ed110adf9..568e1900d44 100644 --- a/go/vt/vtgate/semantics/early_rewriter.go +++ b/go/vt/vtgate/semantics/early_rewriter.go @@ -493,7 +493,7 @@ func (r *earlyRewriter) rewriteAliasesInGroupBy(node sqlparser.Expr, sel *sqlpar return } - cursor.Replace(sqlparser.CloneExpr(item.expr)) + cursor.Replace(sqlparser.Clone(item.expr)) } }, nil) diff --git a/go/vt/vtgate/semantics/semantic_state.go b/go/vt/vtgate/semantics/semantic_state.go index c81442a6c5b..bedb9105116 100644 --- a/go/vt/vtgate/semantics/semantic_state.go +++ b/go/vt/vtgate/semantics/semantic_state.go @@ -966,7 +966,7 @@ func (st *SemTable) Clone(n sqlparser.SQLNode) sqlparser.SQLNode { if !isExpr { return } - cursor.Replace(sqlparser.CloneExpr(expr)) + cursor.Replace(sqlparser.Clone(expr)) }, st.CopySemanticInfo) } diff --git a/go/vt/vtgate/simplifier/expression_simplifier.go b/go/vt/vtgate/simplifier/expression_simplifier.go index 86e3471baea..b64402cfaac 100644 --- a/go/vt/vtgate/simplifier/expression_simplifier.go +++ b/go/vt/vtgate/simplifier/expression_simplifier.go @@ -29,14 +29,14 @@ type CheckF = func(sqlparser.Expr) bool func SimplifyExpr(in sqlparser.Expr, test CheckF) sqlparser.Expr { // since we can't rewrite the top level, wrap the expr in an Exprs object - smallestKnown := sqlparser.Exprs{sqlparser.CloneExpr(in)} + smallestKnown := sqlparser.Exprs{sqlparser.Clone(in)} alwaysVisit := func(node, parent sqlparser.SQLNode) bool { return true } up := func(cursor *sqlparser.Cursor) bool { - node := sqlparser.CloneSQLNode(cursor.Node()) + node := sqlparser.Clone(cursor.Node()) s := &shrinker{orig: node} expr := s.Next() for expr != nil { @@ -57,7 +57,7 @@ func SimplifyExpr(in sqlparser.Expr, test CheckF) sqlparser.Expr { // loop until rewriting introduces no more changes for { - prevSmallest := sqlparser.CloneExprs(smallestKnown) + prevSmallest := sqlparser.Clone(smallestKnown) sqlparser.SafeRewrite(smallestKnown, alwaysVisit, up) if sqlparser.Equals.Exprs(prevSmallest, smallestKnown) { break @@ -185,7 +185,7 @@ func (s *shrinker) fillQueue() bool { s.queue = append(s.queue, ae) } - clone := sqlparser.CloneAggrFunc(e) + clone := sqlparser.Clone(e) if da, ok := clone.(sqlparser.DistinctableAggr); ok { if da.IsDistinct() { da.SetDistinct(false) diff --git a/go/vt/vtgate/simplifier/simplifier.go b/go/vt/vtgate/simplifier/simplifier.go index e96660c99ec..e838450e3a2 100644 --- a/go/vt/vtgate/simplifier/simplifier.go +++ b/go/vt/vtgate/simplifier/simplifier.go @@ -37,31 +37,31 @@ func SimplifyStatement( test := func(s sqlparser.SelectStatement) bool { // Since our semantic analysis changes the AST, we clone it first, so we have a pristine AST to play with - return testF(sqlparser.CloneSelectStatement(s)) + return testF(sqlparser.Clone(s)) } // first we try to simplify the query by removing any unions - if success := trySimplifyUnions(sqlparser.CloneSelectStatement(in), test); success != nil { + if success := trySimplifyUnions(sqlparser.Clone(in), test); success != nil { return SimplifyStatement(success, currentDB, si, testF) } // then we try to remove a table and all uses of it - if success := tryRemoveTable(tables, sqlparser.CloneSelectStatement(in), currentDB, si, testF); success != nil { + if success := tryRemoveTable(tables, sqlparser.Clone(in), currentDB, si, testF); success != nil { return SimplifyStatement(success, currentDB, si, testF) } // now let's try to simplify * expressions - if success := simplifyStarExpr(sqlparser.CloneSelectStatement(in), test); success != nil { + if success := simplifyStarExpr(sqlparser.Clone(in), test); success != nil { return SimplifyStatement(success, currentDB, si, testF) } // we try to remove/replace any expressions next - if success := trySimplifyExpressions(sqlparser.CloneSelectStatement(in), test); success != nil { + if success := trySimplifyExpressions(sqlparser.Clone(in), test); success != nil { return SimplifyStatement(success, currentDB, si, testF) } // we try to remove distinct last - if success := trySimplifyDistinct(sqlparser.CloneSelectStatement(in), test); success != nil { + if success := trySimplifyDistinct(sqlparser.Clone(in), test); success != nil { return SimplifyStatement(success, currentDB, si, testF) } @@ -144,10 +144,10 @@ func trySimplifyExpressions(in sqlparser.SelectStatement, test func(sqlparser.Se func trySimplifyUnions(in sqlparser.SelectStatement, test func(sqlparser.SelectStatement) bool) (res sqlparser.SelectStatement) { if union, ok := in.(*sqlparser.Union); ok { // the root object is an UNION - if test(sqlparser.CloneSelectStatement(union.Left)) { + if test(sqlparser.Clone(union.Left)) { return union.Left } - if test(sqlparser.CloneSelectStatement(union.Right)) { + if test(sqlparser.Clone(union.Right)) { return union.Right } } @@ -165,14 +165,14 @@ func trySimplifyUnions(in sqlparser.SelectStatement, test func(sqlparser.SelectS return true } cursor.Replace(node.Left) - clone := sqlparser.CloneSelectStatement(in) + clone := sqlparser.Clone(in) if test(clone) { log.Errorf("replaced UNION with its left child: %s -> %s", sqlparser.String(node), sqlparser.String(node.Left)) simplified = true return true } cursor.Replace(node.Right) - clone = sqlparser.CloneSelectStatement(in) + clone = sqlparser.Clone(in) if test(clone) { log.Errorf("replaced UNION with its right child: %s -> %s", sqlparser.String(node), sqlparser.String(node.Right)) simplified = true @@ -196,7 +196,7 @@ func trySimplifyUnions(in sqlparser.SelectStatement, test func(sqlparser.SelectS func tryRemoveTable(tables []semantics.TableInfo, in sqlparser.SelectStatement, currentDB string, si semantics.SchemaInformation, test func(sqlparser.SelectStatement) bool) sqlparser.SelectStatement { // we start by removing one table at a time, and see if we still have an interesting plan for idx, tbl := range tables { - clone := sqlparser.CloneSelectStatement(in) + clone := sqlparser.Clone(in) searchedTS := semantics.SingleTableSet(idx) simplified := removeTable(clone, searchedTS, currentDB, si) name, _ := tbl.Name() @@ -211,7 +211,7 @@ func tryRemoveTable(tables []semantics.TableInfo, in sqlparser.SelectStatement, func getTables(in sqlparser.SelectStatement, currentDB string, si semantics.SchemaInformation) ([]semantics.TableInfo, error) { // Since our semantic analysis changes the AST, we clone it first, so we have a pristine AST to play with - clone := sqlparser.CloneSelectStatement(in) + clone := sqlparser.Clone(in) semTable, err := semantics.Analyze(clone, currentDB, si) if err != nil { return nil, err @@ -467,7 +467,7 @@ func visitSelectExprs(node sqlparser.SelectExprs, cursor *sqlparser.Cursor, visi continue } removed := false - original := sqlparser.CloneExpr(expr.Expr) + original := sqlparser.Clone(expr.Expr) item := newExprCursor( expr.Expr, /*replace*/ func(replaceWith sqlparser.Expr) { @@ -561,7 +561,7 @@ func visitOrderBy(node sqlparser.OrderBy, cursor *sqlparser.Cursor, visit func(e for idx := 0; idx < len(node); idx++ { order := node[idx] removed := false - original := sqlparser.CloneExpr(order.Expr) + original := sqlparser.Clone(order.Expr) item := newExprCursor( order.Expr, /*replace*/ func(replaceWith sqlparser.Expr) { diff --git a/go/vt/vtgate/vindexes/vschema.go b/go/vt/vtgate/vindexes/vschema.go index 020b07f7073..924a28b309d 100644 --- a/go/vt/vtgate/vindexes/vschema.go +++ b/go/vt/vtgate/vindexes/vschema.go @@ -1242,7 +1242,7 @@ func (vschema *VSchema) FindView(keyspace, name string) sqlparser.SelectStatemen } // We do this to make sure there is no shared state between uses of this AST - statement = sqlparser.CloneSelectStatement(statement) + statement = sqlparser.Clone(statement) sqlparser.SafeRewrite(statement, nil, func(cursor *sqlparser.Cursor) bool { col, ok := cursor.Node().(*sqlparser.ColName) if ok { diff --git a/go/vt/vtgate/vschema_manager.go b/go/vt/vtgate/vschema_manager.go index dbac5589ce8..c73266af769 100644 --- a/go/vt/vtgate/vschema_manager.go +++ b/go/vt/vtgate/vschema_manager.go @@ -213,7 +213,7 @@ func (vm *VSchemaManager) updateViewInfo(ks *vindexes.KeyspaceSchema, ksName str if views != nil { ks.Views = make(map[string]sqlparser.SelectStatement, len(views)) for name, def := range views { - ks.Views[name] = sqlparser.CloneSelectStatement(def) + ks.Views[name] = sqlparser.Clone(def) } } } diff --git a/go/vt/vttablet/onlineddl/executor.go b/go/vt/vttablet/onlineddl/executor.go index ebce8f0619c..2fff13d3f73 100644 --- a/go/vt/vttablet/onlineddl/executor.go +++ b/go/vt/vttablet/onlineddl/executor.go @@ -1402,7 +1402,7 @@ func (e *Executor) duplicateCreateTable(ctx context.Context, onlineDDL *schema.O if !ok { return nil, nil, nil, vterrors.Errorf(vtrpcpb.Code_INTERNAL, "expected CreateTable statement, got: %v", sqlparser.CanonicalString(stmt)) } - newCreateTable = sqlparser.CloneRefOfCreateTable(originalCreateTable) + newCreateTable = sqlparser.Clone(originalCreateTable) newCreateTable.SetTable(newCreateTable.GetTable().Qualifier.CompliantName(), newTableName) // manipulate CreateTable statement: take care of constraints names which have to be // unique across the schema @@ -3033,7 +3033,7 @@ func (e *Executor) executeCreateDDLActionMigration(ctx context.Context, onlineDD } } if originalCreateTable, ok := ddlStmt.(*sqlparser.CreateTable); ok { - newCreateTable := sqlparser.CloneRefOfCreateTable(originalCreateTable) + newCreateTable := sqlparser.Clone(originalCreateTable) // Rewrite this CREATE TABLE statement such that CONSTRAINT names are edited, // specifically removing any prefix. if _, err := e.validateAndEditCreateTableStatement(onlineDDL, newCreateTable); err != nil { @@ -3117,7 +3117,7 @@ func (e *Executor) executeAlterViewOnline(ctx context.Context, onlineDDL *schema Select: viewStmt.Select, CheckOption: viewStmt.CheckOption, IsReplace: true, - Comments: sqlparser.CloneRefOfParsedComments(viewStmt.Comments), + Comments: sqlparser.Clone(viewStmt.Comments), } stmt.SetTable("", artifactViewName) default: From 9d78ad367303b39514e753d55d541d05747288dd Mon Sep 17 00:00:00 2001 From: Andres Taylor Date: Fri, 7 Jun 2024 07:35:07 +0200 Subject: [PATCH 3/3] fix: only clone types that implement SQLNode Signed-off-by: Andres Taylor --- go/vt/schemadiff/schema.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/go/vt/schemadiff/schema.go b/go/vt/schemadiff/schema.go index 034b784ffb5..f4cc0b67217 100644 --- a/go/vt/schemadiff/schema.go +++ b/go/vt/schemadiff/schema.go @@ -1079,8 +1079,8 @@ func (s *Schema) getViewColumnNames(v *CreateViewEntity, schemaInformation *decl case *sqlparser.StarExpr: if tableName := node.TableName.Name.String(); tableName != "" { for _, col := range schemaInformation.Tables[tableName].Columns { - name := sqlparser.Clone(&col.Name) - columnNames = append(columnNames, name) + name := sqlparser.Clone(col.Name) + columnNames = append(columnNames, &name) } } else { dependentNames := getViewDependentTableNames(v.CreateView) @@ -1088,8 +1088,8 @@ func (s *Schema) getViewColumnNames(v *CreateViewEntity, schemaInformation *decl for _, entityName := range dependentNames { if schemaInformation.Tables[entityName] != nil { // is nil for dual/DUAL for _, col := range schemaInformation.Tables[entityName].Columns { - name := sqlparser.Clone(&col.Name) - columnNames = append(columnNames, name) + name := sqlparser.Clone(col.Name) + columnNames = append(columnNames, &name) } } }