> > 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
diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c index 86b0fb3..ac21ff2 100644 --- a/src/backend/commands/copy.c +++ b/src/backend/commands/copy.c @@ -2784,17 +2784,6 @@ CopyFrom(CopyState cstate) nPartitionChanges = 0; } - /* - * Tests have shown that using multi-inserts when the - * partition changes on every tuple slightly decreases the - * performance, however, there are benefits even when only - * some batches have just 2 tuples, so let's enable - * multi-inserts even when the average is quite low. - */ - leafpart_use_multi_insert = avgTuplesPerPartChange >= 1.3 && - !has_before_insert_row_trig && - !has_instead_insert_row_trig && - resultRelInfo->ri_FdwRoutine == NULL; } else leafpart_use_multi_insert = false; @@ -2820,6 +2809,18 @@ CopyFrom(CopyState cstate) has_instead_insert_row_trig = (resultRelInfo->ri_TrigDesc && resultRelInfo->ri_TrigDesc->trig_insert_instead_row); + /* + * Tests have shown that using multi-inserts when the + * partition changes on every tuple slightly decreases the + * performance, however, there are benefits even when only + * some batches have just 2 tuples, so let's enable + * multi-inserts even when the average is quite low. + */ + if (insertMethod == CIM_MULTI_CONDITIONAL) + leafpart_use_multi_insert = avgTuplesPerPartChange >= 1.3 && + !has_before_insert_row_trig && + !has_instead_insert_row_trig && + resultRelInfo->ri_FdwRoutine == NULL; /* * We'd better make the bulk insert mechanism gets a new diff --git a/src/test/regress/input/copy.source b/src/test/regress/input/copy.source index 014b1b5..4cb03c5 100644 --- a/src/test/regress/input/copy.source +++ b/src/test/regress/input/copy.source @@ -162,4 +162,23 @@ copy parted_copytest from '@abs_builddir@/results/parted_copytest.csv'; select tableoid::regclass,count(*),sum(a) from parted_copytest group by tableoid order by tableoid::regclass::name; +truncate parted_copytest; + +-- create before insert row trigger on parted_copytest_a2 +create function part_ins_func() returns trigger language plpgsql as $$ +begin + return new; +end; +$$; + +create trigger part_ins_trig + before insert on parted_copytest_a2 + for each row + execute procedure part_ins_func(); + +copy parted_copytest from '@abs_builddir@/results/parted_copytest.csv'; + +select tableoid::regclass,count(*),sum(a) from parted_copytest +group by tableoid order by tableoid::regclass::name; + drop table parted_copytest; diff --git a/src/test/regress/output/copy.source b/src/test/regress/output/copy.source index ab09615..ddd652c 100644 --- a/src/test/regress/output/copy.source +++ b/src/test/regress/output/copy.source @@ -121,4 +121,24 @@ group by tableoid order by tableoid::regclass::name; parted_copytest_a2 | 10 | 10055 (2 rows) +truncate parted_copytest; +-- create before insert row trigger on parted_copytest_a2 +create function part_ins_func() returns trigger language plpgsql as $$ +begin + return new; +end; +$$; +create trigger part_ins_trig + before insert on parted_copytest_a2 + for each row + execute procedure part_ins_func(); +copy parted_copytest from '@abs_builddir@/results/parted_copytest.csv'; +select tableoid::regclass,count(*),sum(a) from parted_copytest +group by tableoid order by tableoid::regclass::name; + tableoid | count | sum +--------------------+-------+-------- + parted_copytest_a1 | 1010 | 510655 + parted_copytest_a2 | 10 | 10055 +(2 rows) + drop table parted_copytest;
