Re: Inconsistent Behavior of GROUP BY ROLLUP in v17 vs master - Mailing list pgsql-hackers

From Richard Guo
Subject Re: Inconsistent Behavior of GROUP BY ROLLUP in v17 vs master
Date
Msg-id CAMbWs4-MBRte3E00wEY+gx4ifyQGWQpEeHdtZb1hPz2iAxbNew@mail.gmail.com
Whole thread Raw
In response to Re: Inconsistent Behavior of GROUP BY ROLLUP in v17 vs master  (David Rowley <dgrowleyml@gmail.com>)
List pgsql-hackers
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



pgsql-hackers by date:

Previous
From: Michael Paquier
Date:
Subject: Re: Incorrect logic in XLogNeedsFlush()
Next
From: Michael Paquier
Date:
Subject: Re: Remove obsolate comments from 047_checkpoint_physical_slot