I wrote:
> shihao zhong <[email protected]> writes:
>> 1. adjust_group_pathkeys_for_groupagg() sets aggpresorted on the
>> aggregates it picks and nothing clears it. When self join elimination
>> removes a rel and the second pass picks a different presorted group,
>> the stale flag makes the executor skip the sort.
> Hmm. I've not looked at exactly what that function does, but just
> going by the name, why is it being called inside the restart loop
> rather than later? I feel like anything to do with identifying
> pathkeys could/should wait till after join removal.
Concretely, the attached fixes the given test case and passes
check-world; but I've not tried harder than that to break it.
regards, tom lane
diff --git a/src/backend/optimizer/plan/planmain.c b/src/backend/optimizer/plan/planmain.c
index 68d3409476c..9045c490486 100644
--- a/src/backend/optimizer/plan/planmain.c
+++ b/src/backend/optimizer/plan/planmain.c
@@ -260,13 +260,6 @@ restart:
*/
generate_base_implied_equalities(root);
- /*
- * We have completed merging equivalence sets, so it's now possible to
- * generate pathkeys in canonical form; so compute query_pathkeys and
- * other pathkeys fields in PlannerInfo.
- */
- (*qp_callback) (root, qp_extra);
-
/*
* Examine any "placeholder" expressions generated during subquery pullup.
* Make sure that the Vars they need are marked as needed at the relevant
@@ -305,6 +298,13 @@ restart:
* this point does not need to be able to restart.
*/
+ /*
+ * We have completed merging equivalence sets, so it's now possible to
+ * generate pathkeys in canonical form; so compute query_pathkeys and
+ * other pathkeys fields in PlannerInfo.
+ */
+ (*qp_callback) (root, qp_extra);
+
/*
* Now distribute "placeholders" to base rels as needed. This has to be
* done after join removal because removal could change whether a
