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

[release-20.0] Fix Incorrect Optimization with LIMIT and GROUP BY (#16263) #16268

Merged
merged 6 commits into from
Jun 27, 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
17 changes: 17 additions & 0 deletions go/test/endtoend/vtgate/queries/aggregation/aggregation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package aggregation

import (
"fmt"
"math/rand/v2"
"slices"
"sort"
"strings"
Expand Down Expand Up @@ -69,6 +70,22 @@ func start(t *testing.T) (utils.MySQLCompare, func()) {
}
}

func TestAggrWithLimit(t *testing.T) {
version, err := cluster.GetMajorVersion("vtgate")
require.NoError(t, err)
if version != 20 {
t.Skip("Test requires VTGate version 20")
}
mcmp, closer := start(t)
defer closer()

for i := range 1000 {
r := rand.IntN(50)
mcmp.Exec(fmt.Sprintf("insert into aggr_test(id, val1, val2) values(%d, 'a', %d)", i, r))
}
mcmp.Exec("select val2, count(*) from aggr_test group by val2 order by count(*), val2 limit 10")
}

func TestAggregateTypes(t *testing.T) {
mcmp, closer := start(t)
defer closer()
Expand Down
8 changes: 6 additions & 2 deletions go/vt/vtgate/planbuilder/operators/query_planning.go
Original file line number Diff line number Diff line change
Expand Up @@ -490,6 +490,11 @@ func setUpperLimit(in *Limit) (Operator, *ApplyResult) {
case *Join, *ApplyJoin, *SubQueryContainer, *SubQuery:
// we can't push limits down on either side
return SkipChildren
case *Aggregator:
if len(op.Grouping) > 0 {
// we can't push limits down if we have a group by
return SkipChildren
}
case *Route:
newSrc := &Limit{
Source: op.Source,
Expand All @@ -498,9 +503,8 @@ func setUpperLimit(in *Limit) (Operator, *ApplyResult) {
op.Source = newSrc
result = result.Merge(Rewrote("push upper limit under route"))
return SkipChildren
default:
return VisitChildren
}
return VisitChildren
}

TopDown(in.Source, TableID, visitor, shouldVisit)
Expand Down
2 changes: 1 addition & 1 deletion go/vt/vtgate/planbuilder/testdata/memory_sort_cases.json
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@
},
"FieldQuery": "select a, b, count(*) as k, weight_string(a) from `user` where 1 != 1 group by a, weight_string(a)",
"OrderBy": "(0|3) ASC",
"Query": "select a, b, count(*) as k, weight_string(a) from `user` group by a, weight_string(a) order by a asc limit :__upper_limit",
"Query": "select a, b, count(*) as k, weight_string(a) from `user` group by a, weight_string(a) order by a asc",
"Table": "`user`"
}
]
Expand Down
Loading