Re: An inefficient query caused by unnecessary PlaceHolderVar

On Wed, May 31, 2023 at 1:27 AM James Coleman <jtc331@gmail.com> wrote:

> This looks good to me.


Thanks for the review!


> A few small tweaks suggested to comment wording:
>
> +-- lateral reference for simple Var can escape PlaceHolderVar if the
> +-- referenced rel is under the same lowest nulling outer join
> +explain (verbose, costs off)
>
> I think this is clearer: "lateral references to simple Vars do not
> need a PlaceHolderVar when the referenced rel is part of the same
> lowest nulling outer join"?


Thanks for the suggestion!  How about we go with "lateral references to
simple Vars do not need a PlaceHolderVar when the referenced rel is
under the same lowest nulling outer join"?  This seems a little more
consistent with the comment in prepjointree.c.


>                   * 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.  Even then, we could omit the
> PlaceHolderVar if
> +                 * the referenced rel is under the same lowest nulling
> outer
> +                 * join.
>
> I think this is clearer: "references something outside the subquery
> being pulled up and is not under the same lowest outer join."


Agreed.  Will use this one.


> One other thing: it would be helpful to have the test query output be
> stable between HEAD and this patch; perhaps add:
>
> order by 1, 2, 3, 4, 5, 6, 7
>
> to ensure stability?


Thanks for the suggestion!  I wondered about that too but I'm a bit
confused about whether we should add ORDER BY in test case.  I checked
'sql/join.sql' and found that some queries are using ORDER BY but some
are not.  Not sure what the criteria are.

Thanks
Richard