Re: BUG #15613: Bug in PG Planner for Foreign Data Wrappers

Etsuro Fujita <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp> writes:
>>> We could either split the function into two or three functions, or add
>>> still more overhead to it to notice what kind of relation has been
>>> passed and adjust its behavior for that. I'm not really thrilled with
>>> the latter: the fact that it's called create_foreignSCAN_path means,
>>> to me, that it's not supposed to be used for anything but baserel
>>> cases.

>> I don't have any strong opinion on that.

> On second thoughts, I think it would be a good idea to split that
> function, because we can minimize the parameters list passed to each
> function, making it easy to call that function; as you mentioned,
> 'required_outer' isn't required for upperrels, and 'fdw_outerpath' isn't
> required for baserels and upperrels. Not sure we should do that for PG12.

Yeah, I agree. Attached is a draft of that. I've not thought very hard
about how we would want to manage parameterization of foreign joins, so
for now create_foreign_join_path doesn't support that. We'll have to
change its API whenever we do want to support that, which is a good reason
why it should be separate from create_foreignscan_path: that way we don't
break API for FDWs that are only implementing plain scans.

I propose that we apply this or something much like it to HEAD, and leave
the problem of actually working out parameterized foreign joins for later.

Not sure whether we should do the same in the back branches. It might be
fine to decide that we're never going to support parameterized foreign
joins in the back branches, in which case I think just adding the Assert
to create_foreignscan_path would be enough there.

regards, tom lane