Skip to content

Commit

Permalink
fail aggregations queries that are not supported
Browse files Browse the repository at this point in the history
Signed-off-by: Andres Taylor <[email protected]>
  • Loading branch information
systay committed Jul 16, 2021
1 parent 22d832c commit 4f8c5b1
Show file tree
Hide file tree
Showing 5 changed files with 33 additions and 17 deletions.
1 change: 1 addition & 0 deletions go/vt/sqlparser/ast_funcs.go
Original file line number Diff line number Diff line change
Expand Up @@ -1373,6 +1373,7 @@ func ContainsAggregation(e Expr) bool {
_ = Walk(func(node SQLNode) (kontinue bool, err error) {
if IsAggregation(node) {
hasAggregates = true
return false, nil
}
return true, nil
}, e)
Expand Down
40 changes: 26 additions & 14 deletions go/vt/vtgate/planbuilder/abstract/queryprojection.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,22 +68,19 @@ func CreateQPFromSelect(sel *sqlparser.Select) (*QueryProjection, error) {
if !ok {
return nil, semantics.Gen4NotSupportedF("%T in select list", selExp)
}
fExpr, ok := exp.Expr.(*sqlparser.FuncExpr)
if ok && fExpr.IsAggregate() {
if len(fExpr.Exprs) != 1 {
return nil, vterrors.NewErrorf(vtrpcpb.Code_INVALID_ARGUMENT, vterrors.SyntaxError, "aggregate functions take a single argument '%s'", sqlparser.String(fExpr))
}
if fExpr.Distinct {
return nil, semantics.Gen4NotSupportedF("distinct aggregation")
}

if err := checkForInvalidAggregations(exp); err != nil {
return nil, err
}
col := SelectExpr{
Col: exp,
}
if sqlparser.ContainsAggregation(exp.Expr) {
col.Aggr = true
qp.HasAggr = true
qp.SelectExprs = append(qp.SelectExprs, SelectExpr{
Col: exp,
Aggr: true,
})
continue
}
qp.SelectExprs = append(qp.SelectExprs, SelectExpr{Col: exp})

qp.SelectExprs = append(qp.SelectExprs, col)
}

for _, group := range sel.GroupBy {
Expand Down Expand Up @@ -131,6 +128,21 @@ func CreateQPFromSelect(sel *sqlparser.Select) (*QueryProjection, error) {
return qp, nil
}

func checkForInvalidAggregations(exp *sqlparser.AliasedExpr) error {
return sqlparser.Walk(func(node sqlparser.SQLNode) (kontinue bool, err error) {
fExpr, ok := node.(*sqlparser.FuncExpr)
if ok && fExpr.IsAggregate() {
if len(fExpr.Exprs) != 1 {
return false, vterrors.NewErrorf(vtrpcpb.Code_INVALID_ARGUMENT, vterrors.SyntaxError, "aggregate functions take a single argument '%s'", sqlparser.String(fExpr))
}
if fExpr.Distinct {
return false, semantics.Gen4NotSupportedF("distinct aggregation")
}
}
return true, nil
}, exp.Expr)
}

func (qp *QueryProjection) getNonAggrExprNotMatchingGroupByExprs() sqlparser.Expr {
for _, expr := range qp.SelectExprs {
if expr.Aggr {
Expand Down
5 changes: 4 additions & 1 deletion go/vt/vtgate/planbuilder/selectGen4.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,10 @@ func planAggregations(qp *abstract.QueryProjection, plan logicalPlan, semTable *
return nil, false, false, err
}
if e.Aggr && oa != nil {
fExpr := e.Col.Expr.(*sqlparser.FuncExpr)
fExpr, isFunc := e.Col.Expr.(*sqlparser.FuncExpr)
if !isFunc {
return nil, false, false, vterrors.Errorf(vtrpcpb.Code_UNIMPLEMENTED, "unsupported: in scatter query: complex aggregate expression")
}
opcode := engine.SupportedAggregates[fExpr.Name.Lowered()]
oa.eaggr.Aggregates = append(oa.eaggr.Aggregates, engine.AggregateParams{
Opcode: opcode,
Expand Down
2 changes: 1 addition & 1 deletion go/vt/vtgate/planbuilder/testdata/aggr_cases.txt
Original file line number Diff line number Diff line change
Expand Up @@ -680,7 +680,7 @@ Gen4 plan same as above
"Table": "`user`"
}
}
Gen4 plan same as above
Gen4 error: Expression of SELECT list is not in GROUP BY clause and contains nonaggregated column 'val' which is not functionally dependent on columns in GROUP BY clause; this is incompatible with sql_mode=only_full_group_by

# group by a unique vindex where it should skip non-aliased expressions.
"select *, id, 1+count(*) from user group by id"
Expand Down
2 changes: 1 addition & 1 deletion go/vt/vtgate/planbuilder/testdata/onecase.txt
Original file line number Diff line number Diff line change
@@ -1 +1 @@
# Add your test case here for debugging and run go test -run=One.
# Add your test case here for debugging and run go test -run=One.

0 comments on commit 4f8c5b1

Please sign in to comment.