> > Thanks! It looks good!
>
> Thanks for checking. I'll mark this as RfC.
Hi,
The patch cannot be applied to the latest head branch, it will be nice if you
can rebase it.
And when looking into the patch, I have some comments on it.
1)
IIRC, After the commit c5b7ba4, the initialization of
mt_partition_tuple_routing was postponed out of ExecInitModifyTable.
So, the following if-test use "proute" which is initialized at the beginning of
the ExecModifyTable() could be out of date.
And the regression test of postgres_fdw failed with the patch after the commit
c5b7ba4.
+ * If the query's main target relation is a partitioned table, any
inserts
+ * would have been performed using tuple routing, so use the appropriate
+ * set of target relations. Note that this also covers any inserts
+ * performed during cross-partition UPDATEs that would have occurred
+ * through tuple routing.
*/
if (proute)
...
It seems we should get the mt_partition_tuple_routing just before the if-test.
2)
+ foreach(lc, estate->es_opened_result_relations)
+ {
+ resultRelInfo = lfirst(lc);
+ if (resultRelInfo &&
I am not sure do we need to check if resultRelInfo == NULL because the the
existing code did not check it.
And if we need to check it, it might be better use "if (resultRelInfo == NULL
&&..."
3)
+ if (fmstate && fmstate->aux_fmstate != NULL)
+ fmstate = fmstate->aux_fmstate;
It might be better to write like " if (fmstate != NULL && fmstate->aux_fmstate
!= NULL)".
Best regards,
houzj
