Fujita-san,
On Wed, Jul 9, 2025 at 5:07 PM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
> On Tue, Jul 1, 2025 at 11:55 AM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
> > So I would
> > like to propose to fix this by the following: 1) disable using direct
> > modify to modify foreign-table partitions if there are any
> > transition-table triggers on the partitioned table, and then 2) throw
> > an error in ExecARInsertTriggers()/ExecARUpdateTriggers()/ExecARDeleteTriggers()
> > if they collects transition tuple(s) from a foreign-table partition.
> >
> > Attached is a WIP patch for that.
>
> Here is an updated version of the patch, in which I added 1) an Assert
> to ExecAR* functions to ensure that the passed-in ResultRelInfo
> pointer is not NULL, 2) added/tweaked comments a bit more, and 3)
> added regression tests.
Thanks for the new patch. LGTM.
While reading it, I noticed that the functions performing table_open()
are repeatedly called in this condition, which runs for every
qualifying foreign child relations:
if (fdwroutine != NULL &&
fdwroutine->PlanDirectModify != NULL &&
fdwroutine->BeginDirectModify != NULL &&
fdwroutine->IterateDirectModify != NULL &&
fdwroutine->EndDirectModify != NULL &&
withCheckOptionLists == NIL &&
!has_row_triggers(root, rti, operation) &&
!has_stored_generated_columns(root, rti))
That seems a bit expensive. It might be worth using *_valid flags to
avoid redundant table_open() calls like you're doing for transition
table checking. Maybe something to consider in a separate patch.
--
Thanks, Amit Langote