From 9539bafea7cb1ff33a5161d27a4f29c2880acd5d Mon Sep 17 00:00:00 2001 From: Harshit Gangal Date: Fri, 20 Dec 2024 19:59:43 +0530 Subject: [PATCH] Backport v19: Fixing Column aliases in outer join queries (#15384) (#17418) Signed-off-by: Manan Gupta Signed-off-by: Florent Poinsard <35779988+frouioui@users.noreply.github.com> Signed-off-by: Harshit Gangal Co-authored-by: Manan Gupta <35839558+GuptaManan100@users.noreply.github.com> Co-authored-by: Florent Poinsard <35779988+frouioui@users.noreply.github.com> --- .../endtoend/vtgate/queries/misc/misc_test.go | 10 ++- go/vt/vtgate/engine/cached_size.go | 9 ++- go/vt/vtgate/engine/simple_projection.go | 27 +++++-- go/vt/vtgate/engine/simple_projection_test.go | 15 ++-- .../planbuilder/operator_transformers.go | 9 +-- .../planbuilder/operators/projection.go | 16 +++-- go/vt/vtgate/planbuilder/plan_test.go | 2 +- .../planbuilder/testdata/aggr_cases.json | 3 + .../planbuilder/testdata/cte_cases.json | 7 ++ .../planbuilder/testdata/from_cases.json | 12 ++-- .../testdata/info_schema57_cases.json | 4 +- .../testdata/info_schema80_cases.json | 4 +- .../testdata/memory_sort_cases.json | 10 +++ .../testdata/postprocess_cases.json | 4 ++ .../planbuilder/testdata/rails_cases.json | 4 +- .../planbuilder/testdata/select_cases.json | 72 +++++++++++++++++-- .../planbuilder/testdata/union_cases.json | 6 ++ .../testdata/vindex_func_cases.json | 10 +++ go/vt/vtgate/semantics/early_rewriter.go | 6 +- go/vt/vtgate/semantics/early_rewriter_test.go | 32 ++++----- 20 files changed, 206 insertions(+), 56 deletions(-) diff --git a/go/test/endtoend/vtgate/queries/misc/misc_test.go b/go/test/endtoend/vtgate/queries/misc/misc_test.go index b8509769690..d57345ed0e6 100644 --- a/go/test/endtoend/vtgate/queries/misc/misc_test.go +++ b/go/test/endtoend/vtgate/queries/misc/misc_test.go @@ -319,13 +319,21 @@ func TestTransactionModeVar(t *testing.T) { // TestAliasesInOuterJoinQueries tests that aliases work in queries that have outer join clauses. func TestAliasesInOuterJoinQueries(t *testing.T) { + utils.SkipIfBinaryIsBelowVersion(t, 19, "vtgate") + mcmp, closer := start(t) defer closer() // Insert data into the 2 tables mcmp.Exec("insert into t1(id1, id2) values (1,2), (42,5), (5, 42)") mcmp.Exec("insert into tbl(id, unq_col, nonunq_col) values (1,2,3), (2,5,3), (3, 42, 2)") - mcmp.ExecWithColumnCompare("select * from t1 t left join tbl on t.id1 = 666 and t.id2 = tbl.id") + + // Check that the select query works as intended and then run it again verifying the column names as well. + mcmp.AssertMatches("select t1.id1 as t0, t1.id1 as t1, tbl.unq_col as col from t1 left outer join tbl on t1.id2 = tbl.nonunq_col", `[[INT64(1) INT64(1) INT64(42)] [INT64(5) INT64(5) NULL] [INT64(42) INT64(42) NULL]]`) + mcmp.ExecWithColumnCompare("select t1.id1 as t0, t1.id1 as t1, tbl.unq_col as col from t1 left outer join tbl on t1.id2 = tbl.nonunq_col") + + mcmp.AssertMatches("select t1.id1 as t0, t1.id1 as t1, tbl.unq_col as col from t1 left outer join tbl on t1.id2 = tbl.nonunq_col order by t1.id2 limit 2", `[[INT64(1) INT64(1) INT64(42)] [INT64(42) INT64(42) NULL]]`) + mcmp.ExecWithColumnCompare("select t1.id1 as t0, t1.id1 as t1, tbl.unq_col as col from t1 left outer join tbl on t1.id2 = tbl.nonunq_col order by t1.id2 limit 2") } func TestAlterTableWithView(t *testing.T) { diff --git a/go/vt/vtgate/engine/cached_size.go b/go/vt/vtgate/engine/cached_size.go index 2221830c8b4..12d15b6e5da 100644 --- a/go/vt/vtgate/engine/cached_size.go +++ b/go/vt/vtgate/engine/cached_size.go @@ -1149,12 +1149,19 @@ func (cached *SimpleProjection) CachedSize(alloc bool) int64 { } size := int64(0) if alloc { - size += int64(48) + size += int64(64) } // field Cols []int { size += hack.RuntimeAllocSize(int64(cap(cached.Cols)) * int64(8)) } + // field ColNames []string + { + size += hack.RuntimeAllocSize(int64(cap(cached.ColNames)) * int64(16)) + for _, elem := range cached.ColNames { + size += hack.RuntimeAllocSize(int64(len(elem))) + } + } // field Input vitess.io/vitess/go/vt/vtgate/engine.Primitive if cc, ok := cached.Input.(cachedObject); ok { size += cc.CachedSize(true) diff --git a/go/vt/vtgate/engine/simple_projection.go b/go/vt/vtgate/engine/simple_projection.go index 1a4f4ce92c4..dc80e14b1e1 100644 --- a/go/vt/vtgate/engine/simple_projection.go +++ b/go/vt/vtgate/engine/simple_projection.go @@ -19,6 +19,8 @@ package engine import ( "context" + "google.golang.org/protobuf/proto" + "vitess.io/vitess/go/sqltypes" querypb "vitess.io/vitess/go/vt/proto/query" ) @@ -29,8 +31,10 @@ var _ Primitive = (*SimpleProjection)(nil) type SimpleProjection struct { // Cols defines the column numbers from the underlying primitive // to be returned. - Cols []int - Input Primitive + Cols []int + // ColNames are the column names to use for the columns. + ColNames []string + Input Primitive } // NeedsTransaction implements the Primitive interface @@ -104,8 +108,13 @@ func (sc *SimpleProjection) buildFields(inner *sqltypes.Result) []*querypb.Field return nil } fields := make([]*querypb.Field, 0, len(sc.Cols)) - for _, col := range sc.Cols { - fields = append(fields, inner.Fields[col]) + for idx, col := range sc.Cols { + field := inner.Fields[col] + if sc.ColNames[idx] != "" { + field = proto.Clone(field).(*querypb.Field) + field.Name = sc.ColNames[idx] + } + fields = append(fields, field) } return fields } @@ -114,6 +123,16 @@ func (sc *SimpleProjection) description() PrimitiveDescription { other := map[string]any{ "Columns": sc.Cols, } + emptyColNames := true + for _, cName := range sc.ColNames { + if cName != "" { + emptyColNames = false + break + } + } + if !emptyColNames { + other["ColumnNames"] = sc.ColNames + } return PrimitiveDescription{ OperatorType: "SimpleProjection", Other: other, diff --git a/go/vt/vtgate/engine/simple_projection_test.go b/go/vt/vtgate/engine/simple_projection_test.go index 6fdc288095c..37c5a4d1dc0 100644 --- a/go/vt/vtgate/engine/simple_projection_test.go +++ b/go/vt/vtgate/engine/simple_projection_test.go @@ -44,8 +44,9 @@ func TestSubqueryExecute(t *testing.T) { } sq := &SimpleProjection{ - Cols: []int{0, 2}, - Input: prim, + Cols: []int{0, 2}, + ColNames: []string{"", ""}, + Input: prim, } bv := map[string]*querypb.BindVariable{ @@ -93,8 +94,9 @@ func TestSubqueryStreamExecute(t *testing.T) { } sq := &SimpleProjection{ - Cols: []int{0, 2}, - Input: prim, + Cols: []int{0, 2}, + ColNames: []string{"", ""}, + Input: prim, } bv := map[string]*querypb.BindVariable{ @@ -142,8 +144,9 @@ func TestSubqueryGetFields(t *testing.T) { } sq := &SimpleProjection{ - Cols: []int{0, 2}, - Input: prim, + Cols: []int{0, 2}, + ColNames: []string{"", ""}, + Input: prim, } bv := map[string]*querypb.BindVariable{ diff --git a/go/vt/vtgate/planbuilder/operator_transformers.go b/go/vt/vtgate/planbuilder/operator_transformers.go index 486cadf2fe8..67fd8f9f33e 100644 --- a/go/vt/vtgate/planbuilder/operator_transformers.go +++ b/go/vt/vtgate/planbuilder/operator_transformers.go @@ -351,10 +351,10 @@ func transformProjection(ctx *plancontext.PlanningContext, op *operators.Project return nil, err } - if cols := op.AllOffsets(); cols != nil { + if cols, colNames := op.AllOffsets(); cols != nil { // if all this op is doing is passing through columns from the input, we // can use the faster SimpleProjection - return useSimpleProjection(ctx, op, cols, src) + return useSimpleProjection(ctx, op, cols, colNames, src) } ap, err := op.GetAliasedProjections() @@ -399,7 +399,7 @@ func getEvalEngingeExpr(ctx *plancontext.PlanningContext, pe *operators.ProjExpr // useSimpleProjection uses nothing at all if the output is already correct, // or SimpleProjection when we have to reorder or truncate the columns -func useSimpleProjection(ctx *plancontext.PlanningContext, op *operators.Projection, cols []int, src logicalPlan) (logicalPlan, error) { +func useSimpleProjection(ctx *plancontext.PlanningContext, op *operators.Projection, cols []int, colNames []string, src logicalPlan) (logicalPlan, error) { columns := op.Source.GetColumns(ctx) if len(columns) == len(cols) && elementsMatchIndices(cols) { // the columns are already in the right order. we don't need anything at all here @@ -408,7 +408,8 @@ func useSimpleProjection(ctx *plancontext.PlanningContext, op *operators.Project return &simpleProjection{ logicalPlanCommon: newBuilderCommon(src), eSimpleProj: &engine.SimpleProjection{ - Cols: cols, + Cols: cols, + ColNames: colNames, }, }, nil } diff --git a/go/vt/vtgate/planbuilder/operators/projection.go b/go/vt/vtgate/planbuilder/operators/projection.go index 38164b71a94..89c2f042089 100644 --- a/go/vt/vtgate/planbuilder/operators/projection.go +++ b/go/vt/vtgate/planbuilder/operators/projection.go @@ -423,18 +423,23 @@ func (p *Projection) GetOrdering(ctx *plancontext.PlanningContext) []OrderBy { // AllOffsets returns a slice of integer offsets for all columns in the Projection // if all columns are of type Offset. If any column is not of type Offset, it returns nil. -func (p *Projection) AllOffsets() (cols []int) { +func (p *Projection) AllOffsets() (cols []int, colNames []string) { ap, err := p.GetAliasedProjections() if err != nil { - return nil + return nil, nil } for _, c := range ap { offset, ok := c.Info.(Offset) if !ok { - return nil + return nil, nil + } + colName := "" + if c.Original.As.NotEmpty() { + colName = c.Original.As.String() } cols = append(cols, int(offset)) + colNames = append(colNames, colName) } return } @@ -469,7 +474,7 @@ func (p *Projection) Compact(ctx *plancontext.PlanningContext) (Operator, *Apply needed := false for i, projection := range ap { e, ok := projection.Info.(Offset) - if !ok || int(e) != i { + if !ok || int(e) != i || projection.Original.As.NotEmpty() { needed = true break } @@ -499,6 +504,9 @@ func (p *Projection) compactWithJoin(ctx *plancontext.PlanningContext, join *App for _, col := range ap { switch colInfo := col.Info.(type) { case Offset: + if col.Original.As.NotEmpty() { + return p, NoRewrite + } newColumns = append(newColumns, join.Columns[colInfo]) newColumnsAST.add(join.JoinColumns.columns[colInfo]) case nil: diff --git a/go/vt/vtgate/planbuilder/plan_test.go b/go/vt/vtgate/planbuilder/plan_test.go index 62dd567b0d8..a69eff60acf 100644 --- a/go/vt/vtgate/planbuilder/plan_test.go +++ b/go/vt/vtgate/planbuilder/plan_test.go @@ -674,7 +674,7 @@ func (s *planTestSuite) testFile(filename string, vschema *vschemawrapper.VSchem if tcase.Skip { t.Skip(message) } else { - t.Errorf(message) + t.Error(message) } } else if tcase.Skip { t.Errorf("query is correct even though it is skipped:\n %s", tcase.Query) diff --git a/go/vt/vtgate/planbuilder/testdata/aggr_cases.json b/go/vt/vtgate/planbuilder/testdata/aggr_cases.json index 8a703d8a620..6ba88fc9efa 100644 --- a/go/vt/vtgate/planbuilder/testdata/aggr_cases.json +++ b/go/vt/vtgate/planbuilder/testdata/aggr_cases.json @@ -3644,6 +3644,9 @@ "Original": "select * from (select id from user having count(*) = 1) s", "Instructions": { "OperatorType": "SimpleProjection", + "ColumnNames": [ + "id" + ], "Columns": [ 0 ], diff --git a/go/vt/vtgate/planbuilder/testdata/cte_cases.json b/go/vt/vtgate/planbuilder/testdata/cte_cases.json index 2fd8d573fe0..0ce7fa280a0 100644 --- a/go/vt/vtgate/planbuilder/testdata/cte_cases.json +++ b/go/vt/vtgate/planbuilder/testdata/cte_cases.json @@ -370,6 +370,9 @@ "Original": "with s as (select id from user having count(*) = 1) select * from s", "Instructions": { "OperatorType": "SimpleProjection", + "ColumnNames": [ + "id" + ], "Columns": [ 0 ], @@ -1300,6 +1303,10 @@ "Original": "with t as (select user.id from user join user_extra) select id, t.id from t", "Instructions": { "OperatorType": "SimpleProjection", + "ColumnNames": [ + "id", + "id" + ], "Columns": [ 0, 0 diff --git a/go/vt/vtgate/planbuilder/testdata/from_cases.json b/go/vt/vtgate/planbuilder/testdata/from_cases.json index 7ed42771579..25c143ac52c 100644 --- a/go/vt/vtgate/planbuilder/testdata/from_cases.json +++ b/go/vt/vtgate/planbuilder/testdata/from_cases.json @@ -2673,6 +2673,10 @@ "Original": "select id, t.id from (select user.id from user join user_extra) as t", "Instructions": { "OperatorType": "SimpleProjection", + "ColumnNames": [ + "id", + "id" + ], "Columns": [ 0, 0 @@ -3989,8 +3993,8 @@ "Name": "user", "Sharded": true }, - "FieldQuery": "select authoritative.col1 as col1, authoritative.user_id as user_id, authoritative.col2 as col2 from authoritative where 1 != 1", - "Query": "select authoritative.col1 as col1, authoritative.user_id as user_id, authoritative.col2 as col2 from authoritative", + "FieldQuery": "select authoritative.col1, authoritative.user_id, authoritative.col2 from authoritative where 1 != 1", + "Query": "select authoritative.col1, authoritative.user_id, authoritative.col2 from authoritative", "Table": "authoritative" }, { @@ -4000,8 +4004,8 @@ "Name": "main", "Sharded": false }, - "FieldQuery": "select unsharded_authoritative.col2 as col2 from unsharded_authoritative where 1 != 1", - "Query": "select unsharded_authoritative.col2 as col2 from unsharded_authoritative where unsharded_authoritative.col1 = :authoritative_col1", + "FieldQuery": "select unsharded_authoritative.col2 from unsharded_authoritative where 1 != 1", + "Query": "select unsharded_authoritative.col2 from unsharded_authoritative where unsharded_authoritative.col1 = :authoritative_col1", "Table": "unsharded_authoritative" } ] diff --git a/go/vt/vtgate/planbuilder/testdata/info_schema57_cases.json b/go/vt/vtgate/planbuilder/testdata/info_schema57_cases.json index 397d9ce6046..12ddfa6e049 100644 --- a/go/vt/vtgate/planbuilder/testdata/info_schema57_cases.json +++ b/go/vt/vtgate/planbuilder/testdata/info_schema57_cases.json @@ -750,8 +750,8 @@ "Name": "main", "Sharded": false }, - "FieldQuery": "select a.VARIABLE_NAME as VARIABLE_NAME, a.VARIABLE_VALUE as VARIABLE_VALUE, b.CHARACTER_SET_NAME as CHARACTER_SET_NAME, b.DEFAULT_COLLATE_NAME as DEFAULT_COLLATE_NAME, b.DESCRIPTION as DESCRIPTION, b.MAXLEN as MAXLEN from information_schema.GLOBAL_STATUS as a, information_schema.CHARACTER_SETS as b where 1 != 1", - "Query": "select a.VARIABLE_NAME as VARIABLE_NAME, a.VARIABLE_VALUE as VARIABLE_VALUE, b.CHARACTER_SET_NAME as CHARACTER_SET_NAME, b.DEFAULT_COLLATE_NAME as DEFAULT_COLLATE_NAME, b.DESCRIPTION as DESCRIPTION, b.MAXLEN as MAXLEN from information_schema.GLOBAL_STATUS as a, information_schema.CHARACTER_SETS as b", + "FieldQuery": "select a.VARIABLE_NAME, a.VARIABLE_VALUE, b.CHARACTER_SET_NAME, b.DEFAULT_COLLATE_NAME, b.DESCRIPTION, b.MAXLEN from information_schema.GLOBAL_STATUS as a, information_schema.CHARACTER_SETS as b where 1 != 1", + "Query": "select a.VARIABLE_NAME, a.VARIABLE_VALUE, b.CHARACTER_SET_NAME, b.DEFAULT_COLLATE_NAME, b.DESCRIPTION, b.MAXLEN from information_schema.GLOBAL_STATUS as a, information_schema.CHARACTER_SETS as b", "Table": "information_schema.CHARACTER_SETS, information_schema.GLOBAL_STATUS" } } diff --git a/go/vt/vtgate/planbuilder/testdata/info_schema80_cases.json b/go/vt/vtgate/planbuilder/testdata/info_schema80_cases.json index 626bf0d505a..18721a44fd1 100644 --- a/go/vt/vtgate/planbuilder/testdata/info_schema80_cases.json +++ b/go/vt/vtgate/planbuilder/testdata/info_schema80_cases.json @@ -815,8 +815,8 @@ "Name": "main", "Sharded": false }, - "FieldQuery": "select a.CONSTRAINT_CATALOG as CONSTRAINT_CATALOG, a.CONSTRAINT_SCHEMA as CONSTRAINT_SCHEMA, a.CONSTRAINT_NAME as CONSTRAINT_NAME, a.CHECK_CLAUSE as CHECK_CLAUSE, b.CHARACTER_SET_NAME as CHARACTER_SET_NAME, b.DEFAULT_COLLATE_NAME as DEFAULT_COLLATE_NAME, b.DESCRIPTION as DESCRIPTION, b.MAXLEN as MAXLEN from information_schema.CHECK_CONSTRAINTS as a, information_schema.CHARACTER_SETS as b where 1 != 1", - "Query": "select a.CONSTRAINT_CATALOG as CONSTRAINT_CATALOG, a.CONSTRAINT_SCHEMA as CONSTRAINT_SCHEMA, a.CONSTRAINT_NAME as CONSTRAINT_NAME, a.CHECK_CLAUSE as CHECK_CLAUSE, b.CHARACTER_SET_NAME as CHARACTER_SET_NAME, b.DEFAULT_COLLATE_NAME as DEFAULT_COLLATE_NAME, b.DESCRIPTION as DESCRIPTION, b.MAXLEN as MAXLEN from information_schema.CHECK_CONSTRAINTS as a, information_schema.CHARACTER_SETS as b", + "FieldQuery": "select a.CONSTRAINT_CATALOG, a.CONSTRAINT_SCHEMA, a.CONSTRAINT_NAME, a.CHECK_CLAUSE, b.CHARACTER_SET_NAME, b.DEFAULT_COLLATE_NAME, b.DESCRIPTION, b.MAXLEN from information_schema.CHECK_CONSTRAINTS as a, information_schema.CHARACTER_SETS as b where 1 != 1", + "Query": "select a.CONSTRAINT_CATALOG, a.CONSTRAINT_SCHEMA, a.CONSTRAINT_NAME, a.CHECK_CLAUSE, b.CHARACTER_SET_NAME, b.DEFAULT_COLLATE_NAME, b.DESCRIPTION, b.MAXLEN from information_schema.CHECK_CONSTRAINTS as a, information_schema.CHARACTER_SETS as b", "Table": "information_schema.CHARACTER_SETS, information_schema.CHECK_CONSTRAINTS" } } diff --git a/go/vt/vtgate/planbuilder/testdata/memory_sort_cases.json b/go/vt/vtgate/planbuilder/testdata/memory_sort_cases.json index 8390dde80bc..ea0cca5f4da 100644 --- a/go/vt/vtgate/planbuilder/testdata/memory_sort_cases.json +++ b/go/vt/vtgate/planbuilder/testdata/memory_sort_cases.json @@ -289,6 +289,11 @@ "Original": "select user.col1 as a, user.col2 b, music.col3 c from user, music where user.id = music.id and user.id = 1 order by c", "Instructions": { "OperatorType": "SimpleProjection", + "ColumnNames": [ + "a", + "b", + "c" + ], "Columns": [ 0, 1, @@ -360,6 +365,11 @@ "Original": "select user.col1 as a, user.col2, music.col3 from user join music on user.id = music.id where user.id = 1 order by 1 asc, 3 desc, 2 asc", "Instructions": { "OperatorType": "SimpleProjection", + "ColumnNames": [ + "a", + "col2", + "col3" + ], "Columns": [ 0, 1, diff --git a/go/vt/vtgate/planbuilder/testdata/postprocess_cases.json b/go/vt/vtgate/planbuilder/testdata/postprocess_cases.json index 78144f5c33c..1363d3780db 100644 --- a/go/vt/vtgate/planbuilder/testdata/postprocess_cases.json +++ b/go/vt/vtgate/planbuilder/testdata/postprocess_cases.json @@ -1586,6 +1586,10 @@ "Original": "select name, name from user, music order by name", "Instructions": { "OperatorType": "SimpleProjection", + "ColumnNames": [ + "name", + "name" + ], "Columns": [ 0, 0 diff --git a/go/vt/vtgate/planbuilder/testdata/rails_cases.json b/go/vt/vtgate/planbuilder/testdata/rails_cases.json index ef36b79c855..c8ab8b7b9d8 100644 --- a/go/vt/vtgate/planbuilder/testdata/rails_cases.json +++ b/go/vt/vtgate/planbuilder/testdata/rails_cases.json @@ -50,8 +50,8 @@ "Name": "user", "Sharded": true }, - "FieldQuery": "select author5s.id as id, author5s.`name` as `name`, author5s.created_at as created_at, author5s.updated_at as updated_at, book6s.supplier5_id, book6s.id from author5s, book6s where 1 != 1", - "Query": "select author5s.id as id, author5s.`name` as `name`, author5s.created_at as created_at, author5s.updated_at as updated_at, book6s.supplier5_id, book6s.id from author5s, book6s where book6s.author5_id = author5s.id", + "FieldQuery": "select author5s.id, author5s.`name`, author5s.created_at, author5s.updated_at, book6s.supplier5_id, book6s.id from author5s, book6s where 1 != 1", + "Query": "select author5s.id, author5s.`name`, author5s.created_at, author5s.updated_at, book6s.supplier5_id, book6s.id from author5s, book6s where book6s.author5_id = author5s.id", "Table": "author5s, book6s" }, { diff --git a/go/vt/vtgate/planbuilder/testdata/select_cases.json b/go/vt/vtgate/planbuilder/testdata/select_cases.json index 698a9aceaa5..a5ccfb4596d 100644 --- a/go/vt/vtgate/planbuilder/testdata/select_cases.json +++ b/go/vt/vtgate/planbuilder/testdata/select_cases.json @@ -390,8 +390,8 @@ "Name": "user", "Sharded": true }, - "FieldQuery": "select a.user_id as user_id, a.col1 as col1, a.col2 as col2, b.user_id as user_id, b.col1 as col1, b.col2 as col2 from authoritative as a, authoritative as b where 1 != 1", - "Query": "select a.user_id as user_id, a.col1 as col1, a.col2 as col2, b.user_id as user_id, b.col1 as col1, b.col2 as col2 from authoritative as a, authoritative as b where a.user_id = b.user_id", + "FieldQuery": "select a.user_id, a.col1, a.col2, b.user_id, b.col1, b.col2 from authoritative as a, authoritative as b where 1 != 1", + "Query": "select a.user_id, a.col1, a.col2, b.user_id, b.col1, b.col2 from authoritative as a, authoritative as b where a.user_id = b.user_id", "Table": "authoritative" }, "TablesUsed": [ @@ -462,8 +462,8 @@ "Name": "user", "Sharded": true }, - "FieldQuery": "select `user`.id, a.user_id as user_id, a.col1 as col1, a.col2 as col2, `user`.col1 from authoritative as a, `user` where 1 != 1", - "Query": "select `user`.id, a.user_id as user_id, a.col1 as col1, a.col2 as col2, `user`.col1 from authoritative as a, `user` where a.user_id = `user`.id", + "FieldQuery": "select `user`.id, a.user_id, a.col1, a.col2, `user`.col1 from authoritative as a, `user` where 1 != 1", + "Query": "select `user`.id, a.user_id, a.col1, a.col2, `user`.col1 from authoritative as a, `user` where a.user_id = `user`.id", "Table": "`user`, authoritative" }, "TablesUsed": [ @@ -2313,6 +2313,9 @@ "Original": "select col from user where exists(select user_id from user_extra where user_id = 3 and user_id < user.id)", "Instructions": { "OperatorType": "SimpleProjection", + "ColumnNames": [ + "col" + ], "Columns": [ 0 ], @@ -2370,6 +2373,9 @@ "Original": "select col from user where exists(select user_id from user_extra where user_id = 3 and user_id < user.id) order by col", "Instructions": { "OperatorType": "SimpleProjection", + "ColumnNames": [ + "col" + ], "Columns": [ 0 ], @@ -5032,5 +5038,63 @@ "user.user" ] } + }, + { + "comment": "column name aliases in outer join queries", + "query": "select name as t0, name as t1 from user left outer join user_extra on user.cola = user_extra.cola", + "plan": { + "QueryType": "SELECT", + "Original": "select name as t0, name as t1 from user left outer join user_extra on user.cola = user_extra.cola", + "Instructions": { + "OperatorType": "SimpleProjection", + "ColumnNames": [ + "t0", + "t1" + ], + "Columns": [ + 0, + 0 + ], + "Inputs": [ + { + "OperatorType": "Join", + "Variant": "LeftJoin", + "JoinColumnIndexes": "L:0", + "JoinVars": { + "user_cola": 1 + }, + "TableName": "`user`_user_extra", + "Inputs": [ + { + "OperatorType": "Route", + "Variant": "Scatter", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select `name`, `user`.cola from `user` where 1 != 1", + "Query": "select `name`, `user`.cola from `user`", + "Table": "`user`" + }, + { + "OperatorType": "Route", + "Variant": "Scatter", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select 1 from user_extra where 1 != 1", + "Query": "select 1 from user_extra where user_extra.cola = :user_cola", + "Table": "user_extra" + } + ] + } + ] + }, + "TablesUsed": [ + "user.user", + "user.user_extra" + ] + } } ] diff --git a/go/vt/vtgate/planbuilder/testdata/union_cases.json b/go/vt/vtgate/planbuilder/testdata/union_cases.json index 7c225862235..2808bb43e84 100644 --- a/go/vt/vtgate/planbuilder/testdata/union_cases.json +++ b/go/vt/vtgate/planbuilder/testdata/union_cases.json @@ -108,6 +108,9 @@ "Original": "(SELECT id FROM user ORDER BY id DESC LIMIT 1) UNION ALL (SELECT id FROM music ORDER BY id DESC LIMIT 1)", "Instructions": { "OperatorType": "SimpleProjection", + "ColumnNames": [ + "id" + ], "Columns": [ 0 ], @@ -238,6 +241,9 @@ "Original": "(select id from user order by id limit 5) union all (select id from music order by id desc limit 5)", "Instructions": { "OperatorType": "SimpleProjection", + "ColumnNames": [ + "id" + ], "Columns": [ 0 ], diff --git a/go/vt/vtgate/planbuilder/testdata/vindex_func_cases.json b/go/vt/vtgate/planbuilder/testdata/vindex_func_cases.json index 4c6256d93cc..1c030b71f09 100644 --- a/go/vt/vtgate/planbuilder/testdata/vindex_func_cases.json +++ b/go/vt/vtgate/planbuilder/testdata/vindex_func_cases.json @@ -73,6 +73,11 @@ "Original": "select id, keyspace_id, id from user_index where id = :id", "Instructions": { "OperatorType": "SimpleProjection", + "ColumnNames": [ + "id", + "keyspace_id", + "id" + ], "Columns": [ 0, 1, @@ -113,6 +118,11 @@ "Original": "select id, keyspace_id, id from second_user.hash_dup where id = :id", "Instructions": { "OperatorType": "SimpleProjection", + "ColumnNames": [ + "id", + "keyspace_id", + "id" + ], "Columns": [ 0, 1, diff --git a/go/vt/vtgate/semantics/early_rewriter.go b/go/vt/vtgate/semantics/early_rewriter.go index 2c731c4bfc8..d54e1dbcd30 100644 --- a/go/vt/vtgate/semantics/early_rewriter.go +++ b/go/vt/vtgate/semantics/early_rewriter.go @@ -1224,17 +1224,13 @@ type expanderState struct { // addColumn adds columns to the expander state. If we have vschema info about the query, // we also store which columns were expanded func (e *expanderState) addColumn(col ColumnInfo, tbl TableInfo, tblName sqlparser.TableName) { - withQualifier := e.needsQualifier var colName *sqlparser.ColName var alias sqlparser.IdentifierCI - if withQualifier { + if e.needsQualifier { colName = sqlparser.NewColNameWithQualifier(col.Name, tblName) } else { colName = sqlparser.NewColName(col.Name) } - if e.needsQualifier { - alias = sqlparser.NewIdentifierCI(col.Name) - } e.colNames = append(e.colNames, &sqlparser.AliasedExpr{Expr: colName, As: alias}) e.storeExpandInfo(tbl, tblName, colName) } diff --git a/go/vt/vtgate/semantics/early_rewriter_test.go b/go/vt/vtgate/semantics/early_rewriter_test.go index adf26017d87..c9d565c1afb 100644 --- a/go/vt/vtgate/semantics/early_rewriter_test.go +++ b/go/vt/vtgate/semantics/early_rewriter_test.go @@ -122,17 +122,17 @@ func TestExpandStar(t *testing.T) { expSQL: "select 42, a, b, c from t1", }, { sql: "select * from t1, t2", - expSQL: "select t1.a as a, t1.b as b, t1.c as c, t2.c1 as c1, t2.c2 as c2 from t1, t2", + expSQL: "select t1.a, t1.b, t1.c, t2.c1, t2.c2 from t1, t2", expanded: "main.t1.a, main.t1.b, main.t1.c, main.t2.c1, main.t2.c2", }, { sql: "select t1.* from t1, t2", - expSQL: "select t1.a as a, t1.b as b, t1.c as c from t1, t2", + expSQL: "select t1.a, t1.b, t1.c from t1, t2", }, { sql: "select *, t1.* from t1, t2", - expSQL: "select t1.a as a, t1.b as b, t1.c as c, t2.c1 as c1, t2.c2 as c2, t1.a as a, t1.b as b, t1.c as c from t1, t2", + expSQL: "select t1.a, t1.b, t1.c, t2.c1, t2.c2, t1.a, t1.b, t1.c from t1, t2", }, { // aliased table sql: "select * from t1 a, t2 b", - expSQL: "select a.a as a, a.b as b, a.c as c, b.c1 as c1, b.c2 as c2 from t1 as a, t2 as b", + expSQL: "select a.a, a.b, a.c, b.c1, b.c2 from t1 as a, t2 as b", }, { // t3 is non-authoritative table sql: "select * from t3", expSQL: "select * from t3", @@ -141,36 +141,36 @@ func TestExpandStar(t *testing.T) { expSQL: "select * from t1, t2, t3", }, { // t3 is non-authoritative table sql: "select t1.*, t2.*, t3.* from t1, t2, t3", - expSQL: "select t1.a as a, t1.b as b, t1.c as c, t2.c1 as c1, t2.c2 as c2, t3.* from t1, t2, t3", + expSQL: "select t1.a, t1.b, t1.c, t2.c1, t2.c2, t3.* from t1, t2, t3", }, { sql: "select foo.* from t1, t2", expErr: "Unknown table 'foo'", }, { sql: "select * from t1 join t2 on t1.a = t2.c1", - expSQL: "select t1.a as a, t1.b as b, t1.c as c, t2.c1 as c1, t2.c2 as c2 from t1 join t2 on t1.a = t2.c1", + expSQL: "select t1.a, t1.b, t1.c, t2.c1, t2.c2 from t1 join t2 on t1.a = t2.c1", }, { sql: "select * from t1 left join t2 on t1.a = t2.c1", - expSQL: "select t1.a as a, t1.b as b, t1.c as c, t2.c1 as c1, t2.c2 as c2 from t1 left join t2 on t1.a = t2.c1", + expSQL: "select t1.a, t1.b, t1.c, t2.c1, t2.c2 from t1 left join t2 on t1.a = t2.c1", }, { sql: "select * from t1 right join t2 on t1.a = t2.c1", - expSQL: "select t1.a as a, t1.b as b, t1.c as c, t2.c1 as c1, t2.c2 as c2 from t1 right join t2 on t1.a = t2.c1", + expSQL: "select t1.a, t1.b, t1.c, t2.c1, t2.c2 from t1 right join t2 on t1.a = t2.c1", }, { sql: "select * from t2 join t4 using (c1)", - expSQL: "select t2.c1 as c1, t2.c2 as c2, t4.c4 as c4 from t2 join t4 on t2.c1 = t4.c1", + expSQL: "select t2.c1, t2.c2, t4.c4 from t2 join t4 on t2.c1 = t4.c1", expanded: "main.t2.c1, main.t2.c2, main.t4.c4", }, { sql: "select * from t2 join t4 using (c1) join t2 as X using (c1)", - expSQL: "select t2.c1 as c1, t2.c2 as c2, t4.c4 as c4, X.c2 as c2 from t2 join t4 on t2.c1 = t4.c1 join t2 as X on t2.c1 = t4.c1 and t2.c1 = X.c1 and t4.c1 = X.c1", + expSQL: "select t2.c1, t2.c2, t4.c4, X.c2 from t2 join t4 on t2.c1 = t4.c1 join t2 as X on t2.c1 = t4.c1 and t2.c1 = X.c1 and t4.c1 = X.c1", }, { sql: "select * from t2 join t4 using (c1), t2 as t2b join t4 as t4b using (c1)", - expSQL: "select t2.c1 as c1, t2.c2 as c2, t4.c4 as c4, t2b.c1 as c1, t2b.c2 as c2, t4b.c4 as c4 from t2 join t4 on t2.c1 = t4.c1, t2 as t2b join t4 as t4b on t2b.c1 = t4b.c1", + expSQL: "select t2.c1, t2.c2, t4.c4, t2b.c1, t2b.c2, t4b.c4 from t2 join t4 on t2.c1 = t4.c1, t2 as t2b join t4 as t4b on t2b.c1 = t4b.c1", }, { sql: "select * from t1 join t5 using (b)", - expSQL: "select t1.b as b, t1.a as a, t1.c as c, t5.a as a from t1 join t5 on t1.b = t5.b", + expSQL: "select t1.b, t1.a, t1.c, t5.a from t1 join t5 on t1.b = t5.b", expanded: "main.t1.a, main.t1.b, main.t1.c, main.t5.a", }, { sql: "select * from t1 join t5 using (b) having b = 12", - expSQL: "select t1.b as b, t1.a as a, t1.c as c, t5.a as a from t1 join t5 on t1.b = t5.b having t1.b = 12", + expSQL: "select t1.b, t1.a, t1.c, t5.a from t1 join t5 on t1.b = t5.b having t1.b = 12", }, { sql: "select 1 from t1 join t5 using (b) where b = 12", expSQL: "select 1 from t1 join t5 on t1.b = t5.b where t1.b = 12", @@ -183,7 +183,7 @@ func TestExpandStar(t *testing.T) { }, { // if we are only star-expanding authoritative tables, we don't need to stop the expansion sql: "SELECT * FROM (SELECT t2.*, 12 AS foo FROM t3, t2) as results", - expSQL: "select c1, c2, foo from (select t2.c1 as c1, t2.c2 as c2, 12 as foo from t3, t2) as results", + expSQL: "select c1, c2, foo from (select t2.c1, t2.c2, 12 as foo from t3, t2) as results", }} for _, tcase := range tcases { t.Run(tcase.sql, func(t *testing.T) { @@ -788,11 +788,11 @@ func TestSemTableDependenciesAfterExpandStar(t *testing.T) { otherTbl: -1, sameTbl: 0, expandedCol: 1, }, { sql: "select t2.a, t1.a, t1.* from t1, t2", - expSQL: "select t2.a, t1.a, t1.a as a from t1, t2", + expSQL: "select t2.a, t1.a, t1.a from t1, t2", otherTbl: 0, sameTbl: 1, expandedCol: 2, }, { sql: "select t2.a, t.a, t.* from t1 t, t2", - expSQL: "select t2.a, t.a, t.a as a from t1 as t, t2", + expSQL: "select t2.a, t.a, t.a from t1 as t, t2", otherTbl: 0, sameTbl: 1, expandedCol: 2, }} for _, tcase := range tcases {