Re: Inconsistent Behavior of GROUP BY ROLLUP in v17 vs master
Richard Guo <guofenglinux@gmail.com>
From: Richard Guo <guofenglinux@gmail.com>
To: David Rowley <dgrowleyml@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, 邱宇航 <iamqyh@gmail.com>, Bruce Momjian <bruce@momjian.us>, PostgreSQL-development <pgsql-hackers@postgresql.org>
Date: 2025-09-25T07:55:57Z
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 →
-
Fix test case from 40c242830
- ee49f2cf447a 18.1 landed
-
Fix pushdown of degenerate HAVING clauses
- 40c2428307b8 18.1 landed
- 18d261409348 19 (unreleased) landed
-
Allow pushdown of HAVING clauses with grouping sets
- 67a54b9e83d3 18.0 cited
-
Mark expressions nullable by grouping sets
- f5050f795aea 18.0 cited
On Thu, Sep 25, 2025 at 11:05 AM David Rowley <dgrowleyml@gmail.com> wrote: > What's your confidence levels on the logic now being correct? 100%? > 90%? Hopeful? > > I've not personally had the time to process the patch and figure out > that this is now safe. It sounds like you have, but (with respect) I > expect you also thought that before the initial commit too. I realise > that you now have more of the picture, but how do we know the picture > is now complete? I think we all know this stuff is hard and we also > know that even the most seasoned of planner hackers don't always get > it right first time. > > What I'm now wondering is the risk to reward ratio of fixing this in > 18.1. If it happens that it's still not right for 18.1, then we need > to wait until 18.2, which is currently due Feb-26. I don't quite have > the same confidence levels as you do, but maybe I would if I took the > time to more carefully think about this. For now, my thoughts are > that it's safer to revert and try again for v19. Probably it would > make more sense to try harder for an 18.1 fix if this optimisation was > a more critical and people had been waiting on it and there was no > other way to obtain the benefits of it. But that's not quite the case > here as, in theory, someone could rewrite their query if it's safe for > their use case and end up with the same performance benefits. > > Just so it's clear, I'm not objecting to fixing for 18.1. I just want > to ensure your judgment is for the project and not for > self-preservation. I think most people will respect the decision if > you decide that you need more time to consider this and opt to revert > in v18 and fix only in master. Based on my current understanding of > the optimisation, I'd certainly be more at ease with that. > > On the other hand, if you're 100% confident, I'll step aside. Thanks for the input; that's a reasonable concern. Although I've reviewed my analysis again and didn't find any new issues, I don't think I would claim to be 100% confident that the current logic is bug-free. Realistically, I doubt anyone would make such a claim. I'm completely open to reverting this and revisiting it for v19. However, I'd really appreciate any reviews that point out specific issues in the current logic. I've shared my analysis in my initial email as well as in the commit message of the proposed patch. If anyone can highlight parts that don't make sense, we can discuss them and update the patch accordingly. Without that kind of feedback, I'm concerned that we may just end up repeating the same code in v19. - Richard