Skip to content

Commit

Permalink
make column resolution closer to mysql
Browse files Browse the repository at this point in the history
Signed-off-by: Andres Taylor <andres@planetscale.com>
  • Loading branch information
systay authored and timvaillancourt committed Nov 9, 2023
1 parent a6c860c commit 66b8b39
Show file tree
Hide file tree
Showing 4 changed files with 121 additions and 39 deletions.
60 changes: 34 additions & 26 deletions go/vt/vtgate/planbuilder/abstract/queryprojection.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,17 +152,15 @@ func CreateQPFromSelect(sel *sqlparser.Select) (*QueryProjection, error) {
}
for _, group := range sel.GroupBy {
selectExprIdx, aliasExpr := qp.FindSelectExprIndexForExpr(group)
expr, weightStrExpr, err := qp.GetSimplifiedExpr(group)
if err != nil {
return nil, err
}
selectExprIdx, aliasExpr := qp.FindSelectExprIndexForExpr(ctx, group)

Check failure on line 155 in go/vt/vtgate/planbuilder/abstract/queryprojection.go

View workflow job for this annotation

GitHub Actions / Region Sharding example using etcd on ubuntu-latest

no new variables on left side of :=

Check failure on line 155 in go/vt/vtgate/planbuilder/abstract/queryprojection.go

View workflow job for this annotation

GitHub Actions / Region Sharding example using etcd on ubuntu-latest

undefined: ctx

Check failure on line 155 in go/vt/vtgate/planbuilder/abstract/queryprojection.go

View workflow job for this annotation

GitHub Actions / End-to-End Test

no new variables on left side of :=

Check failure on line 155 in go/vt/vtgate/planbuilder/abstract/queryprojection.go

View workflow job for this annotation

GitHub Actions / End-to-End Test

undefined: ctx

Check failure on line 155 in go/vt/vtgate/planbuilder/abstract/queryprojection.go

View workflow job for this annotation

GitHub Actions / End-to-End Test (Race)

no new variables on left side of :=

Check failure on line 155 in go/vt/vtgate/planbuilder/abstract/queryprojection.go

View workflow job for this annotation

GitHub Actions / End-to-End Test (Race)

undefined: ctx

Check failure on line 155 in go/vt/vtgate/planbuilder/abstract/queryprojection.go

View workflow job for this annotation

GitHub Actions / Local example using etcd on ubuntu-latest

no new variables on left side of :=

Check failure on line 155 in go/vt/vtgate/planbuilder/abstract/queryprojection.go

View workflow job for this annotation

GitHub Actions / Local example using etcd on ubuntu-latest

undefined: ctx

Check failure on line 155 in go/vt/vtgate/planbuilder/abstract/queryprojection.go

View workflow job for this annotation

GitHub Actions / Local example using k8s on ubuntu-latest

no new variables on left side of :=

Check failure on line 155 in go/vt/vtgate/planbuilder/abstract/queryprojection.go

View workflow job for this annotation

GitHub Actions / Local example using k8s on ubuntu-latest

undefined: ctx

Check failure on line 155 in go/vt/vtgate/planbuilder/abstract/queryprojection.go

View workflow job for this annotation

GitHub Actions / Local example using consul on ubuntu-latest

no new variables on left side of :=

Check failure on line 155 in go/vt/vtgate/planbuilder/abstract/queryprojection.go

View workflow job for this annotation

GitHub Actions / Local example using consul on ubuntu-latest

undefined: ctx

Check failure on line 155 in go/vt/vtgate/planbuilder/abstract/queryprojection.go

View workflow job for this annotation

GitHub Actions / Static Code Checks Etc

undeclared name: `ctx` (typecheck)

Check failure on line 155 in go/vt/vtgate/planbuilder/abstract/queryprojection.go

View workflow job for this annotation

GitHub Actions / Docker Test Cluster 10

no new variables on left side of :=

Check failure on line 155 in go/vt/vtgate/planbuilder/abstract/queryprojection.go

View workflow job for this annotation

GitHub Actions / Docker Test Cluster 10

undefined: ctx

Check failure on line 155 in go/vt/vtgate/planbuilder/abstract/queryprojection.go

View workflow job for this annotation

GitHub Actions / Docker Test Cluster 10

no new variables on left side of :=

Check failure on line 155 in go/vt/vtgate/planbuilder/abstract/queryprojection.go

View workflow job for this annotation

GitHub Actions / Docker Test Cluster 10

undefined: ctx

Check failure on line 155 in go/vt/vtgate/planbuilder/abstract/queryprojection.go

View workflow job for this annotation

GitHub Actions / Docker Test Cluster 10

no new variables on left side of :=

Check failure on line 155 in go/vt/vtgate/planbuilder/abstract/queryprojection.go

View workflow job for this annotation

GitHub Actions / Docker Test Cluster 10

undefined: ctx

Check failure on line 155 in go/vt/vtgate/planbuilder/abstract/queryprojection.go

View workflow job for this annotation

GitHub Actions / Run Upgrade Downgrade Test - Backups - E2E

no new variables on left side of :=

Check failure on line 155 in go/vt/vtgate/planbuilder/abstract/queryprojection.go

View workflow job for this annotation

GitHub Actions / Run Upgrade Downgrade Test - Backups - E2E

undefined: ctx

Check failure on line 155 in go/vt/vtgate/planbuilder/abstract/queryprojection.go

View workflow job for this annotation

GitHub Actions / Docker Test Cluster 25

no new variables on left side of :=

Check failure on line 155 in go/vt/vtgate/planbuilder/abstract/queryprojection.go

View workflow job for this annotation

GitHub Actions / Docker Test Cluster 25

undefined: ctx

Check failure on line 155 in go/vt/vtgate/planbuilder/abstract/queryprojection.go

View workflow job for this annotation

GitHub Actions / Docker Test Cluster 25

no new variables on left side of :=

Check failure on line 155 in go/vt/vtgate/planbuilder/abstract/queryprojection.go

View workflow job for this annotation

GitHub Actions / Docker Test Cluster 25

undefined: ctx

Check failure on line 155 in go/vt/vtgate/planbuilder/abstract/queryprojection.go

View workflow job for this annotation

GitHub Actions / Docker Test Cluster 25

no new variables on left side of :=

Check failure on line 155 in go/vt/vtgate/planbuilder/abstract/queryprojection.go

View workflow job for this annotation

GitHub Actions / Docker Test Cluster 25

undefined: ctx

Check failure on line 155 in go/vt/vtgate/planbuilder/abstract/queryprojection.go

View workflow job for this annotation

GitHub Actions / Docker Test Cluster 25

no new variables on left side of :=

Check failure on line 155 in go/vt/vtgate/planbuilder/abstract/queryprojection.go

View workflow job for this annotation

GitHub Actions / Docker Test Cluster 25

undefined: ctx

Check failure on line 155 in go/vt/vtgate/planbuilder/abstract/queryprojection.go

View workflow job for this annotation

GitHub Actions / Run Upgrade Downgrade Test - Backups - Manual

no new variables on left side of :=

Check failure on line 155 in go/vt/vtgate/planbuilder/abstract/queryprojection.go

View workflow job for this annotation

GitHub Actions / Run Upgrade Downgrade Test - Backups - Manual

undefined: ctx

Check failure on line 155 in go/vt/vtgate/planbuilder/abstract/queryprojection.go

View workflow job for this annotation

GitHub Actions / Run Upgrade Downgrade Test - Reparent Old VTTablet

no new variables on left side of :=

Check failure on line 155 in go/vt/vtgate/planbuilder/abstract/queryprojection.go

View workflow job for this annotation

GitHub Actions / Run Upgrade Downgrade Test - Reparent Old VTTablet

undefined: ctx

Check failure on line 155 in go/vt/vtgate/planbuilder/abstract/queryprojection.go

View workflow job for this annotation

GitHub Actions / Run Upgrade Downgrade Test - Reparent Old Vtctl

no new variables on left side of :=

Check failure on line 155 in go/vt/vtgate/planbuilder/abstract/queryprojection.go

View workflow job for this annotation

GitHub Actions / Run Upgrade Downgrade Test - Reparent Old Vtctl

undefined: ctx

Check failure on line 155 in go/vt/vtgate/planbuilder/abstract/queryprojection.go

View workflow job for this annotation

GitHub Actions / Run Upgrade Downgrade Test - Query Serving (Schema)

no new variables on left side of :=

Check failure on line 155 in go/vt/vtgate/planbuilder/abstract/queryprojection.go

View workflow job for this annotation

GitHub Actions / Run Upgrade Downgrade Test - Query Serving (Schema)

undefined: ctx

Check failure on line 155 in go/vt/vtgate/planbuilder/abstract/queryprojection.go

View workflow job for this annotation

GitHub Actions / Run Upgrade Downgrade Test - Query Serving (Queries)

no new variables on left side of :=

Check failure on line 155 in go/vt/vtgate/planbuilder/abstract/queryprojection.go

View workflow job for this annotation

GitHub Actions / Run Upgrade Downgrade Test - Query Serving (Queries)

undefined: ctx
weightStrExpr := qp.GetSimplifiedExpr(group)
err = checkForInvalidGroupingExpressions(weightStrExpr)
if err != nil {
return nil, err
}

groupBy := GroupBy{
Inner: expr,
Inner: group,
WeightStrExpr: weightStrExpr,
InnerIndex: selectExprIdx,
aliasedExpr: aliasExpr,
Expand Down Expand Up @@ -274,17 +272,14 @@ func CreateQPFromUnion(union *sqlparser.Union) (*QueryProjection, error) {
func (qp *QueryProjection) addOrderBy(orderBy sqlparser.OrderBy) error {
canPushDownSorting := true
for _, order := range orderBy {
expr, weightStrExpr, err := qp.GetSimplifiedExpr(order.Expr)
if err != nil {
return err
}
weightStrExpr := qp.GetSimplifiedExpr(order.Expr)
if sqlparser.IsNull(weightStrExpr) {
// ORDER BY null can safely be ignored
continue
}
qp.OrderExprs = append(qp.OrderExprs, OrderBy{
Inner: &sqlparser.Order{
Expr: expr,
Expr: order.Expr,
Direction: order.Direction,
},
WeightStrExpr: weightStrExpr,
Expand Down Expand Up @@ -330,34 +325,47 @@ func (qp *QueryProjection) isExprInGroupByExprs(expr SelectExpr) bool {
}

// GetSimplifiedExpr takes an expression used in ORDER BY or GROUP BY, and returns an expression that is simpler to evaluate
func (qp *QueryProjection) GetSimplifiedExpr(e sqlparser.Expr) (expr sqlparser.Expr, weightStrExpr sqlparser.Expr, err error) {
func (qp *QueryProjection) GetSimplifiedExpr(e sqlparser.Expr) (found sqlparser.Expr) {
if qp == nil {
return e
}
// If the ORDER BY is against a column alias, we need to remember the expression
// behind the alias. The weightstring(.) calls needs to be done against that expression and not the alias.
// Eg - select music.foo as bar, weightstring(music.foo) from music order by bar

colExpr, isColName := e.(*sqlparser.ColName)
if !isColName {
return e, e, nil
}

if sqlparser.IsNull(e) {
return e, nil, nil
in, isColName := e.(*sqlparser.ColName)
if !(isColName && in.Qualifier.IsEmpty()) {
// we are only interested in unqualified column names. if it's not a column name and not unqualified, we're done
return e
}

if colExpr.Qualifier.IsEmpty() {
for _, selectExpr := range qp.SelectExprs {
aliasedExpr, isAliasedExpr := selectExpr.Col.(*sqlparser.AliasedExpr)
if !isAliasedExpr {
for _, selectExpr := range qp.SelectExprs {
ae, ok := selectExpr.Col.(*sqlparser.AliasedExpr)
if !ok {
continue
}
aliased := !ae.As.IsEmpty()
if aliased {
if in.Name.Equal(ae.As) {
return ae.Expr
}
} else {
seCol, ok := ae.Expr.(*sqlparser.ColName)
if !ok {
continue
}
isAliasExpr := !aliasedExpr.As.IsEmpty()
if isAliasExpr && colExpr.Name.Equal(aliasedExpr.As) {
return e, aliasedExpr.Expr, nil
if seCol.Name.Equal(in.Name) {
// If the column name matches, we have a match, even if the table name is not listed
return ae.Expr
}
}
}

return e, e, nil
if found == nil {
found = e
}

return found
}

// toString should only be used for tests
Expand Down
13 changes: 4 additions & 9 deletions go/vt/vtgate/planbuilder/horizon_planning.go
Original file line number Diff line number Diff line change
Expand Up @@ -531,10 +531,8 @@ func (hp *horizonPlanning) handleDistinctAggr(ctx *plancontext.PlanningContext,
continue
}

inner, innerWS, err := hp.qp.GetSimplifiedExpr(expr.Func.GetArg())
if err != nil {
return nil, nil, nil, err
}
inner := expr.Func.GetArg()
innerWS := hp.qp.GetSimplifiedExpr(inner)
if exprHasVindex(ctx.SemTable, innerWS, false) {
aggrs = append(aggrs, expr)
continue
Expand Down Expand Up @@ -590,13 +588,10 @@ func newOffset(col int) offsets {
func (hp *horizonPlanning) createGroupingsForColumns(columns []*sqlparser.ColName) ([]abstract.GroupBy, error) {
var lhsGrouping []abstract.GroupBy
for _, lhsColumn := range columns {
expr, wsExpr, err := hp.qp.GetSimplifiedExpr(lhsColumn)
if err != nil {
return nil, err
}
wsExpr := hp.qp.GetSimplifiedExpr(lhsColumn)

lhsGrouping = append(lhsGrouping, abstract.GroupBy{
Inner: expr,
Inner: lhsColumn,
WeightStrExpr: wsExpr,
})
}
Expand Down
8 changes: 4 additions & 4 deletions go/vt/vtgate/planbuilder/testdata/aggr_cases.json
Original file line number Diff line number Diff line change
Expand Up @@ -4393,9 +4393,9 @@
"Name": "user",
"Sharded": true
},
"FieldQuery": "select id, b as id, count(*), weight_string(b) from `user` where 1 != 1",
"FieldQuery": "select id, b as id, count(*), weight_string(id) from `user` where 1 != 1",
"OrderBy": "(1|3) ASC",
"Query": "select id, b as id, count(*), weight_string(b) from `user` order by id asc",
"Query": "select id, b as id, count(*), weight_string(id) from `user` order by id asc",
"Table": "`user`"
}
]
Expand Down Expand Up @@ -4491,9 +4491,9 @@
"Name": "user",
"Sharded": true
},
"FieldQuery": "select `user`.id, weight_string(id) from `user` where 1 != 1 group by id, weight_string(id)",
"FieldQuery": "select id, weight_string(`user`.id) from `user` where 1 != 1 group by id, weight_string(`user`.id)",
"OrderBy": "(0|1) ASC",
"Query": "select `user`.id, weight_string(id) from `user` group by id, weight_string(id) order by id asc",
"Query": "select id, weight_string(`user`.id) from `user` group by id, weight_string(`user`.id) order by id asc",
"Table": "`user`"
},
{
Expand Down
79 changes: 79 additions & 0 deletions go/vt/vtgate/planbuilder/testdata/select_cases.json
Original file line number Diff line number Diff line change
Expand Up @@ -7983,5 +7983,84 @@
"user.user"
]
}
},
{
"comment": "column with qualifier is correctly used",
"query": "select u.foo, ue.foo as apa from user u, user_extra ue order by foo ",
"v3-plan": {
"QueryType": "SELECT",
"Original": "select u.foo, ue.foo as apa from user u, user_extra ue order by foo ",
"Instructions": {
"OperatorType": "Join",
"Variant": "Join",
"JoinColumnIndexes": "L:0,R:0",
"TableName": "`user`_user_extra",
"Inputs": [
{
"OperatorType": "Route",
"Variant": "Scatter",
"Keyspace": {
"Name": "user",
"Sharded": true
},
"FieldQuery": "select u.foo, weight_string(u.foo) from `user` as u where 1 != 1",
"OrderBy": "(0|1) ASC",
"Query": "select u.foo, weight_string(u.foo) from `user` as u order by foo asc",
"ResultColumns": 1,
"Table": "`user`"
},
{
"OperatorType": "Route",
"Variant": "Scatter",
"Keyspace": {
"Name": "user",
"Sharded": true
},
"FieldQuery": "select ue.foo as apa from user_extra as ue where 1 != 1",
"Query": "select ue.foo as apa from user_extra as ue",
"Table": "user_extra"
}
]
}
},
"gen4-plan": {
"QueryType": "SELECT",
"Original": "select u.foo, ue.foo as apa from user u, user_extra ue order by foo ",
"Instructions": {
"OperatorType": "Join",
"Variant": "Join",
"JoinColumnIndexes": "L:0,R:0",
"TableName": "`user`_user_extra",
"Inputs": [
{
"OperatorType": "Route",
"Variant": "Scatter",
"Keyspace": {
"Name": "user",
"Sharded": true
},
"FieldQuery": "select u.foo, weight_string(u.foo) from `user` as u where 1 != 1",
"OrderBy": "(0|1) ASC",
"Query": "select u.foo, weight_string(u.foo) from `user` as u order by foo asc",
"Table": "`user`"
},
{
"OperatorType": "Route",
"Variant": "Scatter",
"Keyspace": {
"Name": "user",
"Sharded": true
},
"FieldQuery": "select ue.foo as apa from user_extra as ue where 1 != 1",
"Query": "select ue.foo as apa from user_extra as ue",
"Table": "user_extra"
}
]
},
"TablesUsed": [
"user.user",
"user.user_extra"
]
}
}
]

0 comments on commit 66b8b39

Please sign in to comment.