On Wed, Mar 5, 2025 at 8:01 PM Dean Rasheed <dean.a.rash...@gmail.com> wrote: > Yes, that makes sense, and it seems like a fairly straightforward > simplification, given that perform_pullup_replace_vars() sets it back > to false shortly afterwards. > > The same thing happens in pull_up_constant_function().
Thanks for looking. Attached are the patches. 0001 removes the code that sets wrap_non_vars to true for UNION ALL subqueries. And that leaves us only two cases where we need PHVs for identification purposes: 1. If the query uses grouping sets, we need a PHV for each expression of the subquery's targetlist items. 2. In the join quals of a full join, we need PHVs for variable-free expressions. In 0002, I changed the boolean wrap_non_vars to an enum, which indicates whether no expressions, all expressions, or only variable-free expressions need to be wrapped for identification purposes. Another thing is that when deciding whether to wrap the newnode in pullup_replace_vars_callback(), I initially thought that we didn't need to handle Vars/PHVs specifically, and could instead merge them into the branch for handling general expressions. However, doing so causes a plan diff in the regression tests. The reason is that a non-strict construct hidden within a lower-level PlaceHolderVar can lead the code for handling general expressions to mistakenly think that another PHV is needed, even when it isn't. Therefore, the branches for handling Vars/PHVs are retained in 0002. Thanks Richard
v1-0001-Remove-code-setting-wrap_non_vars-to-true-for-UNION-ALL-subqueries.patch
Description: Binary data
v1-0002-Fix-incorrect-handling-of-subquery-pullup.patch
Description: Binary data
