On Tue, May 23, 2023 at 2:38 PM Richard Guo <guofengli...@gmail.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
