On Tue, Oct 21, 2025 at 1:18 PM Tom Lane wrote:
> Richard Guo writes:
> > Oops, I made a mistake in the test case for v18. Fixing it now…
> Bleah. I pretty much don't ever commit things into back branches
> without running regression tests there as well as in master.
> Postgres is a moving tar
Richard Guo writes:
> Oops, I made a mistake in the test case for v18. Fixing it now…
Bleah. I pretty much don't ever commit things into back branches
without running regression tests there as well as in master.
Postgres is a moving target.
regards, tom lane
On Tue, Oct 21, 2025 at 12:58 PM Richard Guo wrote:
> Cool! I've pushed and back-patched v5. Thanks for working on this
> patch.
Oops, I made a mistake in the test case for v18. Fixing it now…
- Richard
On Tue, Oct 21, 2025 at 12:26 PM Tom Lane wrote:
> Richard Guo writes:
> > Regarding the tests, I think we could add another test query to cover
> > the case with no empty grouping sets and degenerate HAVING clauses.
> > This way, all cases for the HAVING pushdown optimization with grouping
> > s
Richard Guo writes:
> Regarding the tests, I think we could add another test query to cover
> the case with no empty grouping sets and degenerate HAVING clauses.
> This way, all cases for the HAVING pushdown optimization with grouping
> sets should be covered.
> I've added such a test in v5, alon
On Tue, Oct 21, 2025 at 1:27 AM Tom Lane wrote:
> I wrote:
> > I like the concept here, but not so much the details. Pulling
> > expand_grouping_sets out of preprocess_grouping_sets feels weird.
> > I guess it's all right given that preprocess_grouping_sets doesn't
> > manipulate the parse tree o
I wrote:
> I like the concept here, but not so much the details. Pulling
> expand_grouping_sets out of preprocess_grouping_sets feels weird.
> I guess it's all right given that preprocess_grouping_sets doesn't
> manipulate the parse tree otherwise, but you didn't update the comment
> for preproces
Richard Guo writes:
> On Sat, Oct 18, 2025 at 6:14 AM Tom Lane wrote:
>>> The proposed patch tries to close the hole by checking whether
>>> the condition is degenerate, but that feels subtly wrong to me:
>>> what we ought to check is whether there is any empty grouping set.
> v1 patch tries to
On Sat, Oct 18, 2025 at 6:14 AM Tom Lane wrote:
> I wrote:
> > The proposed patch tries to close the hole by checking whether
> > the condition is degenerate, but that feels subtly wrong to me:
> > what we ought to check is whether there is any empty grouping set.
> > As proposed, I think we miss
David Rowley writes:
> On Thu, 25 Sept 2025 at 13:01, Richard Guo wrote:
>> I plan to push this patch soon, unless there are any objections.
> What's your confidence levels on the logic now being correct? 100%?
> 90%? Hopeful?
FWIW, my confidence in it is rather low. I've not had time to think
On Tue, Sep 23, 2025 at 07:38:14PM +1200, David Rowley wrote:
> On Tue, 23 Sept 2025 at 15:49, 邱宇航 wrote:
> > I've noticed that two GROUP BY ROLLUP queries behave differently in v17
> > compared to master and REL_18_STABLE. The issue can be reproduced by
> > following SQL:
> >
> > After some git b
On Wed, Sep 24, 2025 at 12:10 PM Richard Guo wrote:
> In summary, the problem occurs when both of the following conditions
> are met: 1) there are both nonempty and empty grouping sets, 2) there
> are variable-free HAVING clauses.
>
> I think the following change fixes this problem.
>
> foreac
On Wed, 24 Sept 2025 at 02:39, Tom Lane wrote:
> Is there anything we can salvage from 67a54b9e, or should
> we just revert it?
It doesn't seem great that we need to reconsider the safety of that
optimisation post-release. It's not as if 67a54b9e added several cases
to test for and got one of the
On Thu, Sep 25, 2025 at 12:24 PM Tom Lane wrote:
> FWIW, my confidence in it is rather low. I've not had time to think
> this through carefully, but it seems to me that the test ought to
> involve whether there is an empty grouping set, yet the proposed
> patch does no such thing --- or at least,
On Wed, Sep 24, 2025 at 5:18 PM Richard Guo wrote:
> Here is the patch.
I plan to push this patch soon, unless there are any objections.
- Richard
David Rowley writes:
> As for fixing it; testing for a Const-false havingClause can't be done
> as that only works for this case which const-folding happens to figure
> out during planning. You could equally have something Var-less like:
Yeah, I don't think the havingClause being constant-false i
I wrote:
> The proposed patch tries to close the hole by checking whether
> the condition is degenerate, but that feels subtly wrong to me:
> what we ought to check is whether there is any empty grouping set.
> As proposed, I think we miss optimization opportunities for
> degenerate HAVING because
I wrote:
> I started to look at this again, and now I'm thinking that there is
> indeed an issue related to "Query 1".
Oh, scratch that, I see my mistake: I was thinking of "1" as a
constant, but actually we must be interpreting it per SQL92 rules
as a reference to output column 1. So that's why
Richard Guo writes:
> Having heard nothing but crickets and not wanting to leave this until
> the 11th hour before November, I'll plan to push the v1 patch next
> week, unless there are any objections.
OK, I finally made some time to think this through, and here's
my thoughts:
The definition of
Richard Guo writes:
> Having heard nothing but crickets and not wanting to leave this until
> the 11th hour before November, I'll plan to push the v1 patch next
> week, unless there are any objections.
I started to look at this again, and now I'm thinking that there is
indeed an issue related to
On Thu, Sep 25, 2025 at 5:27 PM Richard Guo wrote:
> On Thu, Sep 25, 2025 at 12:24 PM Tom Lane wrote:
> > 18.1 will not be coming out till November, so I feel no need to
> > rush to judgment on what to do here.
> Thanks. I'll wait for any feedback on the patch itself before
> deciding how to pr
=?utf-8?B?6YKx5a6H6Iiq?= writes:
> And what about the query 2. This is caused by another commit, and
> it's not mentioned in the commit message or the mailing discussion.
That one indeed seems quite broken. EXPLAIN confirms that it's
pushing the HAVING below the aggregation, which is simply wron
On Thu, Sep 25, 2025 at 11:05 AM David Rowley 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 al
On Thu, 25 Sept 2025 at 13:01, Richard Guo wrote:
> I plan to push this patch soon, unless there are any objections.
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 so
Hi Richard
> I think the following change fixes this problem.
>
> foreach(l, (List *) parse->havingQual)
> {
> Node *havingclause = (Node *) lfirst(l);
> + Relids having_relids;
>
> if (contain_agg_clause(havingclause) ||
> contain_volatile_funct
On Wed, Sep 24, 2025 at 10:23 AM Tom Lane wrote:
> Yeah, I don't think the havingClause being constant-false is the
> key point here. I've not looked closely at 67a54b9e, but I suspect
> that anything Var-free is potentially an issue. Or it might even
> fail for something that has Vars but is no
> 2025年9月23日 15:38，David Rowley 写道：
>
> If you check the release notes and the commit message for f5050f795
> you'll see that it does mention that wrong results could be returned.
>
> What wasn't mentioned was that this wasn't fixed in prior versions.
> The reason being is that the fix required
On Tue, 23 Sept 2025 at 15:49, 邱宇航 wrote:
> I've noticed that two GROUP BY ROLLUP queries behave differently in v17
> compared to master and REL_18_STABLE. The issue can be reproduced by
> following SQL:
>
> After some git bisect work, I traced the root cause:
> - The first issue was introduced by
I've noticed that two GROUP BY ROLLUP queries behave differently in v17
compared to master and REL_18_STABLE. The issue can be reproduced by
following SQL:
``` SQL
CREATE TABLE t(id int);
INSERT INTO t SELECT generate_series(1, 3);
-- Query 1
SELECT DISTINCT 'XXX'
FROM t
GROUP BY ROLLUP (id, 1);
29 matches
Mail list logo
