From: Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
To: Michael Paquier <michael@paquier.xyz>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Cc: skaurus@gmail.com
Cc: pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #15437: Segfault during insert into declarative partitioned table with a trigger creating partition
Date: Mon, 29 Oct 2018 16:32:03 +0900
Message-ID: <751f72ac-e5d5-abef-98d8-ed1ef8c9d6cc@lab.ntt.co.jp> (raw)
In-Reply-To: <20181029065714.GI14242@paquier.xyz>
References: <15437-3fe01ee66bd1bae1@postgresql.org>
<e8079b1f-3e41-d1dc-1575-ff3cce7d76ac@lab.ntt.co.jp>
<8213.1539917571@sss.pgh.pa.us>
<ddc70376-9ddb-8637-eb69-aa7ae6f13378@lab.ntt.co.jp>
<20181029033117.GE14242@paquier.xyz>
<39fc3c42-d34e-794b-184e-4ff2481245e3@lab.ntt.co.jp>
<20181029065714.GI14242@paquier.xyz>
On 2018/10/29 15:57, Michael Paquier wrote:
> On Mon, Oct 29, 2018 at 02:28:53PM +0900, Amit Langote wrote:
>> On 2018/10/29 12:31, Michael Paquier wrote:
>>> In the case of a partition, the execution state relies on the data
>>> inserted, so the restriction sounds fine to me. If one tries to attach
>>> a partition after creating a table you actually get an error:
>>> ERROR: 55006: cannot ALTER TABLE "check_not_in_use" because it is being
>>> used by active queries in this session
>>> CONTEXT: SQL statement "alter table check_not_in_use attach partition
>>> check_not_in_use1 for values IN (1);"
>>
>> Yeah, I forgot to include this example in my previous emails.
>
> We may want to add a new test for this one in the same spirit as what
> has been sent in the previous patch.
OK, added.
>>> Other callers of CheckTableNotInUse() use command tags, which is
>>> inconsistent here, still I cannot think of anything better than
>>> CREATE TABLE "%s" PARTITION OF
>>> or:
>>> CREATE TABLE .. PARTITION OF
>>
>> Maybe the latter is fine, because we want to highlight the parent table's
>> name, not partition's.
>
> Fine by me.
Done that way. I also switched the check back to just partitioned parent
case.
Thanks for looking at this.
Regards,
Amit
Attachments:check-table-not-used-when-create-partition-of-3.patch (text/plain, 5.5K) downloaddiff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.cindex 153aec263e..3b6d672d75 100644--- a/src/backend/commands/tablecmds.c+++ b/src/backend/commands/tablecmds.c@@ -1993,6 +1993,14 @@ MergeAttributes(List *schema, List *supers, char relpersistence,
relation = heap_openrv(parent, AccessExclusiveLock);
/*
+ * Check for active uses of the parent paritioned table in the current+ * transaction, such as being used in some manner by an enclosing+ * command, like by insert's tuple routing.+ */+ if (is_partition)+ CheckTableNotInUse(relation, "CREATE TABLE .. PARTITION OF");++ /*
* We do not allow partitioned tables and partitions to participate in
* regular inheritance.
*/
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.outindex 68cd3e5676..5c94400e74 100644--- a/src/test/regress/expected/alter_table.out+++ b/src/test/regress/expected/alter_table.out@@ -3954,6 +3954,20 @@ ERROR: cannot attach a temporary relation as partition of permanent relation "p
alter table temp_part_parent attach partition temp_part_child default; -- ok
drop table perm_part_parent cascade;
drop table temp_part_parent cascade;
+-- check that adding partitions to a table while it is being used is prevented+create table check_not_in_use (a int) partition by list (a);+create or replace function add_part_func () returns trigger language plpgsql as $$+ begin+ execute 'create table check_not_in_use1 (a int)';+ execute 'alter table check_not_in_use attach partition check_not_in_use1 default';+ end $$;+create trigger add_part_trig before insert on check_not_in_use+ for each statement execute procedure add_part_func();+insert into check_not_in_use values (1);+ERROR: cannot ALTER TABLE "check_not_in_use" because it is being used by active queries in this session+CONTEXT: SQL statement "alter table check_not_in_use attach partition check_not_in_use1 default"+PL/pgSQL function add_part_func() line 4 at EXECUTE+drop table check_not_in_use;
-- test case where the partitioning operator is a SQL function whose
-- evaluation results in the table's relcache being rebuilt partway through
-- the execution of an ATTACH PARTITION command
diff --git a/src/test/regress/expected/create_table.out b/src/test/regress/expected/create_table.outindex 744b9990a6..ebdd875356 100644--- a/src/test/regress/expected/create_table.out+++ b/src/test/regress/expected/create_table.out@@ -910,3 +910,16 @@ ERROR: cannot create a temporary relation as partition of permanent relation "p
create temp table temp_part partition of temp_parted default; -- ok
drop table perm_parted cascade;
drop table temp_parted cascade;
+-- check that adding partitions to a table while it is being used is prevented+create table check_not_in_use (a int) partition by list (a);+create function add_part_func () returns trigger language plpgsql as $$+ begin+ execute 'create table check_not_in_use1 partition of check_not_in_use default';+ end $$;+create trigger add_part_trig before insert on check_not_in_use+ for each statement execute procedure add_part_func();+insert into check_not_in_use values (1);+ERROR: cannot CREATE TABLE .. PARTITION OF "check_not_in_use" because it is being used by active queries in this session+CONTEXT: SQL statement "create table check_not_in_use1 partition of check_not_in_use default"+PL/pgSQL function add_part_func() line 3 at EXECUTE+drop table check_not_in_use;diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sqlindex 6890346637..3d6c9b4599 100644--- a/src/test/regress/sql/alter_table.sql+++ b/src/test/regress/sql/alter_table.sql@@ -2607,6 +2607,18 @@ alter table temp_part_parent attach partition temp_part_child default; -- ok
drop table perm_part_parent cascade;
drop table temp_part_parent cascade;
+-- check that adding partitions to a table while it is being used is prevented+create table check_not_in_use (a int) partition by list (a);+create or replace function add_part_func () returns trigger language plpgsql as $$+ begin+ execute 'create table check_not_in_use1 (a int)';+ execute 'alter table check_not_in_use attach partition check_not_in_use1 default';+ end $$;+create trigger add_part_trig before insert on check_not_in_use+ for each statement execute procedure add_part_func();+insert into check_not_in_use values (1);+drop table check_not_in_use;+
-- test case where the partitioning operator is a SQL function whose
-- evaluation results in the table's relcache being rebuilt partway through
-- the execution of an ATTACH PARTITION command
diff --git a/src/test/regress/sql/create_table.sql b/src/test/regress/sql/create_table.sqlindex 81fa7658b0..a34dd8f0e7 100644--- a/src/test/regress/sql/create_table.sql+++ b/src/test/regress/sql/create_table.sql@@ -730,3 +730,14 @@ create temp table temp_part partition of perm_parted default; -- error
create temp table temp_part partition of temp_parted default; -- ok
drop table perm_parted cascade;
drop table temp_parted cascade;
++-- check that adding partitions to a table while it is being used is prevented+create table check_not_in_use (a int) partition by list (a);+create function add_part_func () returns trigger language plpgsql as $$+ begin+ execute 'create table check_not_in_use1 partition of check_not_in_use default';+ end $$;+create trigger add_part_trig before insert on check_not_in_use+ for each statement execute procedure add_part_func();+insert into check_not_in_use values (1);+drop table check_not_in_use;
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-bugs@postgresql.org
Cc: Langote_Amit_f8@lab.ntt.co.jp, michael@paquier.xyz, tgl@sss.pgh.pa.us, skaurus@gmail.com, pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #15437: Segfault during insert into declarative partitioned table with a trigger creating partition
In-Reply-To: <751f72ac-e5d5-abef-98d8-ed1ef8c9d6cc@lab.ntt.co.jp>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox