Re: apply_scanjoin_target_to_paths and partitionwise join

On Wed, Dec 3, 2025 at 11:03 PM Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> It's a reasonable concern, but I think we should go ahead with the
> patch anyway and see what happens. I believe that comment emerged from
> some problems with buildfarm instability around the time that code was
> committed -- I want to say that the buildfarm turned red, Tom firmly
> informed me that some of the choices I had made were not for the best,
> and I revised things accordingly including adding that comment, but I
> might be remembering the history incorrectly. Regardless, if it turns
> out there's a problem, we'll have to do something more complicated,
> which I assume would mean either adjusting the test cases, or if that
> seems like the wrong approach, then either making a separate
> RelOptInfo, or re-adding some of the paths from the previous path
> list. But I'm reluctant to do any of that without seeing something
> actually go wrong, because who is to say that whatever I change will
> fix whatever the problem is?

Fair point. I had envisioned a separate planning step involving a new
RelOptInfo, where we would re-add paths from the scan/join RelOptInfo
after applying the target, explicitly skipping Append paths for
partitioned tables. But I admit that I am unsure if this addresses
any real problems, so the effort might not be justified. I agree that
maybe we should just go ahead with your current patch and see what
happens.

> Long term, i wonder whether we can find some overall better way to
> handle the toplevel scan/join target. Why do we even go to the trouble
> of generating paths with the wrong scan/join target first and then fix
> them all, instead of getting the scan/join target right from the
> beginning? I suspect the original answer is that there was no real
> downside because it couldn't change which path appeared cheapest and
> surgically inserting the final target seemed easier than making sure
> to have the correct target available when the paths were originally
> generated. But that was before partitionwise join, before declarative
> partitioning, before parallel query, and... I would guess probably not
> before table inheritance, but I don't know for sure. As we've added
> more features to the system, maintaining this after-the-fact scan/join
> target insertion has become more and more of a hack, and I bet the
> right solution is to figure out a way to make the target list
> available earlier.

I suspect the same. IMHO, apply_scanjoin_target_to_paths is quite
brute-force and modifies planner structures in ways we generally
should avoid (no offense intended to the original author of this
function). I agree that the correct long-term solution is to generate
the expected target from the start, which would eliminate the need for
apply_scanjoin_target_to_paths entirely.

> That seems like it would avoid this class of
> problems a lot more thoroughly, but it also seems like a bigger
> project that doesn't really make sense in the context of fixing the
> present issue.

Yes, that is definitely beyond the scope of the fix we are discussing
here.

- Richard