On Sat, Jun 14, 2025 at 5:58 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Here's a WIP patch along that line. It's unfinished in that, for
> testing purposes, I just lobotomized have_dangerous_phv() to return
> constant false rather than taking it out entirely. But of course
> we'd want to clean up all the dead code if we go this way.
In identify_current_nestloop_params(), I wonder if we should use the
join path's required-outer rels rather than the left path's as the
"outerrelids". It seems that there may be cases where the join path
is parameterized by some other rel while its left path itself is not
parameterized at all.
Regarding the test case, I wonder if we should add another test query
to specifically test the changes in create_nestloop_plan(). In
particular, a case where the inner rel A has a parameter that is a
PHV, and that PHV's ph_eval_at includes the outer rel B and some third
rel C. The expected plan should show the PHV being added to B's
target list (such a case might be tricky to construct though).
> I have mixed feelings about whether to back-patch or just make
> this change in HEAD. While we're clearly fixing a bug here,
> the bug's been there for 10 years, so the lack of field reports
> suggests strongly that this is not something ordinary users write.
> Two arguments against back-patching are that
> (1) the odds of introducing a new bug aren't zero;
> (2) removing the have_dangerous_phv() restriction will change
> some plan choices, which we generally dislike doing in stable
> branches.
Agreed. The changes in this patch seem too risky to backport to the
stable branches.
Thanks
Richard
