> On 29 Jun 2026, at 09:20, Chao Li <[email protected]> wrote:
> The fix mostly refactors the existing logic so the GROUP BY ALL path also
> handles the ORDER BY sort clause. See the attached patch for details.
Thanks for the report and the patch, I am going to study it a bit more but I
have a few small comments on the patch.
+/*
+ * Add a targetlist entry to the GROUP BY list, copying matching ORDER BY
+ * operator information if available.
+ */
This comment is a bit light for such an important routine. The foreach has a
really great comment, I think we should move some (most?) of it to the function
comment.
- *flatresult = lappend(*flatresult, grpc);
+ grouplist = lappend(grouplist, grpc);
found = true;
break;
Now that we are in an external function, we could just return grouplist here
instead of breaking with a flag. The code outside the foreach can then call
addTargetToGroupList directly since we know that if we reach thus far there was
no match.
--
Daniel Gustafsson
