Skip to content

Commit

Permalink
minor fixups after review
Browse files Browse the repository at this point in the history
Signed-off-by: Andres Taylor <[email protected]>
  • Loading branch information
systay committed Apr 17, 2024
1 parent 8eaf17a commit 8162d43
Show file tree
Hide file tree
Showing 5 changed files with 11 additions and 7 deletions.
2 changes: 1 addition & 1 deletion go/vt/vtgate/planbuilder/operators/offset_planning.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ func addColumnsToInput(ctx *plancontext.PlanningContext, root Operator) Operator
for _, aggr := range aggrOp.Aggregations {
if aggr.OpCode == opcode.AggregateUDF {
// we don't support UDFs in aggregation if it's still above a route
message := fmt.Sprintf("Aggregate UDF %s must be pushed down to MySQL", sqlparser.String(aggr.Original.Expr))
message := fmt.Sprintf("Aggregate UDF '%s' must be pushed down to MySQL", sqlparser.String(aggr.Original.Expr))
panic(vterrors.VT12001(message))
}
}
Expand Down
2 changes: 1 addition & 1 deletion go/vt/vtgate/planbuilder/testdata/aggr_cases.json
Original file line number Diff line number Diff line change
Expand Up @@ -6962,7 +6962,7 @@
{
"comment": "Aggregate UDFs can't be handled by vtgate",
"query": "select id from t1 group by id having udf_aggr(foo) > 1 and sum(foo) = 10",
"plan": "VT03033: aggregate user-defined function udf_aggr(foo) must be pushed down to mysql"
"plan": "VT12001: unsupported: Aggregate UDF 'udf_aggr(foo)' must be pushed down to MySQL"
},
{
"comment": "Valid to run since we can push down the aggregate function because of the grouping",
Expand Down
2 changes: 1 addition & 1 deletion go/vt/vtgate/semantics/analyzer.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ type analyzer struct {
// newAnalyzer create the semantic analyzer
func newAnalyzer(dbName string, si SchemaInformation, fullAnalysis bool) *analyzer {
// TODO dependencies between these components are a little tangled. We should try to clean up
s := newScoper(si.GetAggregateUDFs())
s := newScoper(si)
a := &analyzer{
scoper: s,
earlyTables: newEarlyTableCollector(si, dbName),
Expand Down
4 changes: 4 additions & 0 deletions go/vt/vtgate/semantics/early_rewriter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -541,6 +541,10 @@ func TestHavingColumnName(t *testing.T) {
expSQL: "select id, sum(t1.foo) as foo from t1 having custom_udf(foo) > 1",
expDeps: TS0,
warning: "Column 'foo' in having clause is ambiguous",
}, {
sql: "select id, custom_udf(t1.foo) as foo from t1 having foo > 1",
expSQL: "select id, custom_udf(t1.foo) as foo from t1 having custom_udf(t1.foo) > 1",
expDeps: TS0,
}, {
sql: "select id, sum(t1.foo) as XYZ from t1 having sum(XYZ) > 1",
expErr: "Invalid use of group function",
Expand Down
8 changes: 4 additions & 4 deletions go/vt/vtgate/semantics/scoper.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ type (
// These scopes are only used for rewriting ORDER BY 1 and GROUP BY 1
specialExprScopes map[*sqlparser.Literal]*scope
statementIDs map[sqlparser.Statement]TableSet
aggrUDFs []string
si SchemaInformation
}

scope struct {
Expand All @@ -54,13 +54,13 @@ type (
}
)

func newScoper(udfs []string) *scoper {
func newScoper(si SchemaInformation) *scoper {
return &scoper{
rScope: map[*sqlparser.Select]*scope{},
wScope: map[*sqlparser.Select]*scope{},
specialExprScopes: map[*sqlparser.Literal]*scope{},
statementIDs: map[sqlparser.Statement]TableSet{},
aggrUDFs: udfs,
si: si,
}
}

Expand Down Expand Up @@ -90,7 +90,7 @@ func (s *scoper) down(cursor *sqlparser.Cursor) error {
if !s.currentScope().inHaving {
break
}
if node.Name.EqualsAnyString(s.aggrUDFs) {
if node.Name.EqualsAnyString(s.si.GetAggregateUDFs()) {
s.currentScope().inHavingAggr = true
}
case *sqlparser.Where:
Expand Down

0 comments on commit 8162d43

Please sign in to comment.