On Thu, Jul 10, 2025 at 22:20 Amit Langote <amitlangot...@gmail.com> wrote:
> Fujita-san, > > On Wed, Jul 9, 2025 at 5:07 PM Etsuro Fujita <etsuro.fuj...@gmail.com> > wrote: > > On Tue, Jul 1, 2025 at 11:55 AM Etsuro Fujita <etsuro.fuj...@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. Ah, scratch that because I missed that transition table checking is done for the “named” relation and these are checking it for child relations. - Amit >
