Richard Guo <guofenglinux@gmail.com> 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 converts integer
> to numeric.
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.)
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."
regards, tom lane
