Re: ERROR: no relation entry for relid 6

On Wed, May 24, 2023 at 2:48 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:

> Richard Guo <guofenglinux@gmail.com> writes:
> > Considering that clone clauses should always be outer-join clauses not
> > filter clauses, I'm wondering if we can add an additional check for that
> > in RINFO_IS_PUSHED_DOWN, something like
>
> >  #define RINFO_IS_PUSHED_DOWN(rinfo, joinrelids) \
> > -   ((rinfo)->is_pushed_down || \
> > -    !bms_is_subset((rinfo)->required_relids, joinrelids))
> > +   (!((rinfo)->has_clone || ((rinfo)->is_clone)) && \
> > +    ((rinfo)->is_pushed_down || \
> > +     !bms_is_subset((rinfo)->required_relids, joinrelids)))
>
> I don't like that one bit; it seems way too likely to introduce
> bad side-effects elsewhere.


Agreed.  I also do not have too much confidence in it.


> I think the real issue is that "is pushed down" has never been a
> conceptually accurate way of thinking about what
> remove_rel_from_query needs to do here.  Using RINFO_IS_PUSHED_DOWN
> happened to work up to now, but it's an abuse of that macro, and
> changing the macro's behavior isn't the right way to fix it.
>
> Having said that, I'm not sure what is a better way to think about it.
> It seems like our data structure doesn't have a clear tie between
> restrictinfos and their original join level, or at least, to the extent
> that it did the "clone clause" mechanism has broken it.
>
> I wonder if we could do something involving recording the
> rinfo_serial numbers of all the clauses extracted from a particular
> syntactic join level (we could keep this in a bitmapset attached
> to each SpecialJoinInfo, perhaps) and then filtering the joinclauses
> on the basis of serial numbers instead of required_relids.


I think this is a better way to fix the issue.  I went ahead and drafted
a patch as attached.  But I doubt that the collecting of rinfo_serial
numbers is thorough in the patch.  Should we also collect the
rinfo_serial of quals generated in reconsider_outer_join_clauses()?  I
believe that we do not need to consider the quals from
generate_base_implied_equalities(), since they are all supposed to be
restriction clauses not join clauses.

Thanks
Richard