Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixing Column aliases in outer join queries #15384

Merged
merged 8 commits into from
Mar 5, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions go/test/endtoend/vtgate/queries/misc/misc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -352,3 +352,22 @@ 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, 20, "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)")

// Check that the select query works as intended and then run it agin verifying the column names as well.
frouioui marked this conversation as resolved.
Show resolved Hide resolved
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")
}
28 changes: 24 additions & 4 deletions go/vt/vtgate/engine/simple_projection.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand All @@ -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
Expand Down Expand Up @@ -104,8 +108,14 @@ 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 {
if sc.ColNames[idx] == "" {
fields = append(fields, inner.Fields[col])
continue
}
field := proto.Clone(inner.Fields[col]).(*querypb.Field)
field.Name = sc.ColNames[idx]
fields = append(fields, field)
GuptaManan100 marked this conversation as resolved.
Show resolved Hide resolved
}
return fields
}
Expand All @@ -114,6 +124,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,
Expand Down
9 changes: 5 additions & 4 deletions go/vt/vtgate/planbuilder/operator_transformers.go
Original file line number Diff line number Diff line change
Expand Up @@ -355,10 +355,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()
Expand Down Expand Up @@ -403,7 +403,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
Expand All @@ -412,7 +412,8 @@ func useSimpleProjection(ctx *plancontext.PlanningContext, op *operators.Project
return &simpleProjection{
logicalPlanCommon: newBuilderCommon(src),
eSimpleProj: &engine.SimpleProjection{
Cols: cols,
Cols: cols,
ColNames: colNames,
},
}, nil
}
Expand Down
16 changes: 12 additions & 4 deletions go/vt/vtgate/planbuilder/operators/projection.go
Original file line number Diff line number Diff line change
Expand Up @@ -399,18 +399,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
}
Expand Down Expand Up @@ -445,7 +450,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
}
Expand Down Expand Up @@ -475,6 +480,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:
Expand Down
58 changes: 58 additions & 0 deletions go/vt/vtgate/planbuilder/testdata/select_cases.json
Original file line number Diff line number Diff line change
Expand Up @@ -4997,5 +4997,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"
]
}
}
]
70 changes: 46 additions & 24 deletions go/vt/vtgate/planbuilder/testdata/union_cases.json
Original file line number Diff line number Diff line change
Expand Up @@ -1490,40 +1490,62 @@
"TableName": "`user`_music",
"Inputs": [
{
"OperatorType": "Distinct",
"Collations": [
"(0:1)"
"OperatorType": "SimpleProjection",
"ColumnNames": [
"foo"
],
"Columns": [
0
],
"Inputs": [
GuptaManan100 marked this conversation as resolved.
Show resolved Hide resolved
{
"OperatorType": "Route",
"Variant": "Scatter",
"Keyspace": {
"Name": "user",
"Sharded": true
},
"FieldQuery": "select dt.foo, weight_string(dt.foo) from (select foo from `user` where 1 != 1 union select foo from `user` where 1 != 1) as dt where 1 != 1",
"Query": "select dt.foo, weight_string(dt.foo) from (select foo from `user` where bar = 12 union select foo from `user` where bar = 134) as dt",
"Table": "`user`"
"OperatorType": "Distinct",
"Collations": [
"(0:1)"
],
"Inputs": [
{
"OperatorType": "Route",
"Variant": "Scatter",
"Keyspace": {
"Name": "user",
"Sharded": true
},
"FieldQuery": "select dt.foo, weight_string(dt.foo) from (select foo from `user` where 1 != 1 union select foo from `user` where 1 != 1) as dt where 1 != 1",
"Query": "select dt.foo, weight_string(dt.foo) from (select foo from `user` where bar = 12 union select foo from `user` where bar = 134) as dt",
"Table": "`user`"
}
]
}
]
},
{
"OperatorType": "Distinct",
"Collations": [
"(0:1)"
"OperatorType": "SimpleProjection",
"ColumnNames": [
"bar"
],
"Columns": [
0
],
"Inputs": [
{
"OperatorType": "Route",
"Variant": "Scatter",
"Keyspace": {
"Name": "user",
"Sharded": true
},
"FieldQuery": "select dt.bar, weight_string(dt.bar) from (select bar from music where 1 != 1 union select bar from music where 1 != 1) as dt where 1 != 1",
"Query": "select dt.bar, weight_string(dt.bar) from (select bar from music where foo = 12 and bar = :t1_foo union select bar from music where foo = 1234 and bar = :t1_foo) as dt",
"Table": "music"
"OperatorType": "Distinct",
"Collations": [
"(0:1)"
],
"Inputs": [
{
"OperatorType": "Route",
"Variant": "Scatter",
"Keyspace": {
"Name": "user",
"Sharded": true
},
"FieldQuery": "select dt.bar, weight_string(dt.bar) from (select bar from music where 1 != 1 union select bar from music where 1 != 1) as dt where 1 != 1",
"Query": "select dt.bar, weight_string(dt.bar) from (select bar from music where foo = 12 and bar = :t1_foo union select bar from music where foo = 1234 and bar = :t1_foo) as dt",
"Table": "music"
}
]
}
]
}
Expand Down
Loading