Amit-san, On Thu, Jul 10, 2025 at 11:54 PM Amit Langote <amitlangot...@gmail.com> wrote: > On Thu, Jul 10, 2025 at 22:20 Amit Langote <amitlangot...@gmail.com> wrote: >> On Wed, Jul 9, 2025 at 5:07 PM Etsuro Fujita <etsuro.fuj...@gmail.com> wrote: >> > 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.
Cool! >> 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. Ok, thanks for taking the time for this patch! After re-reading the patch I noticed two minor things: * The existing code in ExecAR* functions already dereferences the passed-in ResultRelInfo pointer without checking that it is not NULL. The Assert I added to those functions would be an overkill, so I removed it. Sorry for the back and forth. * I added a trigger function trigger_nothing() in the regression tests, but I noticed an existing trigger function above the tests. To make the tests a bit small, I replaced trigger_nothing() with the existing trigger function and removed trigger_nothing(). Attached is a new version of the patch. Best regards, Etsuro Fujita
fix-problem-with-transition-tables-v3.patch
Description: Binary data
