Horton

Re: [PATCH] GROUP BY ALL

Tom Lane <tgl@sss.pgh.pa.us>

View thread
From: Tom Lane <tgl@sss.pgh.pa.us>
To: David Christensen <david@pgguru.net>
Cc: Peter Eisentraut <peter@eisentraut.org>, pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2025-09-26T20:37:58Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Add GROUP BY ALL.

  2. Refactor to avoid code duplication in transformPLAssignStmt.

  3. Fix missed copying of groupDistinct in transformPLAssignStmt.

Attachments

[ trimming the cc: list because gmail decided my last was spam ]

David Christensen <david@pgguru.net> writes:
> Great feedback, thanks; attached is v4 which should address these comments.

I did a pass of cleanup over this --- mostly cosmetic, but not
entirely.  Along the way I discovered a pre-existing bug:
transformSelectStmt does
 	qry->groupDistinct = stmt->groupDistinct;
but transformPLAssignStmt fails to, with the result that GROUP BY
DISTINCT would misbehave if used in a plpgsql "expression" context.
I'm not hugely surprised that no one has reported that from the
field, but nonetheless it's broken.  In the attached v5 I just
quickly added the missing line and moved on, but we'll need to
back-patch that bit.  (Maybe we'd be well advised to refactor to
reduce the amount of duplicated code that needs to be kept in sync?)

I have not attempted to address the definitional issues I just
queried Peter about.  Other open items:

* Documentation

* The test cases deserve reconsideration now that we think their
charter only goes as far as EXPLAIN'ing the results; some of them
seem pretty redundant in this context.

			regards, tom lane