Re: An inefficient query caused by unnecessary PlaceHolderVar

On Fri, Nov 22, 2024 at 5:08 AM Dmitry Dolgov <9erthalion6(at)gmail(dot)com> wrote:
> 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?

Thank you for your review.

I ended up using 'under the same lowest nulling outer join' to
keep consistent with the wording used elsewhere. Please see the
updated patch attached.

BTW, since commit cb8e50a4a, we've chosen not to wrap a non-var
expression if it contains Vars/PHVs of the pulled-up subquery and does
not contain non-strict constructs. I wonder if we can apply the same
optimization from this patch to non-var expressions: for a LATERAL
subquery, if a non-var expression contains Vars/PHVs of the
lowest_nullable_relids and does not contain non-strict constructs, it
could also escape being wrapped. Any thoughts?

Thanks
Richard