On 10/7/22 11:18, Etsuro Fujita wrote:
On Tue, Sep 27, 2022 at 6:03 PM Etsuro Fujita <etsuro.fuj...@gmail.com> wrote:
I will review the patch a bit more, but I feel that it is
in good shape.
One thing I noticed is this bit added to CopyMultiInsertBufferFlush()
to run triggers on the foreign table.
+ /* Run AFTER ROW INSERT triggers */
+ if (resultRelInfo->ri_TrigDesc != NULL &&
+ (resultRelInfo->ri_TrigDesc->trig_insert_after_row ||
+ resultRelInfo->ri_TrigDesc->trig_insert_new_table))
+ {
+ Oid relid =
RelationGetRelid(resultRelInfo->ri_RelationDesc);
+
+ for (i = 0; i < inserted; i++)
+ {
+ TupleTableSlot *slot = rslots[i];
+
+ /*
+ * AFTER ROW Triggers might reference the tableoid column,
+ * so (re-)initialize tts_tableOid before evaluating them.
+ */
+ slot->tts_tableOid = relid;
+
+ ExecARInsertTriggers(estate, resultRelInfo,
+ slot, NIL,
+ cstate->transition_capture);
+ }
+ }
Since foreign tables cannot have transition tables, we have
trig_insert_new_table=false. So I simplified the if test and added an
assertion ensuring trig_insert_new_table=false. Attached is a new
version of the patch. I tweaked some comments a bit as well. I think
the patch is committable. So I plan on committing it next week if
there are no objections.
I reviewed the patch one more time. Only one question: bistate and
ri_FdwRoutine are strongly bounded. Maybe to add some assertion on
(ri_FdwRoutine XOR bistate) ? Just to prevent possible errors in future.
--
Regards
Andrey Lepikhov
Postgres Professional
