From: Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
To: 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: Fri, 19 Oct 2018 12:45:33 +0900
Message-ID: <ddc70376-9ddb-8637-eb69-aa7ae6f13378@lab.ntt.co.jp> (raw)
In-Reply-To: <8213.1539917571@sss.pgh.pa.us>
References: <15437-3fe01ee66bd1bae1@postgresql.org>
<e8079b1f-3e41-d1dc-1575-ff3cce7d76ac@lab.ntt.co.jp>
<8213.1539917571@sss.pgh.pa.us>
On 2018/10/19 11:52, Tom Lane wrote:
> Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> writes:
>> On 2018/10/18 20:57, PG Bug reporting form wrote:
>>> I tried to use declarative partitioning and, to avoid creating partitions by
>>> hand, to make them in ON BEFORE STATEMENT trigger. Trigger executes
>>> successfully (proved that with RAISE NOTICE), but server crashes.
>
>> The problem here is with the server allowing to create a partition of the
>> table being inserted into, inside the table's BEFORE INSERT trigger.
>> Generally speaking, the command that's run inside the trigger shouldn't
>> have been allowed to proceed if it might change the table's properties
>> that the execution of the ongoing command is depending upon.
>
> Check.
>
>> I propose the attached to fix that.
>
> Hmm ... I wonder if we shouldn't do CheckTableNotInUse for *both* cases,
> is_partition or no?
Yeah, that would be better robustness-wise, but I couldn't think of a case
where not doing CheckTableNotInUse for the !is_partition case would be
problematic. Adding an inheritance child doesn't change the relcache
content of the parent table, but for partitioning it does. Maybe I'm
missing something though.
Attached updated patch adds the check for both cases, although I'm not
sure what the error message text added by the patch should look like. Is
the following OK?
CheckTableNotInUse(relation, "CREATE TABLE INHERITS / PARTITION OF");
Thanks,
Amit
Attachments:check-table-not-used-when-create-partition-of-2.patch (text/plain, 2.9K) downloaddiff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.cindex 3e112b4ef4..154f2efb5c 100644--- a/src/backend/commands/tablecmds.c+++ b/src/backend/commands/tablecmds.c@@ -1993,6 +1993,13 @@ MergeAttributes(List *schema, List *supers, char relpersistence,
relation = heap_openrv(parent, AccessExclusiveLock);
/*
+ * Check for active uses of the relation in the current+ * transaction, such as being used in some manner by an enclosing+ * command, like by insert's tuple routing.+ */+ CheckTableNotInUse(relation, "CREATE TABLE INHERITS / PARTITION OF");++ /*
* We do not allow partitioned tables and partitions to participate in
* regular inheritance.
*/
diff --git a/src/test/regress/expected/create_table.out b/src/test/regress/expected/create_table.outindex 744b9990a6..7020539fc1 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 INHERITS / 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/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, 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: <ddc70376-9ddb-8637-eb69-aa7ae6f13378@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