Hi,
(added Alvaro, Amit, and David)
While working on an update-tuple-routing bug in postgres_fdw [1], I
noticed this change to ExecCleanupTupleRouting() made by commit
3f2393edefa5ef2b6970a5a2fa2c7e9c55cc10cf:
+ /*
+ * Check if this result rel is one belonging to the node's subplans,
+ * if so, let ExecEndPlan() clean it up.
+ */
+ if (htab)
+ {
+ Oid partoid;
+ bool found;
+
+ partoid = RelationGetRelid(resultRelInfo->ri_RelationDesc);
+
+ (void) hash_search(htab, &partoid, HASH_FIND, &found);
+ if (found)
+ continue;
+ }
/* Allow any FDWs to shut down if they've been exercised */
- if (resultRelInfo->ri_PartitionReadyForRouting &&
- resultRelInfo->ri_FdwRoutine != NULL &&
+ if (resultRelInfo->ri_FdwRoutine != NULL &&
resultRelInfo->ri_FdwRoutine->EndForeignInsert != NULL)
resultRelInfo->ri_FdwRoutine->EndForeignInsert(mtstate->ps.state,
resultRelInfo);
This skips subplan resultrels before calling EndForeignInsert() if they
are foreign tables, which I think causes an issue: the FDWs would fail
to release resources for their foreign insert operations, because
ExecEndPlan() and ExecEndModifyTable() don't do anything to allow them
to do that. So I think we should skip subplan resultrels after
EndForeignInsert(). Attached is a small patch for that.
Best regards,
Etsuro Fujita
[1]
https://www.postgresql.org/message-id/21e7eaa4-0d4d-20c2-a1f7-c7e96f4ce440%40lab.ntt.co.jp
*** a/src/backend/executor/execPartition.c
--- b/src/backend/executor/execPartition.c
***************
*** 1126,1131 **** ExecCleanupTupleRouting(ModifyTableState *mtstate,
--- 1126,1137 ----
{
ResultRelInfo *resultRelInfo = proute->partitions[i];
+ /* Allow any FDWs to shut down */
+ if (resultRelInfo->ri_FdwRoutine != NULL &&
+ resultRelInfo->ri_FdwRoutine->EndForeignInsert != NULL)
+ resultRelInfo->ri_FdwRoutine->EndForeignInsert(mtstate->ps.state,
+ resultRelInfo);
+
/*
* Check if this result rel is one belonging to the node's subplans,
* if so, let ExecEndPlan() clean it up.
***************
*** 1142,1153 **** ExecCleanupTupleRouting(ModifyTableState *mtstate,
continue;
}
- /* Allow any FDWs to shut down if they've been exercised */
- if (resultRelInfo->ri_FdwRoutine != NULL &&
- resultRelInfo->ri_FdwRoutine->EndForeignInsert != NULL)
- resultRelInfo->ri_FdwRoutine->EndForeignInsert(mtstate->ps.state,
- resultRelInfo);
-
ExecCloseIndices(resultRelInfo);
table_close(resultRelInfo->ri_RelationDesc, NoLock);
}
--- 1148,1153 ----
