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.
I've attached a patch.
--
Amit Langote
EDB: http://www.enterprisedb.com
