On Sun, Jan 24, 2021 at 2:17 AM Tomas Vondra <tomas.von...@enterprisedb.com> wrote: > On 1/23/21 9:31 AM, Amit Langote wrote: > > I was looking at this and it looks like we've got a problematic case > > where postgresGetForeignModifyBatchSize() is called from > > ExecInitRoutingInfo(). > > > > That case is when the insert is performed as part of a cross-partition > > update of a partitioned table containing postgres_fdw foreign table > > partitions. Because we don't check the operation in > > ExecInitRoutingInfo() when calling GetForeignModifyBatchSize(), such > > inserts attempt to use batching. However the ResultRelInfo may be one > > for the original update operation, so ri_FdwState contains a > > PgFdwModifyState with batch_size set to 0, because updates don't > > support batching. As things stand now, > > postgresGetForeignModifyBatchSize() simply returns that, tripping the > > following Assert in the caller. > > > > Assert(partRelInfo->ri_BatchSize >= 1); > > > > Use this example to see the crash: > > > > create table p (a int) partition by list (a); > > create table p1 (like p); > > create extension postgres_fdw; > > create server lb foreign data wrapper postgres_fdw ; > > create user mapping for current_user server lb; > > create foreign table fp1 (a int) server lb options (table_name 'p1'); > > alter table p attach partition fp1 for values in (1); > > create or replace function report_trig_details() returns trigger as $$ > > begin raise notice '% % on %', tg_when, tg_op, tg_relname; if tg_op = > > 'DELETE' then return old; end if; return new; end; $$ language > > plpgsql; > > create trigger trig before update on fp1 for each row execute function > > report_trig_details(); > > create table p2 partition of p for values in (2); > > insert into p values (2); > > update p set a = 1; -- crashes > > > > So we let's check mtstate->operation == CMD_INSERT in > > ExecInitRoutingInfo() to prevent calling GetForeignModifyBatchSize() > > in cross-update situations where mtstate->operation would be > > CMD_UPDATE. > > > > I've attached a patch. > > Thanks for catching this. I think it'd be good if the fix included a > regression test. The example seems like a good starting point, not sure > if it can be simplified further.
Yes, it can be simplified by using a local join to prevent the update of the foreign partition from being pushed to the remote server, for which my example in the previous email used a local trigger. Note that the update of the foreign partition to be done locally is a prerequisite for this bug to occur. I've added that simplified test case in the attached updated patch. -- Amit Langote EDB: http://www.enterprisedb.com
v2-0001-Prevent-FDW-insert-batching-during-cross-partitio.patch
Description: Binary data
