Re: ERROR: no relation entry for relid 6

On Tue, May 23, 2023 at 2:38 PM Richard Guo <guofenglinux(at)gmail(dot)com> wrote:

> I came across $subject on master and here is the query I'm running.
>
> create table t (a int unique, b int);
>
> explain (costs off)
> select 1 from t t1
> left join t t2 on true
> inner join t t3 on true
> left join t t4 on t2.a = t4.a and t2.a = t3.a;
> ERROR: no relation entry for relid 6
>
> I looked into it and it should be caused by some problem in outer-join
> removal. After we've decided that the join to t4 is removable, which is
> no problem, one of the things we need to do is to remove any joinquals
> referencing t4 from the joininfo lists. In this query, there are two
> such quals, 't2.a = t4.a' and 't2.a = t3.a'. And both quals have two
> versions, one with t1/t2 join in their nulling bitmap and one without.
> The former version would be treated as being "pushed down" because its
> required_relids exceed the scope of joinrelids, due to the t1/t2 join
> included in the qual's nulling bitmap but not included in joinrelids.
> And as a result this version of quals would be put back. I think this
> is where the problem is. Ideally we should not put them back.
>
> This issue seems to have existed for a while, and is revealed by the
> change in c8b881d2 recently. I'm not sure how to fix it yet. What I'm
> thinking is that maybe this has something to do with the loose ends we
> have in make_outerjoininfo. Actually in this query the t1/t2 join
> cannot commute with the join to t4. If we can know that in
> make_outerjoininfo, we'd have added t1/t2 join to the min_lefthand of
> the join to t4, and that would avoid this issue.
>

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)))

This change can fix the case shown upthread. But I doubt it's the
perfect fix we want.

Thanks
Richard