Re: Fix a wrong comment in setrefs.c

On Tue, Nov 28, 2023 at 8:19 PM Heikki Linnakangas wrote: > On 03/11/2023 08:10, Richard Guo wrote: > > On Tue, Sep 26, 2023 at 9:51 AM Richard Guo > > wrote: > > On Tue, Sep 26, 2023 at 5:45 AM Tom Lane > > wrote: > > > >

Re: Fix a wrong comment in setrefs.c

On 03/11/2023 08:10, Richard Guo wrote: On Tue, Sep 26, 2023 at 9:51 AM Richard Guo > wrote: On Tue, Sep 26, 2023 at 5:45 AM Tom Lane mailto:t...@sss.pgh.pa.us>> wrote: I'm inclined to write the comment more like "Usually the equal() check is

Re: Fix a wrong comment in setrefs.c

On Tue, Sep 26, 2023 at 9:51 AM Richard Guo wrote: > On Tue, Sep 26, 2023 at 5:45 AM Tom Lane wrote: > >> I'm inclined to write the comment more like "Usually the equal() >> check is redundant, but in setop plans it may not be, since >> prepunion.c assigns ressortgroupref equal to the column

Re: Fix a wrong comment in setrefs.c

The following review has been posted through the commitfest application: make installcheck-world: not tested Implements feature: not tested Spec compliant: not tested Documentation:not tested That looks correct for me The new status of this patch is: Ready for

Re: Fix a wrong comment in setrefs.c

On Tue, Sep 26, 2023 at 5:45 AM Tom Lane wrote: > Hmm. This kind of makes me itch, because in principle a ressortgroupref > identifier should uniquely identify a sorting/grouping column. If it > fails to do so here, maybe there are outright bugs lurking elsewhere? > > I poked into it a little

Re: Fix a wrong comment in setrefs.c

Richard Guo writes: > I noticed a wrong comment in search_indexed_tlist_for_sortgroupref(). > /* The equal() check should be redundant, but let's be paranoid */ > It turns out that the equal() check is necessary, because the given > sort/group expression might be type of FuncExpr which

Fix a wrong comment in setrefs.c

I noticed a wrong comment in search_indexed_tlist_for_sortgroupref(). foreach(lc, itlist->tlist) { TargetEntry *tle = (TargetEntry *) lfirst(lc); /* The equal() check should be redundant, but let's be paranoid */ if (tle->ressortgroupref == sortgroupref &&