Re: remove_useless_joins vs. bug #19560

On Mon, Aug 31, 2026 at 1:20 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> BTW, this comment reminds me strongly of an earlier draft I had,
> which eliminated the assertion failure by just not letting
> ChangeVarNodes recurse into outer-level PHVs, on the grounds that
> we can ignore their contents until they get replaced by Params.

+ /*
+ * If it's an outer-level PHV, do not recurse into its contents. Such
+ * a PHV must have been pushed into this query level without
+ * expression preprocessing, so it may contain Vars that we think are
+ * due to be deleted, triggering our INVALID_VAR assertions. There is
+ * no need to modify such a PHV anyway, since it will be replaced by a
+ * Param before anything very interesting happens.
+ */
+ if (phv->phlevelsup > 0)
+ return node;

(Typo: I assume you meant to return false here.)

I'm kind of nervous about this change. AFAIU, the premise that an
outer-level PHV "will be replaced by a Param before anything very
interesting happens" only holds within the subquery: there the PHV
node is replaced by a Param during expression preprocessing, before
the subquery's plan is built. But copies of its content can escape
back to the PHV's own query level before that replacement, and AFAICS
we do quite a few things with them at that level. I think
extract_lateral_references is an example of that.

I'm also not sure about skipping outer-level PHVs in
eval_const_expressions. I'm worried that that can cause us to carry
the raw expressions in the outer query somewhere.

- Richard