Re: BUG #18953: Planner fails to build plan for complex query with LATERAL references - Mailing list pgsql-bugs

On Fri, Jun 27, 2025 at 3:37 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Alexander Lakhin <exclusion@gmail.com> writes:
> > I've managed to discover one more anomaly introduced by a16ef313f, that is
> > not fixed by fix-bug-18953-some-more:

> Just to note that I am studying this.  It looks to me like the
> issue is that identify_current_nestloop_params() is handing back
> a PHV with too few nullingrel bits set for the place that we want
> to put it, as is acknowledged to be possible in its comments.
> We thought we could get away with that, but in this context setrefs.c
> will complain.  I'm inclined to try to make it set the bits more
> accurately, rather than further weaken setrefs.c's cross-checks.
>
> I'm wondering again whether this isn't a case that is reachable
> before a16ef313f.  Perhaps the have_dangerous_phv check prevented
> forming plan trees that could have this issue, but it's not real
> clear why.

I can reproduce this error with the following query.

select * from t t1
    left join (select 1 as x, * from t t2) t2 on t1.i = t2.i
    left join t t3 on false
    left join t t4 on t4.i > t2.x;
ERROR:  wrong phnullingrels (b) (expected (b 3)) for PlaceHolderVar 1

There are two versions of the join clause "t4.i > t2.x" due to
outer-join identity 3: one involving PHV(t2.x) with an empty
phnullingrels, and another where PHV(t2.x) is nulled by the t1/t2
join.

When creating index paths for t4, we need to identify its join clauses
that match the index.  The join_clause_is_movable_to() check there
rejects is_clone versions, assuming that "generating one path from the
minimally-parameterized has_clone version is sufficient".  As a
result, only the version with empty phnullingrels appears in the t4's
scan_clauses (via its ParamPathInfo.ppi_clauses).  That version of PHV
eventually gets pulled into nestParams by replace_nestloop_params().

Prior to a16ef313f, this does not cause problems because we are aware
that Vars/PHVs seen in NestLoopParams may have nullingrels that are
just a subset of those in the Vars/PHVs actually available from the
outer side.  Accordingly, when fixing up NestLoopParams, we only
require a NRM_SUBSET match.

Starting from a16ef313f, create_nestloop_plan() might insert PHVs from
NestLoopParams into the outer subplan's tlist if they are not already
present there.  To check for presence, it uses equal(), which compares
phnullingrels as well.  So in this case, the PHV with empty
phnullingrels is added to the t1/t2/t3 join's tlist, even though a
semantically equivalent one already exists there with a different
phnullingrels.  This causes setrefs.c to complain when fixing up the
tlist for the t1/t2/t3 join, because it requires a NRM_SUPERSET match.

I think we should avoid adding the PHV to the outer subplan's tlist if
there is already an equivalent one there.  Ideally, we should fix this
by making the PHVs in the NestLoopParam expressions have accurate
nullingrels, but that doesn't seem like a trivial task.  As a band-aid
fix, I think we could match on phid only in create_nestloop_plan()
when checking whether the PHV is already available, like below.

-       if (tlist_member((Expr *) phv, outer_tlist))
+       foreach(lc2, outer_tlist)
+       {
+           TargetEntry *tlentry = (TargetEntry *) lfirst(lc2);
+           PlaceHolderVar *subphv = (PlaceHolderVar *) tlentry->expr;
+
+           if (IsA(subphv, PlaceHolderVar) &&
+               phv->phid == subphv->phid)
+               break;
+       }
+       if (lc2 != NULL)
            continue;           /* already available */

Thanks
Richard