> On 29 Jun 2026, at 09:20, Chao Li <li.evan.chao@gmail.com> wrote:
> The fix mostly refactors the existing logic so the GROUP BY ALL path also handles the ORDER BY sort clause. See the
attachedpatch 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
