From 31fea9ec7c55f260f8a18dab04ea22d18512e4bc Mon Sep 17 00:00:00 2001 From: Florent Poinsard Date: Thu, 20 Feb 2025 08:02:58 -0600 Subject: [PATCH] Fix queryBuilder for Values Signed-off-by: Florent Poinsard --- .../vtgate/planbuilder/operators/op_to_ast.go | 9 +++++++-- .../planbuilder/operators/query_builder.go | 18 ++++++++++++++++++ .../planbuilder/operators/query_planning.go | 3 +-- .../planbuilder/operators/values_join.go | 4 ++-- .../plancontext/planning_context.go | 9 +++++++++ go/vt/vtgate/planbuilder/testdata/onecase.json | 2 +- go/vt/vtgate/semantics/table_set.go | 17 +++++++++++++++++ 7 files changed, 55 insertions(+), 7 deletions(-) diff --git a/go/vt/vtgate/planbuilder/operators/op_to_ast.go b/go/vt/vtgate/planbuilder/operators/op_to_ast.go index 7e6f57b5954..68f87d3bc58 100644 --- a/go/vt/vtgate/planbuilder/operators/op_to_ast.go +++ b/go/vt/vtgate/planbuilder/operators/op_to_ast.go @@ -154,29 +154,34 @@ func buildValues(op *Values, qb *queryBuilder) { apa := semantics.EmptyTableSet() for _, ae := range qb.ctx.ValuesJoinColumns[op.Name] { apa = apa.Merge(qb.ctx.SemTable.RecursiveDeps(ae.Expr)) + fmt.Printf("%v\n", apa) } tableName := getTableName(qb.ctx, apa) - qb.addTableExpr(tableName, tableName, TableID(op), expr, nil, op.getColumnNamesFromCtx(qb.ctx)) + qb.addTableExprNoSemTableChange(tableName, tableName, expr, op.getColumnNamesFromCtx(qb.ctx)) } func getTableName(ctx *plancontext.PlanningContext, id semantics.TableSet) string { var names []string for _, ts := range id.Constituents() { ti, err := ctx.SemTable.TableInfoFor(ts) + fmt.Printf("1: %v\n", ti) if err != nil { names = append(names, "X") continue } name, err := ti.Name() + fmt.Printf("2: %v\n", name) if err != nil { names = append(names, "X") continue } names = append(names, name.Name.String()) } - return strings.Join(names, "_") + x := strings.Join(names, "_") + fmt.Println("3: ", x) + return x } func buildDelete(op *Delete, qb *queryBuilder) { diff --git a/go/vt/vtgate/planbuilder/operators/query_builder.go b/go/vt/vtgate/planbuilder/operators/query_builder.go index 891d742fe8f..48ef69cecad 100644 --- a/go/vt/vtgate/planbuilder/operators/query_builder.go +++ b/go/vt/vtgate/planbuilder/operators/query_builder.go @@ -103,6 +103,24 @@ func (qb *queryBuilder) addTableExpr( qb.tableNames = append(qb.tableNames, tableName) } +func (qb *queryBuilder) addTableExprNoSemTableChange( + tableName, alias string, + tblExpr sqlparser.SimpleTableExpr, + columnAliases sqlparser.Columns, +) { + if qb.stmt == nil { + qb.stmt = &sqlparser.Select{} + } + tbl := &sqlparser.AliasedTableExpr{ + Expr: tblExpr, + Partitions: nil, + As: sqlparser.NewIdentifierCS(alias), + Columns: columnAliases, + } + qb.stmt.(FromStatement).SetFrom(append(qb.stmt.(FromStatement).GetFrom(), tbl)) + qb.tableNames = append(qb.tableNames, tableName) +} + func (qb *queryBuilder) addPredicate(expr sqlparser.Expr) { if qb.ctx.ShouldSkip(expr) { // This is a predicate that was added to the RHS of an ApplyJoin. diff --git a/go/vt/vtgate/planbuilder/operators/query_planning.go b/go/vt/vtgate/planbuilder/operators/query_planning.go index 4938f73e09a..4562287600d 100644 --- a/go/vt/vtgate/planbuilder/operators/query_planning.go +++ b/go/vt/vtgate/planbuilder/operators/query_planning.go @@ -130,8 +130,7 @@ func tryPushValues(ctx *plancontext.PlanningContext, in *Values) (Operator, *App switch src := in.Source.(type) { case *ValuesJoin: src.LHS = in.Clone([]Operator{src.LHS}) - src.RHS = in.Clone([]Operator{src.RHS}) - return src, Rewrote("pushed values under value join") + return src, Rewrote("pushed values to the LHS of values join") case *Filter: return Swap(in, src, "pushed values under filter") case *Route: diff --git a/go/vt/vtgate/planbuilder/operators/values_join.go b/go/vt/vtgate/planbuilder/operators/values_join.go index b9164c8d2ee..3fc27fbe3c4 100644 --- a/go/vt/vtgate/planbuilder/operators/values_join.go +++ b/go/vt/vtgate/planbuilder/operators/values_join.go @@ -160,7 +160,7 @@ func (vj *ValuesJoin) GetOrdering(ctx *plancontext.PlanningContext) []OrderBy { } func (vj *ValuesJoin) planOffsets(ctx *plancontext.PlanningContext) Operator { - exprs := ctx.ValuesJoinColumns[vj.ValuesDestination] + exprs := ctx.GetColumns(vj.ValuesDestination) for _, jc := range vj.JoinColumns { newExprs := vj.planOffsetsForLHSExprs(ctx, jc.LHS) exprs = append(exprs, newExprs...) @@ -172,7 +172,7 @@ func (vj *ValuesJoin) planOffsets(ctx *plancontext.PlanningContext) Operator { newExprs := vj.planOffsetsForLHSExprs(ctx, jc.LHS) exprs = append(exprs, newExprs...) } - ctx.ValuesJoinColumns[vj.ValuesDestination] = exprs + ctx.SetColumns(vj.ValuesDestination, exprs) return vj } diff --git a/go/vt/vtgate/planbuilder/plancontext/planning_context.go b/go/vt/vtgate/planbuilder/plancontext/planning_context.go index 1963bc9a4c0..c48fb9fa7b3 100644 --- a/go/vt/vtgate/planbuilder/plancontext/planning_context.go +++ b/go/vt/vtgate/planbuilder/plancontext/planning_context.go @@ -474,6 +474,15 @@ func (ctx *PlanningContext) ActiveCTE() *ContextCTE { return ctx.CurrentCTE[len(ctx.CurrentCTE)-1] } +func (ctx *PlanningContext) GetColumns(joinName string) []*sqlparser.AliasedExpr { + valuesName := ctx.ValueJoins[joinName] + return ctx.ValuesJoinColumns[valuesName] +} +func (ctx *PlanningContext) SetColumns(joinName string, cols []*sqlparser.AliasedExpr) { + valuesName := ctx.ValueJoins[joinName] + ctx.ValuesJoinColumns[valuesName] = cols +} + func (ctx *PlanningContext) UseMirror() *PlanningContext { if ctx.isMirrored { panic(vterrors.VT13001("cannot mirror already mirrored planning context")) diff --git a/go/vt/vtgate/planbuilder/testdata/onecase.json b/go/vt/vtgate/planbuilder/testdata/onecase.json index 9d653b2f6e9..279fd68988a 100644 --- a/go/vt/vtgate/planbuilder/testdata/onecase.json +++ b/go/vt/vtgate/planbuilder/testdata/onecase.json @@ -1,7 +1,7 @@ [ { "comment": "Add your test case here for debugging and run go test -run=One.", - "query": "", + "query": "select /*vt+ ALLOW_VALUES_JOIN */ user.foo, user_extra.user_id, user_metadata.name from user, user_extra, user_metadata where user.id = user_extra.toto and user_extra.age = user_metadata.age", "plan": { } } diff --git a/go/vt/vtgate/semantics/table_set.go b/go/vt/vtgate/semantics/table_set.go index acc83306869..65d714fc340 100644 --- a/go/vt/vtgate/semantics/table_set.go +++ b/go/vt/vtgate/semantics/table_set.go @@ -18,6 +18,7 @@ package semantics import ( "fmt" + "strings" "vitess.io/vitess/go/vt/vtgate/semantics/bitset" ) @@ -41,6 +42,22 @@ func (ts TableSet) Format(f fmt.State, verb rune) { fmt.Fprintf(f, "}") } +func (ts TableSet) DebugString() string { + var f strings.Builder + first := true + f.WriteString("TableSet{") + bitset.Bitset(ts).ForEach(func(tid int) { + if first { + f.WriteString(fmt.Sprintf("%d", tid)) + first = false + } else { + f.WriteString(fmt.Sprintf(",%d", tid)) + } + }) + f.WriteString("}") + return f.String() +} + // IsOverlapping returns true if at least one table exists in both sets func (ts TableSet) IsOverlapping(other TableSet) bool { return bitset.Bitset(ts).Overlaps(bitset.Bitset(other))