Re: Fix a wrong comment in setrefs.c

On Tue, Sep 26, 2023 at 5:45 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> 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 and determined that the problematic
> ressortgroupref values are being assigned during prepunion.c,
> which says
>
> * By convention, all non-resjunk columns in a setop tree have
> * ressortgroupref equal to their resno. In some cases the ref
> isn't
> * needed, but this is a cleaner way than modifying the tlist
> later.
>
> So at the end of that, we can have Vars in the upper relations'
> targetlists that are associated by ressortgroupref with values
> in the setop input relations' tlists, but don't match.
> (You are right that added-on implicit coercions are one reason for
> the expressions to be different, but it's not the only one.)

Ah. Thanks for the investigation.

> 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 resno
> without regard to whether that matches the topmost level's
> sortgrouprefs and without regard to whether any implicit coercions
> are added in the setop tree. We might have to clean that up someday;
> but for now, just ignore any false matches."

+1. It explains the situation much more clearly and accurately.

Thanks
Richard