Re: Inconsistent Behavior of GROUP BY ROLLUP in v17 vs master

From: Richard Guo <guofenglinux(at)gmail(dot)com>
To: David Rowley <dgrowleyml(at)gmail(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, 邱宇航 <iamqyh(at)gmail(dot)com>, Bruce Momjian <bruce(at)momjian(dot)us>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Inconsistent Behavior of GROUP BY ROLLUP in v17 vs master
Date: 2025-09-25 07:55:57
Message-ID: CAMbWs4-MBRte3E00wEY+gx4ifyQGWQpEeHdtZb1hPz2iAxbNew@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Sep 25, 2025 at 11:05 AM David Rowley <dgrowleyml(at)gmail(dot)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

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2025-09-25 07:59:41 Re: Remove obsolate comments from 047_checkpoint_physical_slot
Previous Message Michael Paquier 2025-09-25 07:53:29 Re: Incorrect logic in XLogNeedsFlush()