Amit: Good catch. bq. ExecInitRoutingInfo() that is in the charge of initialing
Should be 'ExecInitRoutingInfo() that is in charge of initializing' Cheers On Sat, Jan 23, 2021 at 12:31 AM Amit Langote <amitlangot...@gmail.com> wrote: > On Thu, Jan 21, 2021 at 11:36 AM Tomas Vondra > <tomas.von...@enterprisedb.com> wrote: > > On 1/21/21 3:09 AM, tsunakawa.ta...@fujitsu.com wrote: > > > From: Tomas Vondra <tomas.von...@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. > > I've attached a patch. > > > > > -- > Amit Langote > EDB: http://www.enterprisedb.com >
