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.