Re: BUG #18953: Planner fails to build plan for complex query with LATERAL references

Richard Guo <guofenglinux@gmail.com> writes:
> 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.

I don't love this solution, because it's not clear that it'd avoid
a failure if there *isn't* an equivalent PHV already there.
Also it seems like it's piling another level of band-aid on what's
already a mess.

I experimented with making identify_current_nestloop_params compute
the correct nullingrels "from scratch", and that seems to resolve
the problem.  Now this is arguably not an ideal answer, because the
point of the setrefs.c cross-checks is to verify that we placed
Vars and PHVs at safe places to calculate correctly-nulled values,
and we're basically defeating those checks.  But this feels like
the least crufty answer, and it's also confining the cruft to the
right place: if we can think of a more principled answer for
computing the nestloop params' nullingrels, we just have to fix
identify_current_nestloop_params to do that instead.

BTW, having done this it seems like we could get rid of the use
of NRM_SUBSET and instead use NRM_EQUAL for processing nestloop
params in setrefs.c.  I tried that, and it gets through check-world,
but I'm too chicken to risk such a change in v18 at this stage.
I propose applying the attached for now and then removing NRM_SUBSET
when v19 opens for business.

For ease of review, 0001 attached is the same patch as before and
then 0002 is the new stuff.  I'd squash them to one patch for
commit, though.

			regards, tom lane