Multi-insert into a partitioned table with before insert row trigger causes server crash on latest HEAD

Started by Ashutosh Sharmaalmost 8 years ago8 messageshackers
Beta feature

Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.

won't retrysuccessCI history

You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:

docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t39495
psql -h localhost -U postgres

Built from patchset v3 (message #3), July 28, 2026 at 05:09 AM.

Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:

git clone --branch t39495_3 https://github.com/hackorum-dev/postgres.git

In a checkout you already have, add the fork once:

git remote add hackorum https://github.com/hackorum-dev/postgres.git

then, for this patchset and every later one:

git fetch hackorum t39495_3 && git checkout t39495_3

Patchset v3 (message #3) is on t39495_3

Jump to latest
#1Ashutosh Sharma
ashu.coek88@gmail.com

Hi All,

The copy command on partitioned table causes server crash when before
insert row trigger is created on one of its partition. Please find the
following test-case to reproduce the crash.

-- create a partitioned table
create table part_copy_test (a int, b int, c text) partition by list (b);
create table part_copy_test_a1 partition of part_copy_test for values in(1);
create table part_copy_test_a2 partition of part_copy_test for values in(2);

-- insert enough rows to allow multi-insert into a partitioned table.
copy (select x,1,'One' from generate_series(1,1000) x
union all
select x,2,'two' from generate_series(1001,1010) x
union all
select x,1,'One' from generate_series(1011,1020) x) to
'/tmp/multi_insert_data.csv';

-- create before insert row trigger on part_copy_test_a2
create function part_ins_func() returns trigger language plpgsql as
$$
begin
return new;
end;
$$;

create trigger part_ins_trig before insert on part_copy_test_a2
for each row execute procedure part_ins_func();

-- run copy command on partitioned table.
copy part_copy_test from '/tmp/multi_insert_data.csv';

postgres=# copy part_copy_test from '/tmp/multi_insert_data.csv';
server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.
The connection to the server was lost. Attempting reset: Failed.
!>

I've spent some time looking into this issue and found that,
CopyFrom() is trying perform multi-insert for the partition that has
before insert row trigger created on it which is not expected. When a
normal table with before insert row trigger is created, CopyFrom
doesn't allow multi insert on such tables and i guess same should be
the case with table partition as well. Please correct me if i my
understanding is wrong ?

Thanks,

--
With Regards,
Ashutosh Sharma
EnterpriseDB:http://www.enterprisedb.com

#2Andres Freund
andres@anarazel.de
In reply to: Ashutosh Sharma (#1)
Re: Multi-insert into a partitioned table with before insert row trigger causes server crash on latest HEAD

Hi,

On 2018-10-12 21:16:25 +0530, Ashutosh Sharma wrote:

The copy command on partitioned table causes server crash when before
insert row trigger is created on one of its partition. Please find the
following test-case to reproduce the crash.

-- create a partitioned table
create table part_copy_test (a int, b int, c text) partition by list (b);
create table part_copy_test_a1 partition of part_copy_test for values in(1);
create table part_copy_test_a2 partition of part_copy_test for values in(2);

-- insert enough rows to allow multi-insert into a partitioned table.
copy (select x,1,'One' from generate_series(1,1000) x
union all
select x,2,'two' from generate_series(1001,1010) x
union all
select x,1,'One' from generate_series(1011,1020) x) to
'/tmp/multi_insert_data.csv';

-- create before insert row trigger on part_copy_test_a2
create function part_ins_func() returns trigger language plpgsql as
$$
begin
return new;
end;
$$;

create trigger part_ins_trig before insert on part_copy_test_a2
for each row execute procedure part_ins_func();

-- run copy command on partitioned table.
copy part_copy_test from '/tmp/multi_insert_data.csv';

postgres=# copy part_copy_test from '/tmp/multi_insert_data.csv';
server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.
The connection to the server was lost. Attempting reset: Failed.
!>

I've spent some time looking into this issue and found that,
CopyFrom() is trying perform multi-insert for the partition that has
before insert row trigger created on it which is not expected. When a
normal table with before insert row trigger is created, CopyFrom
doesn't allow multi insert on such tables and i guess same should be
the case with table partition as well. Please correct me if i my
understanding is wrong ?

Yea, that anlysis sounds right. Peter?

Greetings,

Andres Freund

#3Ashutosh Sharma
ashu.coek88@gmail.com
In reply to: Andres Freund (#2)
Re: Multi-insert into a partitioned table with before insert row trigger causes server crash on latest HEAD

Yea, that anlysis sounds right. Peter?

Thanks Andres for the confirmation on my initial analysis. I'm not
sure why Peter hasn't commented on it so far. However, I continued
investigating on it further and here are findings

I think, the root cause of this problem is that CopyFrom() is using
the stale value of *has_before_insert_row_trig* to determine if the
current partition is okay for multi-insert or not i.e.
has_before_insert_row_trig used to determine multi-insert condition
for the current partition actually belongs to old partition. I think,
*has_before_insert_row_trig* needs to updated before CopyFrom()
evaluates if the current partition is good to go for multi insert or
not. Attached is the patch based on this. I've also added the relevant
test-case for it. Peter, David, Could you please have a look into the
attached patch and share your thoughts. Thank you.

--
With Regards,
Ashutosh Sharma
EnterpriseDB:http://www.enterprisedb.com

Attachments:

t39495_3
use-updated-trigger-flag-for-multi-insert-partitioned-tab.patchtext/x-patch; charset=US-ASCII; name=use-updated-trigger-flag-for-multi-insert-partitioned-tab.patchDownload+51-11
#4Peter Eisentraut
peter_e@gmx.net
In reply to: Ashutosh Sharma (#3)
Re: Multi-insert into a partitioned table with before insert row trigger causes server crash on latest HEAD

On 16/10/2018 06:33, Ashutosh Sharma wrote:

I think, the root cause of this problem is that CopyFrom() is using
the stale value of *has_before_insert_row_trig* to determine if the
current partition is okay for multi-insert or not i.e.
has_before_insert_row_trig used to determine multi-insert condition
for the current partition actually belongs to old partition. I think,
*has_before_insert_row_trig* needs to updated before CopyFrom()
evaluates if the current partition is good to go for multi insert or
not. Attached is the patch based on this. I've also added the relevant
test-case for it. Peter, David, Could you please have a look into the
attached patch and share your thoughts. Thank you.

I have committed your fix and test, moving some code around a bit. Thanks.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#5Ashutosh Sharma
ashu.coek88@gmail.com
In reply to: Peter Eisentraut (#4)
Re: Multi-insert into a partitioned table with before insert row trigger causes server crash on latest HEAD

On Thu, Oct 18, 2018 at 12:15 AM Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:

On 16/10/2018 06:33, Ashutosh Sharma wrote:

I think, the root cause of this problem is that CopyFrom() is using
the stale value of *has_before_insert_row_trig* to determine if the
current partition is okay for multi-insert or not i.e.
has_before_insert_row_trig used to determine multi-insert condition
for the current partition actually belongs to old partition. I think,
*has_before_insert_row_trig* needs to updated before CopyFrom()
evaluates if the current partition is good to go for multi insert or
not. Attached is the patch based on this. I've also added the relevant
test-case for it. Peter, David, Could you please have a look into the
attached patch and share your thoughts. Thank you.

I have committed your fix and test, moving some code around a bit. Thanks.

Thanks Peter.

--
With Regards,
Ashutosh Sharma
EnterpriseDB:http://www.enterprisedb.com

#6David Rowley
dgrowleyml@gmail.com
In reply to: Peter Eisentraut (#4)
Re: Multi-insert into a partitioned table with before insert row trigger causes server crash on latest HEAD

(Returns from leave and beyond the reaches of the internet)

On 18 October 2018 at 07:45, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:

On 16/10/2018 06:33, Ashutosh Sharma wrote:

I think, the root cause of this problem is that CopyFrom() is using
the stale value of *has_before_insert_row_trig* to determine if the
current partition is okay for multi-insert or not i.e.
has_before_insert_row_trig used to determine multi-insert condition
for the current partition actually belongs to old partition. I think,
*has_before_insert_row_trig* needs to updated before CopyFrom()
evaluates if the current partition is good to go for multi insert or
not. Attached is the patch based on this. I've also added the relevant
test-case for it. Peter, David, Could you please have a look into the
attached patch and share your thoughts. Thank you.

I have committed your fix and test, moving some code around a bit. Thanks.

Thanks for pushing that fix.

Originally my patch in [1]/messages/by-id/CAKJS1f9f8yuj04X_rffNu2JPbvhy+YP_aVH6iwCTJ1OL=YwCOg@mail.gmail.com only could set leafpart_use_multi_insert to
true within the `if (insertMethod == CIM_MULTI_CONDITIONAL)` test, so
wouldn't have suffered from this problem.

I'm not sure that doubling up the `insertMethod ==
CIM_MULTI_CONDITIONAL` test is the cleanest fix. Personally, I liked
the way it was in the v6 edition of the patch, but I'm used to getting
outvoted.

[1]: /messages/by-id/CAKJS1f9f8yuj04X_rffNu2JPbvhy+YP_aVH6iwCTJ1OL=YwCOg@mail.gmail.com

--
David Rowley http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

#7Ashutosh Sharma
ashu.coek88@gmail.com
In reply to: David Rowley (#6)
Re: Multi-insert into a partitioned table with before insert row trigger causes server crash on latest HEAD

Thanks for pushing that fix.

Originally my patch in [1] only could set leafpart_use_multi_insert to
true within the `if (insertMethod == CIM_MULTI_CONDITIONAL)` test, so
wouldn't have suffered from this problem.

The problem is reproducible even with the patch in [1]/messages/by-id/CAKJS1f9f8yuj04X_rffNu2JPbvhy+YP_aVH6iwCTJ1OL=YwCOg@mail.gmail.com combined with
the v6 edition of delta patch. Actually this problem has nothing to do
with where we set leafpart_use_multi_insert flag. It's about using the
right values (updated values) to decide if the current partition is
good to go for multi-insert or not. Like i mentioned earlier reply,
we actually came across this problem because we have used outdated
value for before insert row trigger to evaluate
leafpart_use_multi_insert flag.

[1]: /messages/by-id/CAKJS1f9f8yuj04X_rffNu2JPbvhy+YP_aVH6iwCTJ1OL=YwCOg@mail.gmail.com

--
With Regards,
Ashutosh Sharma
EnterpriseDB:http://www.enterprisedb.com

#8David Rowley
dgrowleyml@gmail.com
In reply to: Ashutosh Sharma (#7)
Re: Multi-insert into a partitioned table with before insert row trigger causes server crash on latest HEAD

On 21 October 2018 at 04:03, Ashutosh Sharma <ashu.coek88@gmail.com> wrote:

Thanks for pushing that fix.

Originally my patch in [1] only could set leafpart_use_multi_insert to
true within the `if (insertMethod == CIM_MULTI_CONDITIONAL)` test, so
wouldn't have suffered from this problem.

The problem is reproducible even with the patch in [1] combined with
the v6 edition of delta patch. Actually this problem has nothing to do
with where we set leafpart_use_multi_insert flag. It's about using the
right values (updated values) to decide if the current partition is
good to go for multi-insert or not. Like i mentioned earlier reply,
we actually came across this problem because we have used outdated
value for before insert row trigger to evaluate
leafpart_use_multi_insert flag.

[1] /messages/by-id/CAKJS1f9f8yuj04X_rffNu2JPbvhy+YP_aVH6iwCTJ1OL=YwCOg@mail.gmail.com

Oops. I was completely wrong about all of what I said above. Seems my
analysis was rushed and very incorrect.

Thanks for writing the patch Ashutosh and thanks Peter for committing
the modified version of it.

--
David Rowley http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services