Re: ERROR: no relation entry for relid 6

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

> Richard Guo <guofenglinux(at)gmail(dot)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