Re: A gap in preventing bogus parameterizations in a798660ebe3

On 11/9/26 09:10, Richard Guo wrote:
> On Mon, Nov 9, 2026 at 4:47 AM Tomas Vondra <tomas(at)vondra(dot)me> wrote:
>> I'm now fairly confident the fix is correct. I'd still welcome someone
>> else doing a quick review, of course.
>
> I looked at the master patch, and it LGTM.
>
> One nit on the commit message: "we still reject the path, just a bit
> later" isn't quite accurate. AFAICS nothing rejects such a path
> later. It'd be added to the joinrel's pathlist, just that it can
> never be used, because its parameterization includes a relid within
> the joinrel, which the other input of an upper join can never supply.
> Maybe something like "In non-assert builds this is mostly harmless,
> since such paths can never appear in a complete plan."
>

Right. That's what I mean by "rejected" by I agree it's a bit unclear.

> This check only covers parameters that need an outer join completed at
> the current join, though. It misses the case where a parameter can
> only be supplied above some outer join that itself has to be formed
> above the current join. If such a parameter comes from a lateral
> reference, every path of the joinrel is dead, and the join has to be
> rejected in join_is_legal() instead. I'm working on that in [1]. So
> if your join generator hits "failed to build any N-way joins" with
> LATERAL, it's likely that issue rather than another hole in this
> patch.
>
> [1] https://postgr.es/m/CAMbWs48YGwbxxz-3_g0snstuVNWdkMfJwM7iwqf8NY5_uLQ23Q@mail.gmail.com
Ah, OK. I don't think I've seen such failures yes, but good to know.

Thanks!

--
Tomas Vondra