diff --git a/expression/integration_test/integration_test.go b/expression/integration_test/integration_test.go index a82a777f4d807..e70434fca4794 100644 --- a/expression/integration_test/integration_test.go +++ b/expression/integration_test/integration_test.go @@ -7960,3 +7960,14 @@ func TestIssue45410(t *testing.T) { tk.MustExec("INSERT INTO t1 VALUES (0);") tk.MustQuery("SELECT c1>=CAST('-787360724' AS TIME) FROM t1;").Check(testkit.Rows("1")) } + +func TestIssue41986(t *testing.T) { + store := testkit.CreateMockStore(t) + tk := testkit.NewTestKit(t, store) + + tk.MustExec("use test") + tk.MustExec("CREATE TABLE poi_clearing_time_topic (effective_date datetime DEFAULT NULL , clearing_time int(11) DEFAULT NULL);") + tk.MustExec("insert into poi_clearing_time_topic values ('2023:08:25', 1)") + // shouldn't report they can't find column error and return the right result. + tk.MustQuery("SELECT GROUP_CONCAT(effective_date order by stlmnt_hour DESC) FROM ( SELECT (COALESCE(pct.clearing_time, 0)/3600000) AS stlmnt_hour ,COALESCE(pct.effective_date, '1970-01-01 08:00:00') AS effective_date FROM poi_clearing_time_topic pct ORDER BY pct.effective_date DESC ) a;").Check(testkit.Rows("2023-08-25 00:00:00")) +} diff --git a/planner/core/rule_aggregation_push_down.go b/planner/core/rule_aggregation_push_down.go index c8f5ec8b34bea..aef3ea6136f3a 100644 --- a/planner/core/rule_aggregation_push_down.go +++ b/planner/core/rule_aggregation_push_down.go @@ -23,6 +23,7 @@ import ( "github.com/pingcap/tidb/expression/aggregation" "github.com/pingcap/tidb/parser/ast" "github.com/pingcap/tidb/parser/mysql" + "github.com/pingcap/tidb/planner/util" "github.com/pingcap/tidb/sessionctx" "github.com/pingcap/tidb/types" ) @@ -559,6 +560,8 @@ func (a *aggregationPushDownSolver) aggPushDown(p LogicalPlan, opt *logicalOptim } oldAggFuncsArgs := make([][]expression.Expression, 0, len(agg.AggFuncs)) newAggFuncsArgs := make([][]expression.Expression, 0, len(agg.AggFuncs)) + oldAggOrderItems := make([][]*util.ByItems, 0, len(agg.AggFuncs)) + newAggOrderItems := make([][]*util.ByItems, 0, len(agg.AggFuncs)) if noSideEffects { for _, aggFunc := range agg.AggFuncs { oldAggFuncsArgs = append(oldAggFuncsArgs, aggFunc.Args) @@ -571,6 +574,27 @@ func (a *aggregationPushDownSolver) aggPushDown(p LogicalPlan, opt *logicalOptim break } newAggFuncsArgs = append(newAggFuncsArgs, newArgs) + // for ordeByItems, treat it like agg func's args, if it can be substituted by underlying projection's expression recording them temporarily. + if len(aggFunc.OrderByItems) != 0 { + oldAggOrderItems = append(oldAggOrderItems, aggFunc.OrderByItems) + newOrderByItems := make([]expression.Expression, 0, len(aggFunc.OrderByItems)) + for _, oby := range aggFunc.OrderByItems { + newOrderByItems = append(newOrderByItems, expression.ColumnSubstitute(oby.Expr, proj.schema, proj.Exprs)) + } + if ExprsHasSideEffects(newOrderByItems) { + noSideEffects = false + break + } + oneAggOrderByItems := make([]*util.ByItems, 0, len(aggFunc.OrderByItems)) + for i, obyExpr := range newOrderByItems { + oneAggOrderByItems = append(oneAggOrderByItems, &util.ByItems{Expr: obyExpr, Desc: aggFunc.OrderByItems[i].Desc}) + } + newAggOrderItems = append(newAggOrderItems, oneAggOrderByItems) + } else { + // occupy the pos for convenience of subscript index + oldAggOrderItems = append(oldAggOrderItems, nil) + newAggOrderItems = append(newAggOrderItems, nil) + } } } for i, funcsArgs := range oldAggFuncsArgs { @@ -580,6 +604,16 @@ func (a *aggregationPushDownSolver) aggPushDown(p LogicalPlan, opt *logicalOptim break } } + for j, item := range newAggOrderItems { + if item == nil { + continue + } + // substitution happened, check the eval type compatibility. + if oldAggOrderItems[i][j].Expr.GetType().EvalType() != newAggOrderItems[i][j].Expr.GetType().EvalType() { + noSideEffects = false + break + } + } if !noSideEffects { break } @@ -588,6 +622,7 @@ func (a *aggregationPushDownSolver) aggPushDown(p LogicalPlan, opt *logicalOptim agg.GroupByItems = newGbyItems for i, aggFunc := range agg.AggFuncs { aggFunc.Args = newAggFuncsArgs[i] + aggFunc.OrderByItems = newAggOrderItems[i] } projChild := proj.children[0] agg.SetChildren(projChild) diff --git a/planner/core/rule_eliminate_projection.go b/planner/core/rule_eliminate_projection.go index 83936e7cd9e94..f87548062b0ae 100644 --- a/planner/core/rule_eliminate_projection.go +++ b/planner/core/rule_eliminate_projection.go @@ -259,6 +259,9 @@ func (la *LogicalAggregation) replaceExprColumns(replace map[string]*expression. for _, aggExpr := range agg.Args { ResolveExprAndReplace(aggExpr, replace) } + for _, orderExpr := range agg.OrderByItems { + ResolveExprAndReplace(orderExpr.Expr, replace) + } } for _, gbyItem := range la.GroupByItems { ResolveExprAndReplace(gbyItem, replace)