Re: An inefficient query caused by unnecessary PlaceHolderVar - Mailing list pgsql-hackers

> On Fri, Jun 21, 2024 at 10:35:30AM GMT, Richard Guo wrote:
> On Mon, Jan 15, 2024 at 1:50 PM Richard Guo <guofenglinux@gmail.com> wrote:
> > Updated this patch over 29f114b6ff, which indicates that we should apply
> > the same rules for PHVs.
>
> Here is a new rebase of this patch, with some tweaks to comments.  I've
> also updated the commit message to better explain the context.
>
> To recap, this patch tries to avoid wrapping Vars and PHVs from subquery
> output that are lateral references to something outside the subquery.
> Typically this kind of wrapping is necessary when the Var/PHV references
> the non-nullable side of the outer join from the nullable side, because
> we need to ensure that it is evaluated at the right place and hence is
> forced to null when the outer join should do so.  But if the referenced
> rel is under the same lowest nulling outer join, we can actually omit
> the wrapping.  The PHVs generated from such kind of wrapping imply
> lateral dependencies and force us to resort to nestloop joins, so we'd
> better get rid of them.
>
> This patch performs this check by remembering the relids of the nullable
> side of the lowest outer join the subquery is within.  So I think it
> improves the overall plan in the related cases with very little extra
> cost.

The patch looks good to me, the implementation is concise and clear. I can't
imagine any visible overhead due to storing lowest_nullable_relids in this
context. The only nitpick I have is about this commentary:

      /*
       * Simple Vars always escape being wrapped, unless they are
       * lateral references to something outside the subquery being
    -  * pulled up.  (Even then, we could omit the PlaceHolderVar if
    -  * the referenced rel is under the same lowest outer join, but
    -  * it doesn't seem worth the trouble to check that.)
    +  * pulled up and the referenced rel is not under the same
    +  * lowest outer join.
       */

It mentions "lowest outer join", as in the original version of the text. Would
it be more precise to mention nullable side of the outer join as well?

I'm going to switch it to RFC.