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 all 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 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")
}
9 changes: 8 additions & 1 deletion go/vt/vtgate/engine/cached_size.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

27 changes: 23 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 @@
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 @@
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,13 @@
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]

Check warning on line 115 in go/vt/vtgate/engine/simple_projection.go

View check run for this annotation

Codecov / codecov/patch

go/vt/vtgate/engine/simple_projection.go#L114-L115

Added lines #L114 - L115 were not covered by tests
}
fields = append(fields, field)
}
return fields
}
Expand All @@ -114,6 +123,16 @@
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
15 changes: 9 additions & 6 deletions go/vt/vtgate/engine/simple_projection_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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{
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 @@

// 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

Check warning on line 405 in go/vt/vtgate/planbuilder/operators/projection.go

View check run for this annotation

Codecov / codecov/patch

go/vt/vtgate/planbuilder/operators/projection.go#L405

Added line #L405 was not covered by tests
}
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 @@
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 @@
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
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@
"Name": "user",
"Sharded": true
},
"Query": "create view view_a as 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 join authoritative as b on a.user_id = b.user_id"
"Query": "create view view_a as select a.user_id, a.col1, a.col2, b.user_id, b.col1, b.col2 from authoritative as a join authoritative as b on a.user_id = b.user_id"
},
"TablesUsed": [
"user.view_a"
Expand Down Expand Up @@ -201,7 +201,7 @@
"Name": "user",
"Sharded": true
},
"Query": "create view view_a as select `user`.id, a.user_id as user_id, a.col1 as col1, a.col2 as col2, `user`.col1 from authoritative as a join `user` on a.user_id = `user`.id"
"Query": "create view view_a as select `user`.id, a.user_id, a.col1, a.col2, `user`.col1 from authoritative as a join `user` on a.user_id = `user`.id"
},
"TablesUsed": [
"user.view_a"
Expand Down
8 changes: 4 additions & 4 deletions go/vt/vtgate/planbuilder/testdata/from_cases.json
Original file line number Diff line number Diff line change
Expand Up @@ -3760,8 +3760,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"
},
{
Expand All @@ -3771,8 +3771,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"
}
]
Expand Down
4 changes: 2 additions & 2 deletions go/vt/vtgate/planbuilder/testdata/info_schema57_cases.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
}
Expand Down
4 changes: 2 additions & 2 deletions go/vt/vtgate/planbuilder/testdata/info_schema80_cases.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
}
Expand Down
4 changes: 2 additions & 2 deletions go/vt/vtgate/planbuilder/testdata/rails_cases.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
},
{
Expand Down
66 changes: 62 additions & 4 deletions go/vt/vtgate/planbuilder/testdata/select_cases.json
Original file line number Diff line number Diff line change
Expand Up @@ -361,8 +361,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": [
Expand Down Expand Up @@ -433,8 +433,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": [
Expand Down 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"
]
}
}
]
6 changes: 1 addition & 5 deletions go/vt/vtgate/semantics/early_rewriter.go
Original file line number Diff line number Diff line change
Expand Up @@ -1154,17 +1154,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 {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

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)
}
Expand Down
Loading
Loading