On Thu, Jan 21, 2021 at 11:36 AM Tomas Vondra <tomas.vondra@enterprisedb.com> wrote: > On 1/21/21 3:09 AM, tsunakawa.takay@fujitsu.com wrote: > > From: Tomas Vondra <tomas.vondra@enterprisedb.com> > >> Right, that's pretty much what I ended up doing (without the CMD_INSERT > >> check it'd add batching info to explain for updates too, for example). > >> I'll do a bit more testing on the attached patch, but I think that's the right fix to > >> push. > > > > Thanks to the outer check for operation == CMD_INSERT, the inner one became unnecessary. > > > > Right. I've pushed the fix, hopefully buildfarm will get happy again.
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.
