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-19.0] planner: Handle ORDER BY inside derived tables (#16353) #16359

Merged
merged 5 commits into from
Jul 10, 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
Original file line number Diff line number Diff line change
Expand Up @@ -433,7 +433,12 @@ func TestOrderByCount(t *testing.T) {

mcmp.Exec("insert into t9(id1, id2, id3) values(1, '1', '1'), (2, '2', '2'), (3, '2', '2'), (4, '3', '3'), (5, '3', '3'), (6, '3', '3')")

mcmp.AssertMatches("SELECT t9.id2 FROM t9 GROUP BY t9.id2 ORDER BY COUNT(t9.id2) DESC", `[[VARCHAR("3")] [VARCHAR("2")] [VARCHAR("1")]]`)
mcmp.Exec("SELECT t9.id2 FROM t9 GROUP BY t9.id2 ORDER BY COUNT(t9.id2) DESC")
version, err := cluster.GetMajorVersion("vtgate")
require.NoError(t, err)
if version == 19 {
mcmp.Exec("select COUNT(*) from (select 1 as one FROM t9 WHERE id3 = 3 ORDER BY id1 DESC LIMIT 3 OFFSET 0) subquery_for_count")
}
}

func TestAggregateAnyValue(t *testing.T) {
Expand Down
9 changes: 8 additions & 1 deletion go/vt/vtgate/planbuilder/operators/horizon_expanding.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,16 @@ func expandUnionHorizon(ctx *plancontext.PlanningContext, horizon *Horizon, unio
}

func expandSelectHorizon(ctx *plancontext.PlanningContext, horizon *Horizon, sel *sqlparser.Select) (Operator, *ApplyResult) {
op := createProjectionFromSelect(ctx, horizon)
qp := horizon.getQP(ctx)
var extracted []string
if horizon.IsDerived() {
// if we are dealing with a derived table, we need to make sure that the ordering columns
// are available outside the derived table
for _, order := range horizon.Query.GetOrderBy() {
qp.addColumn(ctx, order.Expr)
}
}
op := createProjectionFromSelect(ctx, horizon)
if qp.HasAggr {
extracted = append(extracted, "Aggregation")
} else {
Expand Down
17 changes: 17 additions & 0 deletions go/vt/vtgate/planbuilder/operators/queryprojection.go
Original file line number Diff line number Diff line change
Expand Up @@ -743,6 +743,23 @@
return true
}

// addColumn adds a column to the QueryProjection if it is not already present
func (qp *QueryProjection) addColumn(ctx *plancontext.PlanningContext, expr sqlparser.Expr) {
for _, selectExpr := range qp.SelectExprs {
getExpr, err := selectExpr.GetExpr()
if err != nil {
continue

Check warning on line 751 in go/vt/vtgate/planbuilder/operators/queryprojection.go

View check run for this annotation

Codecov / codecov/patch

go/vt/vtgate/planbuilder/operators/queryprojection.go#L751

Added line #L751 was not covered by tests
}
if ctx.SemTable.EqualsExprWithDeps(getExpr, expr) {
return
}
}
qp.SelectExprs = append(qp.SelectExprs, SelectExpr{
Col: aeWrap(expr),
Aggr: containsAggr(expr),
})
}

func checkForInvalidGroupingExpressions(expr sqlparser.Expr) {
_ = sqlparser.Walk(func(node sqlparser.SQLNode) (bool, error) {
if _, isAggregate := node.(sqlparser.AggrFunc); isAggregate {
Expand Down
45 changes: 45 additions & 0 deletions go/vt/vtgate/planbuilder/testdata/aggr_cases.json
Original file line number Diff line number Diff line change
Expand Up @@ -6696,6 +6696,51 @@
]
}
},
{
"comment": "Aggregation over a ORDER BY/LIMIT inside a derived table",
"query": "SELECT COUNT(*) FROM (SELECT 1 AS one FROM `user` WHERE `user`.`is_not_deleted` = true ORDER BY id DESC LIMIT 25 OFFSET 0) subquery_for_count",
"plan": {
"QueryType": "SELECT",
"Original": "SELECT COUNT(*) FROM (SELECT 1 AS one FROM `user` WHERE `user`.`is_not_deleted` = true ORDER BY id DESC LIMIT 25 OFFSET 0) subquery_for_count",
"Instructions": {
"OperatorType": "Aggregate",
"Variant": "Scalar",
"Aggregates": "count_star(0) AS count(*)",
"Inputs": [
{
"OperatorType": "SimpleProjection",
"Columns": [
2
],
"Inputs": [
{
"OperatorType": "Limit",
"Count": "25",
"Offset": "0",
"Inputs": [
{
"OperatorType": "Route",
"Variant": "Scatter",
"Keyspace": {
"Name": "user",
"Sharded": true
},
"FieldQuery": "select subquery_for_count.one, subquery_for_count.id, 1, weight_string(subquery_for_count.id) from (select 1 as one, id from `user` where 1 != 1) as subquery_for_count where 1 != 1",
"OrderBy": "(1|3) DESC",
"Query": "select subquery_for_count.one, subquery_for_count.id, 1, weight_string(subquery_for_count.id) from (select 1 as one, id from `user` where `user`.is_not_deleted = true) as subquery_for_count order by id desc limit :__upper_limit",
"Table": "`user`"
}
]
}
]
}
]
},
"TablesUsed": [
"user.user"
]
}
},
{
"comment": "sharded subquery inside group_concat multi-column aggregation function on a sharded table on same vindex value",
"query": "select max((select group_concat(col1, col2) from user where id = 1)) from user where id = 1",
Expand Down
Loading