On Thu, Oct 10, 2024 at 6:51 PM Richard Guo wrote:
> On Thu, Oct 10, 2024 at 4:06 PM Richard Guo wrote:
> > While we can fix this issue by propagating the hasGroupRTE mark from
> > the EXISTS subquery to the parent, a better fix might be to remove the
> > subquery's RTE_GROUP entry, since we have
On Thu, Oct 10, 2024 at 4:06 PM Richard Guo wrote:
> While we can fix this issue by propagating the hasGroupRTE mark from
> the EXISTS subquery to the parent, a better fix might be to remove the
> subquery's RTE_GROUP entry, since we have dropped the subquery's
> groupClause before the pull-up (se
On Thu, Oct 10, 2024 at 2:39 PM David Rowley wrote:
> create table a(a int);
> explain select * from a where exists(Select 1 from a a2 where a.a =
> a2.a group by a);
> CREATE TABLE
> server closed the connection unexpectedly
>
> TRAP: failed Assert("parse->hasGroupRTE"), File:
> "../src/backend/
On Tue, 10 Sept 2024 at 16:04, Richard Guo wrote:
> I went ahead and pushed 0001 and 0002, and am now waiting for the
> upcoming bug reports.
Here's one:
create table a(a int);
explain select * from a where exists(Select 1 from a a2 where a.a =
a2.a group by a);
CREATE TABLE
server closed the c
On Wed, Sep 4, 2024 at 9:16 AM Richard Guo wrote:
> I'm seeking the possibility to push 0001 and 0002 sometime this month.
> Please let me know if anyone thinks this is unreasonable.
>
> For 0003, it might be extended to remove all no-op PHVs except those
> that are serving to isolate subexpressio
On Tue, Aug 6, 2024 at 4:17 PM Richard Guo wrote:
> I fixed this issue in v13 by performing the replacement of GROUP Vars
> after we've done with expression preprocessing on targetlist and
> havingQual. An ensuing effect of this approach is that a HAVING
> clause may contain expressions that are
On Wed, Jun 5, 2024 at 5:42 PM Richard Guo wrote:
> I found a bug in the v6 patch. The following query would trigger the
> Assert in make_restrictinfo that the given subexpression should not be
> an AND clause.
>
> select max(a) from t group by a > b and a = b having a > b and a = b;
>
> This is
I've been looking at cases where there are grouping-set keys that
reduce to Consts, and I noticed a plan with v11 patch that is not very
great.
explain (verbose, costs off)
select 1 as one group by rollup(one) order by one nulls first;
QUERY PLAN
---
Sort
Robert Haas writes:
> On Thu, Jul 4, 2024 at 5:52 PM Andres Freund wrote:
>> As-is they can't be backpatched, unless I am missing something? Afaict they
>> introduce rather thorough ABI breaks? And API breaks, actually?
> Aside from that, this looks quite invasive for back-patching, and the
> nu
On Thu, Jul 4, 2024 at 5:52 PM Andres Freund wrote:
> As-is they can't be backpatched, unless I am missing something? Afaict they
> introduce rather thorough ABI breaks? And API breaks, actually?
Aside from that, this looks quite invasive for back-patching, and the
number of bug reports so far su
On Thu, Jul 18, 2024 at 8:31 AM Richard Guo wrote:
> I am confused. Does the SQL standard explicitly define or standardize
> the behavior of grouping by volatile expressions? Does anyone know
> about that?
Just for the record, multiple instances of non-volatile grouping
expressions should alway
On Wed, Jul 17, 2024 at 8:50 AM Paul George wrote:
> > Since a subquery is a volatile expression, each of its instances
> should be evaluated separately.
I don't think this conclusion is correct. Look at:
select random(), random() from t group by random();
random | random
---
Thanks for the work!
> Since a subquery is a volatile expression, each of its instances
should be evaluated separately.
This seems like a valid point, though "query 2" below which groups over a
RANDOM() column and outputs an additional RANDOM() column a potential,
albeit contrived, counter-exampl
On Mon, Jul 15, 2024 at 8:15 AM Richard Guo wrote:
>
> We can see that with the 0001 patch, this query runs ~3 times faster,
> which is no surprise because there are 3 instances of the same
> subquery in the targetlist.
I am not sure if that's the right thing to do.
I am using a slightly elabo
On Mon, Jul 15, 2024 at 4:38 PM Sutou Kouhei wrote:
> I'm not familiar with related codes but here are my
> comments:
Thanks for reviewing this patchset!
> +* Fields valid for a GROUP RTE (else NULL/zero):
>
> There is only one field and it's LIST. So how about using
> the following?
>
>
Hi,
I'm reviewing patches in Commitfest 2024-07 from top to bottom:
https://commitfest.postgresql.org/48/
This is the 3rd patch:
https://commitfest.postgresql.org/48/4583/
FYI: https://commitfest.postgresql.org/48/4681/ is my patch.
In
"Re: Wrong results with grouping sets"
FWIW, in addition to fixing wrong result issues for queries with
grouping sets, the changes in 0001 also improve performance for
queries that have subqueries in the grouping expressions, because
different instances of the same subquery would need to be executed
only once. As a simple example, cons
On Sat, Jul 6, 2024 at 9:26 AM Richard Guo wrote:
> On Thu, Jul 4, 2024 at 6:02 PM Ashutosh Bapat
> wrote:
> > I don't have any specific thoughts on backpatching, but I have started
> > reviewing the patches.
> Thanks for reviewing this patchset!
Here is an updated version of this patchset. I'
On Sat, Jul 6, 2024 at 10:37 AM Tom Lane wrote:
> Richard Guo writes:
> > BTW, from catversion.h I read:
>
> > * Another common reason for a catversion update is a change in parsetree
> > * external representation, since serialized parsetrees appear in stored
> > * rules and new-style SQL func
Richard Guo writes:
> BTW, from catversion.h I read:
> * Another common reason for a catversion update is a change in parsetree
> * external representation, since serialized parsetrees appear in stored
> * rules and new-style SQL functions. Almost any change in primnodes.h or
> * parsenodes.
On Fri, Jul 5, 2024 at 5:51 AM Andres Freund wrote:
> On 2024-07-01 16:29:16 +0800, Richard Guo wrote:
> > Here is an updated version of this patchset. I've run pgindent for it,
> > and also tweaked the commit messages a bit.
> >
> > In principle, 0001 can be backpatched to all supported versions
On Thu, Jul 4, 2024 at 6:02 PM Ashutosh Bapat
wrote:
> On Mon, Jul 1, 2024 at 1:59 PM Richard Guo wrote:
> > Here is an updated version of this patchset. I've run pgindent for it,
> > and also tweaked the commit messages a bit.
> >
> > In principle, 0001 can be backpatched to all supported versi
On 2024-07-01 16:29:16 +0800, Richard Guo wrote:
> On Mon, Jun 10, 2024 at 5:05 PM Richard Guo wrote:
> > This patchset does not apply any more. Here is a new rebase.
>
> Here is an updated version of this patchset. I've run pgindent for it,
> and also tweaked the commit messages a bit.
>
> In
On Mon, Jul 1, 2024 at 1:59 PM Richard Guo wrote:
>
> On Mon, Jun 10, 2024 at 5:05 PM Richard Guo wrote:
> > This patchset does not apply any more. Here is a new rebase.
>
> Here is an updated version of this patchset. I've run pgindent for it,
> and also tweaked the commit messages a bit.
>
>
On Mon, Jun 10, 2024 at 5:05 PM Richard Guo wrote:
> This patchset does not apply any more. Here is a new rebase.
Here is an updated version of this patchset. I've run pgindent for it,
and also tweaked the commit messages a bit.
In principle, 0001 can be backpatched to all supported versions t
On Wed, Jun 5, 2024 at 5:42 PM Richard Guo wrote:
> Hence here is the v7 patchset. I've also added detailed commit messages
> for the two patches.
This patchset does not apply any more. Here is a new rebase.
While at it, I added more checks for 'root->group_rtindex', and also
added a new test
On Fri, May 24, 2024 at 9:08 PM Richard Guo wrote:
> On the basis of the parser infrastructure fixup, 0002 patch adds the
> nullingrel bit that references the grouping RTE to the grouping
> expressions.
I found a bug in the v6 patch. The following query would trigger the
Assert in make_restricti
On the basis of the parser infrastructure fixup, 0002 patch adds the
nullingrel bit that references the grouping RTE to the grouping
expressions.
However, it seems to me that we have to manually remove this nullingrel
bit from expressions in various cases where these expressions are
logically belo
On Fri, May 17, 2024 at 5:41 PM Richard Guo wrote:
> I've spent some more time on this patch, and now it passes all the
> regression tests. But I had to hack explain.c and ruleutils.c to make
> the varprefix stuff work as it did before, which is not great.
>
I've realized that I made a mistake
On Thu, May 16, 2024 at 5:43 PM Richard Guo wrote:
> I have experimented with this approach, and here is the outcome. The
> patch fixes Geoff's query, but it's still somewhat messy as I'm not
> experienced enough in the parser code. And the patch has not yet
> implemented the nullingrel bit man
On Sun, Jan 7, 2024 at 4:59 AM Tom Lane wrote:
> I don't think this is going in quite the right direction. We have
> many serious problems with grouping sets (latest one today at [1]),
> and I don't believe that hacking around EquivalenceClasses is going
> to fix them all.
>
> I think that what
> On 11 Jan 2024, at 20:10, vignesh C wrote:
>
> I have changed the status of the patch to "Waiting on Author" as Tom
> Lane's comments have not yet been addressed, feel free to address them
> and update the commitfest entry accordingly.
This CF entry seems to be a fix for actually unexpected
On Thu, 7 Dec 2023 at 13:52, Richard Guo wrote:
>
>
> On Mon, Sep 25, 2023 at 3:11 PM Richard Guo wrote:
>>
>> If the grouping expression is a Var or PHV, we can just set its
>> nullingrels, very straightforward. For an expression that is neither a
>> Var nor a PHV, I'm not quite sure how to se
Richard Guo writes:
> For a variable-free expression, if it contains volatile functions, SRFs,
> aggregates, or window functions, it would not be treated as a member of
> EC that is redundant (see get_eclass_for_sort_expr()). That means it
> would not be removed from the pathkeys list, so we do n
On Mon, Sep 25, 2023 at 3:11 PM Richard Guo wrote:
> If the grouping expression is a Var or PHV, we can just set its
> nullingrels, very straightforward. For an expression that is neither a
> Var nor a PHV, I'm not quite sure how to set the nullingrels. I tried
> the idea of wrapping it in a n
On Thu, Nov 16, 2023 at 11:25 PM Alena Rybakina
wrote:
> I noticed that this query worked correctly in the main branch with the
> inequality operator:
>
> postgres=# select distinct on (a, b) a, b from (values (3, 1), (2, 2)) as
> t (a, b) where a > b group by grouping sets((a, b), (a)); a | b --
Hi! Thank you for your work on the subject.
On 25.09.2023 10:11, Richard Guo wrote:
I think I've come across a wrong result issue with grouping sets, as
shown by the query below.
-- result is correct with only grouping sets
select a, b
from (values (1, 1), (2, 2)) as t (a, b) where a = b
group
On Mon, Sep 25, 2023 at 3:11 PM Richard Guo wrote:
> I think the root cause is that when we generate distinct_pathkeys, we
> failed to realize that Var 'b' might be nullable by the grouping sets,
> so it's no longer always equal to Var 'a'. It's not correct to deem
> that the PathKey for 'b' is
I think I've come across a wrong result issue with grouping sets, as
shown by the query below.
-- result is correct with only grouping sets
select a, b
from (values (1, 1), (2, 2)) as t (a, b) where a = b
group by grouping sets((a, b), (a));
a | b
---+---
1 | 1
1 |
2 | 2
2 |
(4 rows)
-- resu
39 matches
Mail list logo
