On 2016/11/18 1:43, Robert Haas wrote:
> On Thu, Nov 17, 2016 at 6:27 AM, Amit Langote
> <Langote_Amit_f8@lab.ntt.co.jp> wrote:
>>> - The code in make_modifytable() to swap out the rte_array for a fake
>>> one looks like an unacceptable kludge. I don't know offhand what a
>>> better design would look like, but what you've got is really ugly.
>> Agree that it looks horrible. The problem is we don't add partition
>> (child table) RTEs when planning an insert on the parent and FDW
>> partitions can't do without some planner handling - planForeignModify()
>> expects a valid PlannerInfo for deparsing target lists (basically, to be
>> able to use planner_rt_fetch()).
> If it's only needed for foreign tables, how about for v1 we just throw
> an error and say errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
> errmsg("cannot route inserted tuples to a foreign table") for now. We
> can come back and fix it later. Doing more inheritance expansion
> Coming up with some new FDW API or some modification
> to the existing one is probably better, but I don't really want to get
> hung up on that right now.
I started working on this. I agree that the changes made in
make_modifytable would be unacceptable, but I'd vote for Amit's idea of
passing a modified PlannerInfo to PlanForeignModify so that the FDW can
do query planning for INSERT into a foreign partition in the same way as
for INSERT into a non-partition foreign table. (Though, I think we
should generate a more-valid-looking working-copy of the PlannerInfo
which has Query with the foreign partition as target.) I'm not sure
it's a good idea to add a new FDW API or modify the existing one such as
PlanForeignModify for this purpose.
Best regards,
Etsuro Fujita