Segfault when creating partition with a primary key and sql_drop trigger exists
We're seeing a segmentation fault when creating a partition of a
partitioned table with a primary key when there is a sql_drop trigger on
Postgres 11beta4.
We discovered it because the Citus extension creates a sql_drop trigger,
but it's otherwise unrelated to the Citus extension:
https://github.com/citusdata/citus/issues/2390
To reproduce:
CREATE OR REPLACE FUNCTION on_drop()
RETURNS event_trigger AS $ondrop$
BEGIN
RAISE NOTICE 'drop_trigger';
END;
$ondrop$ LANGUAGE plpgsql;
CREATE EVENT TRIGGER fail_drop_table ON sql_drop
EXECUTE PROCEDURE on_drop();
CREATE TABLE collections_list (
key bigint,
ts timestamptz,
collection_id integer,
value numeric,
PRIMARY KEY(collection_id)
) PARTITION BY LIST ( collection_id );
CREATE TABLE collections_list_1
PARTITION OF collections_list (key, ts, collection_id, value)
FOR VALUES IN (1);
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.
Marco
On Thu, Sep 20, 2018 at 12:00:18PM +0200, Marco Slot wrote:
We're seeing a segmentation fault when creating a partition of a
partitioned table with a primary key when there is a sql_drop trigger on
Postgres 11beta4.
Thanks for reporting ; I reproduced easily so added to open items list, since
indices on partitioned talbes is a feature new in PG11.
Core was generated by `postgres: pryzbyj ts [local] CREATE TABLE '.
Program terminated with signal 11, Segmentation fault.
#0 0x000000000059d186 in EventTriggerAlterTableRelid (objectId=40108800) at event_trigger.c:1745
1745 event_trigger.c: No such file or directory.
in event_trigger.c
(gdb) bt
#0 0x000000000059d186 in EventTriggerAlterTableRelid (objectId=40108800) at event_trigger.c:1745
#1 0x00000000005dfbd3 in AlterTableInternal (relid=40108800, cmds=0x21c39a8, recurse=true) at tablecmds.c:3328
#2 0x00000000005b5b7b in DefineIndex (relationId=40108800, stmt=0x21a7350, indexRelationId=0, parentIndexId=40084714, parentConstraintId=40084715,
is_alter_table=false, check_rights=false, check_not_in_use=false, skip_build=false, quiet=false) at indexcmds.c:669
#3 0x00000000005dcfee in DefineRelation (stmt=0x2116690, relkind=114 'r', ownerId=17609, typaddress=0x0,
queryString=0x20f1e60 "CREATE TABLE collections_list_1\nPARTITION OF collections_list (key, ts, collection_id, value)\nFOR VALUES IN (1);") at tablecmds.c:946
[...]
Justin
On 2018-Sep-20, Marco Slot wrote:
We're seeing a segmentation fault when creating a partition of a
partitioned table with a primary key when there is a sql_drop trigger on
Postgres 11beta4.We discovered it because the Citus extension creates a sql_drop trigger,
but it's otherwise unrelated to the Citus extension:
https://github.com/citusdata/citus/issues/2390
Thanks for the reproducer. Will research.
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Mon, 24 Sep 2018 at 17:58, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
On 2018-Sep-20, Marco Slot wrote:
We're seeing a segmentation fault when creating a partition of a
partitioned table with a primary key when there is a sql_drop trigger on
Postgres 11beta4.We discovered it because the Citus extension creates a sql_drop trigger,
but it's otherwise unrelated to the Citus extension:
https://github.com/citusdata/citus/issues/2390Thanks for the reproducer. Will research.
Shouldn't it be fixed by adding EventTriggerAlterTableStart? Judging from the
following call of ATController, we can just pass NULL as parsetree.
Attachments:
sql_drop_on_partition.patchapplication/octet-stream; name=sql_drop_on_partition.patchDownload
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 03c0b739b3..84818bbb4e 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -3355,6 +3355,7 @@ AlterTableInternal(Oid relid, List *cmds, bool recurse)
rel = relation_open(relid, lockmode);
+ EventTriggerAlterTableStart(NULL);
EventTriggerAlterTableRelid(relid);
ATController(NULL, rel, cmds, recurse, lockmode);
On Tue, Sep 25, 2018 at 01:39:59PM +0200, Dmitry Dolgov wrote:
Shouldn't it be fixed by adding EventTriggerAlterTableStart? Judging from the
following call of ATController, we can just pass NULL as parsetree.
Hmm. I don't think that this is correct as this data could always be
used to fetch a command tag, right? It seems to me instead that we
should pass down IndexStmt and handle things like the attached.
Thoughts?
--
Michael
Attachments:
event-partition-v1.patchtext/x-diff; charset=us-asciiDownload
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index 9229f619d2..b0cb5514d3 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -50,6 +50,7 @@
#include "catalog/pg_type.h"
#include "catalog/storage.h"
#include "commands/tablecmds.h"
+#include "commands/event_trigger.h"
#include "commands/trigger.h"
#include "executor/executor.h"
#include "miscadmin.h"
@@ -212,7 +213,8 @@ relationHasPrimaryKey(Relation rel)
void
index_check_primary_key(Relation heapRel,
IndexInfo *indexInfo,
- bool is_alter_table)
+ bool is_alter_table,
+ IndexStmt *stmt)
{
List *cmds;
int i;
@@ -280,7 +282,11 @@ index_check_primary_key(Relation heapRel,
* unduly.
*/
if (cmds)
+ {
+ EventTriggerAlterTableStart((Node *) stmt);
AlterTableInternal(RelationGetRelid(heapRel), cmds, true);
+ EventTriggerAlterTableEnd();
+ }
}
/*
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index ab3d9a0a48..4fc279e86f 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -666,7 +666,7 @@ DefineIndex(Oid relationId,
* Extra checks when creating a PRIMARY KEY index.
*/
if (stmt->primary)
- index_check_primary_key(rel, indexInfo, is_alter_table);
+ index_check_primary_key(rel, indexInfo, is_alter_table, stmt);
/*
* If this table is partitioned and we're creating a unique index or a
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 1205dbc0b5..64e1059e94 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -7074,7 +7074,7 @@ ATExecAddIndexConstraint(AlteredTableInfo *tab, Relation rel,
/* Extra checks needed if making primary key */
if (stmt->primary)
- index_check_primary_key(rel, indexInfo, true);
+ index_check_primary_key(rel, indexInfo, true, stmt);
/* Note we currently don't support EXCLUSION constraints here */
if (stmt->primary)
diff --git a/src/include/catalog/index.h b/src/include/catalog/index.h
index f20c5f789b..35a29f3498 100644
--- a/src/include/catalog/index.h
+++ b/src/include/catalog/index.h
@@ -40,7 +40,8 @@ typedef enum
extern void index_check_primary_key(Relation heapRel,
IndexInfo *indexInfo,
- bool is_alter_table);
+ bool is_alter_table,
+ IndexStmt *stmt);
#define INDEX_CREATE_IS_PRIMARY (1 << 0)
#define INDEX_CREATE_ADD_CONSTRAINT (1 << 1)
On Wed, 26 Sep 2018 at 05:33, Michael Paquier <michael@paquier.xyz> wrote:
On Tue, Sep 25, 2018 at 01:39:59PM +0200, Dmitry Dolgov wrote:
Shouldn't it be fixed by adding EventTriggerAlterTableStart? Judging from the
following call of ATController, we can just pass NULL as parsetree.Hmm. I don't think that this is correct as this data could always be
used to fetch a command tag, right? It seems to me instead that we
should pass down IndexStmt and handle things like the attached.
Thoughts?
Yes, you're right. Although probably it's not great that the sequence of
EventTriggerAlterTableStart, EventTriggerAlterTableRelid and
EventTriggerAlterTableEnd is distributed between different functions (which
most likely is worth a comment in AlterTableInternal), but it's a minor
concern.
On Thu, Sep 27, 2018 at 11:02:06PM +0200, Dmitry Dolgov wrote:
Yes, you're right. Although probably it's not great that the sequence of
EventTriggerAlterTableStart, EventTriggerAlterTableRelid and
EventTriggerAlterTableEnd is distributed between different functions (which
most likely is worth a comment in AlterTableInternal), but it's a minor
concern.
I think that Alvaro should definitely look at this patch to be sure, or
I could do it, but I would need to spend way more time on this and check
event trigger interactions.
Anyway, I was struggling a bit regarding the location where adding a
regression test. event_trigger.sql makes the most sense but in tests
for drops the objects are created before the event trigger is defined,
so that would need to move around so as the original problem is
reproducible. Perhaps you have an idea for that?
--
Michael
On Fri, Sep 28, 2018 at 12:17:00PM +0900, Michael Paquier wrote:
I think that Alvaro should definitely look at this patch to be sure, or
I could do it, but I would need to spend way more time on this and check
event trigger interactions.Anyway, I was struggling a bit regarding the location where adding a
regression test. event_trigger.sql makes the most sense but in tests
for drops the objects are created before the event trigger is defined,
so that would need to move around so as the original problem is
reproducible. Perhaps you have an idea for that?
Okay. I have spent more time on this issue, and I have been able to
integrate a test in the existing event_trigger.sql which is able to
reproduce the reported failure. Attached is what I am finishing with.
I still want to do more testing on it, and the day is ending here.
Thoughts?
--
Michael
Attachments:
event-partition-v2.patchtext/x-diff; charset=us-asciiDownload
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index 9229f619d2..b0cb5514d3 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -50,6 +50,7 @@
#include "catalog/pg_type.h"
#include "catalog/storage.h"
#include "commands/tablecmds.h"
+#include "commands/event_trigger.h"
#include "commands/trigger.h"
#include "executor/executor.h"
#include "miscadmin.h"
@@ -212,7 +213,8 @@ relationHasPrimaryKey(Relation rel)
void
index_check_primary_key(Relation heapRel,
IndexInfo *indexInfo,
- bool is_alter_table)
+ bool is_alter_table,
+ IndexStmt *stmt)
{
List *cmds;
int i;
@@ -280,7 +282,11 @@ index_check_primary_key(Relation heapRel,
* unduly.
*/
if (cmds)
+ {
+ EventTriggerAlterTableStart((Node *) stmt);
AlterTableInternal(RelationGetRelid(heapRel), cmds, true);
+ EventTriggerAlterTableEnd();
+ }
}
/*
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index ab3d9a0a48..4fc279e86f 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -666,7 +666,7 @@ DefineIndex(Oid relationId,
* Extra checks when creating a PRIMARY KEY index.
*/
if (stmt->primary)
- index_check_primary_key(rel, indexInfo, is_alter_table);
+ index_check_primary_key(rel, indexInfo, is_alter_table, stmt);
/*
* If this table is partitioned and we're creating a unique index or a
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index c145385f84..27f97fdff3 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -7074,7 +7074,7 @@ ATExecAddIndexConstraint(AlteredTableInfo *tab, Relation rel,
/* Extra checks needed if making primary key */
if (stmt->primary)
- index_check_primary_key(rel, indexInfo, true);
+ index_check_primary_key(rel, indexInfo, true, stmt);
/* Note we currently don't support EXCLUSION constraints here */
if (stmt->primary)
diff --git a/src/include/catalog/index.h b/src/include/catalog/index.h
index f20c5f789b..35a29f3498 100644
--- a/src/include/catalog/index.h
+++ b/src/include/catalog/index.h
@@ -40,7 +40,8 @@ typedef enum
extern void index_check_primary_key(Relation heapRel,
IndexInfo *indexInfo,
- bool is_alter_table);
+ bool is_alter_table,
+ IndexStmt *stmt);
#define INDEX_CREATE_IS_PRIMARY (1 << 0)
#define INDEX_CREATE_ADD_CONSTRAINT (1 << 1)
diff --git a/src/test/regress/expected/event_trigger.out b/src/test/regress/expected/event_trigger.out
index 6175a10d77..15528a7cb2 100644
--- a/src/test/regress/expected/event_trigger.out
+++ b/src/test/regress/expected/event_trigger.out
@@ -349,6 +349,18 @@ CREATE SCHEMA evttrig
CREATE TABLE one (col_a SERIAL PRIMARY KEY, col_b text DEFAULT 'forty two')
CREATE INDEX one_idx ON one (col_b)
CREATE TABLE two (col_c INTEGER CHECK (col_c > 0) REFERENCES one DEFAULT 42);
+-- Partitioned tables with shared indexes
+CREATE TABLE evttrig.parted (
+ id int PRIMARY KEY)
+ PARTITION BY RANGE (id);
+CREATE TABLE evttrig.part_1_10 PARTITION OF evttrig.parted (id)
+ FOR VALUES FROM (1) TO (10);
+CREATE TABLE evttrig.part_10_20 PARTITION OF evttrig.parted (id)
+ FOR VALUES FROM (10) TO (20) PARTITION BY RANGE (id);
+CREATE TABLE evttrig.part_10_15 PARTITION OF evttrig.part_10_20 (id)
+ FOR VALUES FROM (10) TO (15);
+CREATE TABLE evttrig.part_15_20 PARTITION OF evttrig.part_10_20 (id)
+ FOR VALUES FROM (15) TO (20);
ALTER TABLE evttrig.two DROP COLUMN col_c;
NOTICE: NORMAL: orig=t normal=f istemp=f type=table column identity=evttrig.two.col_c name={evttrig,two,col_c} args={}
NOTICE: NORMAL: orig=f normal=t istemp=f type=table constraint identity=two_col_c_check on evttrig.two name={evttrig,two,two_col_c_check} args={}
@@ -359,14 +371,20 @@ NOTICE: NORMAL: orig=t normal=f istemp=f type=table constraint identity=one_pke
DROP INDEX evttrig.one_idx;
NOTICE: NORMAL: orig=t normal=f istemp=f type=index identity=evttrig.one_idx name={evttrig,one_idx} args={}
DROP SCHEMA evttrig CASCADE;
-NOTICE: drop cascades to 2 other objects
+NOTICE: drop cascades to 3 other objects
DETAIL: drop cascades to table evttrig.one
drop cascades to table evttrig.two
+drop cascades to table evttrig.parted
NOTICE: NORMAL: orig=t normal=f istemp=f type=schema identity=evttrig name={evttrig} args={}
NOTICE: NORMAL: orig=f normal=t istemp=f type=table identity=evttrig.one name={evttrig,one} args={}
NOTICE: NORMAL: orig=f normal=t istemp=f type=sequence identity=evttrig.one_col_a_seq name={evttrig,one_col_a_seq} args={}
NOTICE: NORMAL: orig=f normal=t istemp=f type=default value identity=for evttrig.one.col_a name={evttrig,one,col_a} args={}
NOTICE: NORMAL: orig=f normal=t istemp=f type=table identity=evttrig.two name={evttrig,two} args={}
+NOTICE: NORMAL: orig=f normal=t istemp=f type=table identity=evttrig.parted name={evttrig,parted} args={}
+NOTICE: NORMAL: orig=f normal=t istemp=f type=table identity=evttrig.part_1_10 name={evttrig,part_1_10} args={}
+NOTICE: NORMAL: orig=f normal=t istemp=f type=table identity=evttrig.part_10_20 name={evttrig,part_10_20} args={}
+NOTICE: NORMAL: orig=f normal=t istemp=f type=table identity=evttrig.part_10_15 name={evttrig,part_10_15} args={}
+NOTICE: NORMAL: orig=f normal=t istemp=f type=table identity=evttrig.part_15_20 name={evttrig,part_15_20} args={}
DROP TABLE a_temp_tbl;
NOTICE: NORMAL: orig=t normal=f istemp=t type=table identity=pg_temp.a_temp_tbl name={pg_temp,a_temp_tbl} args={}
DROP EVENT TRIGGER regress_event_trigger_report_dropped;
diff --git a/src/test/regress/sql/event_trigger.sql b/src/test/regress/sql/event_trigger.sql
index 342aef6449..9831d2f2db 100644
--- a/src/test/regress/sql/event_trigger.sql
+++ b/src/test/regress/sql/event_trigger.sql
@@ -274,6 +274,19 @@ CREATE SCHEMA evttrig
CREATE INDEX one_idx ON one (col_b)
CREATE TABLE two (col_c INTEGER CHECK (col_c > 0) REFERENCES one DEFAULT 42);
+-- Partitioned tables with shared indexes
+CREATE TABLE evttrig.parted (
+ id int PRIMARY KEY)
+ PARTITION BY RANGE (id);
+CREATE TABLE evttrig.part_1_10 PARTITION OF evttrig.parted (id)
+ FOR VALUES FROM (1) TO (10);
+CREATE TABLE evttrig.part_10_20 PARTITION OF evttrig.parted (id)
+ FOR VALUES FROM (10) TO (20) PARTITION BY RANGE (id);
+CREATE TABLE evttrig.part_10_15 PARTITION OF evttrig.part_10_20 (id)
+ FOR VALUES FROM (10) TO (15);
+CREATE TABLE evttrig.part_15_20 PARTITION OF evttrig.part_10_20 (id)
+ FOR VALUES FROM (15) TO (20);
+
ALTER TABLE evttrig.two DROP COLUMN col_c;
ALTER TABLE evttrig.one ALTER COLUMN col_b DROP DEFAULT;
ALTER TABLE evttrig.one DROP CONSTRAINT one_pkey;
I admit I'm surprised that your patch fixes the bug. sql_drop was added
before the command-stashing was added for pg_event_trigger_ddl_commands
was added, and sql_drop only processes objects from the list passed to
performMultipleDeletions, so adding the EventTriggerAlterTableStart() /
End() calls should not affect it ... evidently I must be missing
something here.
Still looking.
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Wed, 3 Oct 2018 at 09:53, Michael Paquier <michael@paquier.xyz> wrote:
On Fri, Sep 28, 2018 at 12:17:00PM +0900, Michael Paquier wrote:
I think that Alvaro should definitely look at this patch to be sure, or
I could do it, but I would need to spend way more time on this and check
event trigger interactions.Anyway, I was struggling a bit regarding the location where adding a
regression test. event_trigger.sql makes the most sense but in tests
for drops the objects are created before the event trigger is defined,
so that would need to move around so as the original problem is
reproducible. Perhaps you have an idea for that?Okay. I have spent more time on this issue, and I have been able to
integrate a test in the existing event_trigger.sql which is able to
reproduce the reported failure. Attached is what I am finishing with.I still want to do more testing on it, and the day is ending here.
Thoughts?
Sorry, couldn't answer your previous message, since was away. So far I don't
see any problems with your proposed patch.
On Thu, 4 Oct 2018 at 17:22, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
I admit I'm surprised that your patch fixes the bug. sql_drop was added
before the command-stashing was added for pg_event_trigger_ddl_commands
was added, and sql_drop only processes objects from the list passed to
performMultipleDeletions, so adding the EventTriggerAlterTableStart() /
End() calls should not affect it ... evidently I must be missing
something here.Still looking.
I also find strange another part of this problem, namely why we ended up doing
AlterTableInternal at all, since I assumed that after MergeAttributes all
attnotnull should be merged with OR. But for example these would work:
CREATE TABLE collections_list_1
PARTITION OF collections_list
FOR VALUES IN (1);
CREATE TABLE collections_list_1
PARTITION OF collections_list (key, ts, collection_id not null, value)
FOR VALUES IN (1);
Looks like in MergeAttributes at the branch:
if (is_partition && list_length(saved_schema) > 0)
we override not null property of ColumnDef:
if (coldef->is_from_parent)
{
coldef->is_not_null = restdef->is_not_null;
It looks a bit confusing, so I wonder if it's how it should be?
On 2018-Oct-04, Alvaro Herrera wrote:
I admit I'm surprised that your patch fixes the bug. sql_drop was added
before the command-stashing was added for pg_event_trigger_ddl_commands
was added, and sql_drop only processes objects from the list passed to
performMultipleDeletions, so adding the EventTriggerAlterTableStart() /
End() calls should not affect it ... evidently I must be missing
something here.
I think the explanation for this is that the problem has nothing to do
with sql_drop per se -- it's only that having a sql_drop trigger causes
the event trigger stuff to get invoked, and the bogus code involving
ddl_command_end (the one that's affected by the
EventTriggerAlterTableStart dance) is what crashes.
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 2018-Oct-03, Michael Paquier wrote:
Okay. I have spent more time on this issue, and I have been able to
integrate a test in the existing event_trigger.sql which is able to
reproduce the reported failure. Attached is what I am finishing with.I still want to do more testing on it, and the day is ending here.
I looked at this, and I think that this particular crash you're fixing
is just a symptom of a larger problem in the event trigger alter table
handling -- I noticed while playing a bit with
src/test/modules/test_ddl_decoding (modify sql/alter_table.sql to create
a partitioned table with nullable columns and a partition, then alter
table to add a primary key).
I'm tied up in something else at the moment so can't spend more time on
it, but I hope to have time to give it a look over the weekend.
Thanks
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Alvaro Herrera <alvherre@2ndquadrant.com> writes:
I'm tied up in something else at the moment so can't spend more time on
it, but I hope to have time to give it a look over the weekend.
Keep in mind that RC1 is scheduled to wrap Monday afternoon ...
regards, tom lane
On Thu, Oct 04, 2018 at 06:04:49PM -0400, Tom Lane wrote:
Alvaro Herrera <alvherre@2ndquadrant.com> writes:
I'm tied up in something else at the moment so can't spend more time on
it, but I hope to have time to give it a look over the weekend.Keep in mind that RC1 is scheduled to wrap Monday afternoon ...
... Which is little time to work on a complete fix. Surely we could
wait for GA for a fix? I don't find nice to release v11 with this bug.
--
Michael
Hi,
On 2018-10-05 08:29:29 +0900, Michael Paquier wrote:
On Thu, Oct 04, 2018 at 06:04:49PM -0400, Tom Lane wrote:
Alvaro Herrera <alvherre@2ndquadrant.com> writes:
I'm tied up in something else at the moment so can't spend more time on
it, but I hope to have time to give it a look over the weekend.Keep in mind that RC1 is scheduled to wrap Monday afternoon ...
... Which is little time to work on a complete fix. Surely we could
wait for GA for a fix? I don't find nice to release v11 with this bug.
Are you suggesting we fix after RC1, or delay RC1? I'm not 100% sure
I'm parsing your sentence correctly.
Greetings,
Andres Freund
On Thu, Oct 04, 2018 at 04:54:45PM -0700, Andres Freund wrote:
Are you suggesting we fix after RC1, or delay RC1? I'm not 100% sure
I'm parsing your sentence correctly.
I am suggesting to fix the issue after RC1 is released, but before GA.
--
Michael
On 10/4/18 8:34 PM, Michael Paquier wrote:
On Thu, Oct 04, 2018 at 04:54:45PM -0700, Andres Freund wrote:
Are you suggesting we fix after RC1, or delay RC1? I'm not 100% sure
I'm parsing your sentence correctly.I am suggesting to fix the issue after RC1 is released, but before GA.
That approach would mean we would require an RC2, which would further
delay the GA.
Based on our release process and various schedules, any delays to the GA
date at this point would push the release well into mid-November. Part
of the reason that we selected the current proposed date for the GA is
that we would have the .1 available 3 weeks after the release during the
regularly scheduled cumulative update.
Ideally it would be great if this was fixed for RC1. However, given the
choice of pushing the release out further vs. saving the fix for .1
which would be relatively soon, I would vote for the latter.
Jonathan
"Jonathan S. Katz" <jkatz@postgresql.org> writes:
On 10/4/18 8:34 PM, Michael Paquier wrote:
I am suggesting to fix the issue after RC1 is released, but before GA.
That approach would mean we would require an RC2, which would further
delay the GA.
Not sure about that. Alvaro seems to think there's a generic problem
in event trigger processing, which if true, was likely there pre-v11.
I don't think that patches that get back-patched further than 11
need to restart the RC clock.
Ideally it would be great if this was fixed for RC1. However, given the
choice of pushing the release out further vs. saving the fix for .1
which would be relatively soon, I would vote for the latter.
There are definitely good calendar reasons to hold to the schedule of
RC1 next week and GA the week after. I'd only want to blow up that
plan if we hit something that is both very bad and very hard to fix.
However, if we have a fix that we believe in that's available post-RC1,
I'm not sure I see why it's better to sit on it till after GA.
regards, tom lane
On 10/4/18 11:37 PM, Tom Lane wrote:
"Jonathan S. Katz" <jkatz@postgresql.org> writes:
On 10/4/18 8:34 PM, Michael Paquier wrote:
I am suggesting to fix the issue after RC1 is released, but before GA.
That approach would mean we would require an RC2, which would further
delay the GA.Not sure about that. Alvaro seems to think there's a generic problem
in event trigger processing, which if true, was likely there pre-v11.
I don't think that patches that get back-patched further than 11
need to restart the RC clock.
Well, unless we are targeting it for the release? AIUI the RCs are
should be equivalent to GA[1]https://www.postgresql.org/about/news/1783/ (and yes I see the qualifier of "should be").
Ideally it would be great if this was fixed for RC1. However, given the
choice of pushing the release out further vs. saving the fix for .1
which would be relatively soon, I would vote for the latter.There are definitely good calendar reasons to hold to the schedule of
RC1 next week and GA the week after. I'd only want to blow up that
plan if we hit something that is both very bad and very hard to fix.However, if we have a fix that we believe in that's available post-RC1,
I'm not sure I see why it's better to sit on it till after GA.
I'm not opposed; after seeing the "should be" qualifier I feel more
comfortable with the above proposal.
Jonathan
Hi,
On 2018-10-05 15:31:37 -0400, Jonathan S. Katz wrote:
On 10/4/18 11:37 PM, Tom Lane wrote:
"Jonathan S. Katz" <jkatz@postgresql.org> writes:
On 10/4/18 8:34 PM, Michael Paquier wrote:
I am suggesting to fix the issue after RC1 is released, but before GA.
That approach would mean we would require an RC2, which would further
delay the GA.Not sure about that. Alvaro seems to think there's a generic problem
in event trigger processing, which if true, was likely there pre-v11.
I don't think that patches that get back-patched further than 11
need to restart the RC clock.Well, unless we are targeting it for the release? AIUI the RCs are
should be equivalent to GA[1] (and yes I see the qualifier of "should be").
FWIW, I think that's a pretty pointless restriction. We release
bugfixes in minor releases all the time, so there's imo absolutely no
point in having a blanket restriction that a fix that we'd put in a
minor release shouldn't be slipped in between RC and GA.
Greetings,
Andres Freund
On 10/5/18 3:35 PM, Andres Freund wrote:
Hi,
On 2018-10-05 15:31:37 -0400, Jonathan S. Katz wrote:
On 10/4/18 11:37 PM, Tom Lane wrote:
"Jonathan S. Katz" <jkatz@postgresql.org> writes:
On 10/4/18 8:34 PM, Michael Paquier wrote:
I am suggesting to fix the issue after RC1 is released, but before GA.
That approach would mean we would require an RC2, which would further
delay the GA.Not sure about that. Alvaro seems to think there's a generic problem
in event trigger processing, which if true, was likely there pre-v11.
I don't think that patches that get back-patched further than 11
need to restart the RC clock.Well, unless we are targeting it for the release? AIUI the RCs are
should be equivalent to GA[1] (and yes I see the qualifier of "should be").FWIW, I think that's a pretty pointless restriction. We release
bugfixes in minor releases all the time, so there's imo absolutely no
point in having a blanket restriction that a fix that we'd put in a
minor release shouldn't be slipped in between RC and GA.
Sounds reasonable to me. Without this getting too off-thread, I'm happy
to update language on pgweb to better describe what a release candidate
is and how it can differ with GA.
Jonathan
On 2018-Oct-04, Tom Lane wrote:
"Jonathan S. Katz" <jkatz@postgresql.org> writes:
On 10/4/18 8:34 PM, Michael Paquier wrote:
I am suggesting to fix the issue after RC1 is released, but before GA.
That approach would mean we would require an RC2, which would further
delay the GA.Not sure about that. Alvaro seems to think there's a generic problem
in event trigger processing, which if true, was likely there pre-v11.
After sleeping on this, I think that a better answer is to fix the crash
per Michael's proposed patch, and fix the rest of the deparse problem
later.
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Alvaro Herrera <alvherre@2ndquadrant.com> writes:
On 2018-Oct-04, Tom Lane wrote:
Not sure about that. Alvaro seems to think there's a generic problem
in event trigger processing, which if true, was likely there pre-v11.
After sleeping on this, I think that a better answer is to fix the crash
per Michael's proposed patch, and fix the rest of the deparse problem
later.
Well, let's push it sooner not later, so we can get buildfarm coverage
before RC1.
regards, tom lane
On Fri, Oct 05, 2018 at 04:04:02PM -0400, Tom Lane wrote:
Alvaro Herrera <alvherre@2ndquadrant.com> writes:
On 2018-Oct-04, Tom Lane wrote:
Not sure about that. Alvaro seems to think there's a generic problem
in event trigger processing, which if true, was likely there pre-v11.After sleeping on this, I think that a better answer is to fix the crash
per Michael's proposed patch, and fix the rest of the deparse problem
later.Well, let's push it sooner not later, so we can get buildfarm coverage
before RC1.
I have to admit that I am not confident enough to commit this patch
myself yet as I would need to spend a couple of extra hours on it and
looking at the event triggers generated for other event types, and there
is a three-day weekend waiting ahead in Japan. Could any of you take
care of it?
--
Michael
On 2018-Oct-06, Michael Paquier wrote:
On Fri, Oct 05, 2018 at 04:04:02PM -0400, Tom Lane wrote:
Alvaro Herrera <alvherre@2ndquadrant.com> writes:
On 2018-Oct-04, Tom Lane wrote:
Not sure about that. Alvaro seems to think there's a generic problem
in event trigger processing, which if true, was likely there pre-v11.After sleeping on this, I think that a better answer is to fix the crash
per Michael's proposed patch, and fix the rest of the deparse problem
later.Well, let's push it sooner not later, so we can get buildfarm coverage
before RC1.I have to admit that I am not confident enough to commit this patch
myself yet as I would need to spend a couple of extra hours on it and
looking at the event triggers generated for other event types, and there
is a three-day weekend waiting ahead in Japan. Could any of you take
care of it?
Sure, I'll look into it tomorrow.
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
here's my proposed patch.
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:
0001-Fix-event-triggers-for-partitioned-tables.patchtext/x-diff; charset=iso-8859-1Download
From 50f049f30875fd6fa9b8b3346c9da176725f37ff Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
Date: Sat, 6 Oct 2018 12:53:01 -0300
Subject: [PATCH] Fix event triggers for partitioned tables
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Index DDL cascading on partitioned tables introduced a way for ALTER
TABLE to be called reentrantly. This caused an an important deficiency
in event trigger support to be exposed: on exiting the reentrant call,
the alter table state object was clobbered, causing a crash when the
outer alter table tries to finalize its processing. Fix the crash by
creating a stack of event trigger state objects. There are still ways
to cause things to misbehave (and probably other crashers) with more
elaborate tricks, but at least it now doesn't crash in the obvious
scenario.
Backpatch to 9.5, where DDL deparsing of event triggers was introduced.
Reported-by: Marco Slot
Authors: Michaël Paquier, Ãlvaro Herrera
Discussion: https://postgr.es/m/CANNhMLCpi+HQ7M36uPfGbJZEQLyTy7XvX=5EFkpR-b1bo0uJew@mail.gmail.com
---
src/backend/catalog/index.c | 8 +++++++-
src/backend/commands/event_trigger.c | 13 +++++++------
src/backend/commands/indexcmds.c | 3 ++-
src/backend/commands/tablecmds.c | 2 +-
src/backend/commands/view.c | 4 ++++
src/include/catalog/index.h | 3 ++-
src/include/tcop/deparse_utility.h | 3 +++
.../test_ddl_deparse/expected/alter_table.out | 12 ++++++++++++
.../modules/test_ddl_deparse/sql/alter_table.sql | 8 ++++++++
src/test/regress/expected/event_trigger.out | 20 +++++++++++++++++++-
src/test/regress/sql/event_trigger.sql | 13 +++++++++++++
11 files changed, 78 insertions(+), 11 deletions(-)
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index 39605732b1..339bb35f9b 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -50,6 +50,7 @@
#include "catalog/pg_type.h"
#include "catalog/storage.h"
#include "commands/tablecmds.h"
+#include "commands/event_trigger.h"
#include "commands/trigger.h"
#include "executor/executor.h"
#include "miscadmin.h"
@@ -212,7 +213,8 @@ relationHasPrimaryKey(Relation rel)
void
index_check_primary_key(Relation heapRel,
IndexInfo *indexInfo,
- bool is_alter_table)
+ bool is_alter_table,
+ IndexStmt *stmt)
{
List *cmds;
int i;
@@ -280,7 +282,11 @@ index_check_primary_key(Relation heapRel,
* unduly.
*/
if (cmds)
+ {
+ EventTriggerAlterTableStart((Node *) stmt);
AlterTableInternal(RelationGetRelid(heapRel), cmds, true);
+ EventTriggerAlterTableEnd();
+ }
}
/*
diff --git a/src/backend/commands/event_trigger.c b/src/backend/commands/event_trigger.c
index eecc85d14e..2c1dc47541 100644
--- a/src/backend/commands/event_trigger.c
+++ b/src/backend/commands/event_trigger.c
@@ -1696,11 +1696,6 @@ EventTriggerCollectSimpleCommand(ObjectAddress address,
* Note we don't collect the command immediately; instead we keep it in
* currentCommand, and only when we're done processing the subcommands we will
* add it to the command list.
- *
- * XXX -- this API isn't considering the possibility of an ALTER TABLE command
- * being called reentrantly by an event trigger function. Do we need stackable
- * commands at this level? Perhaps at least we should detect the condition and
- * raise an error.
*/
void
EventTriggerAlterTableStart(Node *parsetree)
@@ -1725,6 +1720,7 @@ EventTriggerAlterTableStart(Node *parsetree)
command->d.alterTable.subcmds = NIL;
command->parsetree = copyObject(parsetree);
+ command->parent = currentEventTriggerState->currentCommand;
currentEventTriggerState->currentCommand = command;
MemoryContextSwitchTo(oldcxt);
@@ -1765,6 +1761,7 @@ EventTriggerCollectAlterTableSubcmd(Node *subcmd, ObjectAddress address)
return;
Assert(IsA(subcmd, AlterTableCmd));
+ Assert(OidIsValid(currentEventTriggerState->currentCommand));
Assert(OidIsValid(currentEventTriggerState->currentCommand->d.alterTable.objectId));
oldcxt = MemoryContextSwitchTo(currentEventTriggerState->cxt);
@@ -1790,11 +1787,15 @@ EventTriggerCollectAlterTableSubcmd(Node *subcmd, ObjectAddress address)
void
EventTriggerAlterTableEnd(void)
{
+ CollectedCommand *parent;
+
/* ignore if event trigger context not set, or collection disabled */
if (!currentEventTriggerState ||
currentEventTriggerState->commandCollectionInhibited)
return;
+ parent = currentEventTriggerState->currentCommand->parent;
+
/* If no subcommands, don't collect */
if (list_length(currentEventTriggerState->currentCommand->d.alterTable.subcmds) != 0)
{
@@ -1805,7 +1806,7 @@ EventTriggerAlterTableEnd(void)
else
pfree(currentEventTriggerState->currentCommand);
- currentEventTriggerState->currentCommand = NULL;
+ currentEventTriggerState->currentCommand = parent;
}
/*
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index ab3d9a0a48..3975f62c00 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -34,6 +34,7 @@
#include "commands/comment.h"
#include "commands/dbcommands.h"
#include "commands/defrem.h"
+#include "commands/event_trigger.h"
#include "commands/tablecmds.h"
#include "commands/tablespace.h"
#include "mb/pg_wchar.h"
@@ -666,7 +667,7 @@ DefineIndex(Oid relationId,
* Extra checks when creating a PRIMARY KEY index.
*/
if (stmt->primary)
- index_check_primary_key(rel, indexInfo, is_alter_table);
+ index_check_primary_key(rel, indexInfo, is_alter_table, stmt);
/*
* If this table is partitioned and we're creating a unique index or a
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 7df1fc2a76..5d643c06d3 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -7074,7 +7074,7 @@ ATExecAddIndexConstraint(AlteredTableInfo *tab, Relation rel,
/* Extra checks needed if making primary key */
if (stmt->primary)
- index_check_primary_key(rel, indexInfo, true);
+ index_check_primary_key(rel, indexInfo, true, stmt);
/* Note we currently don't support EXCLUSION constraints here */
if (stmt->primary)
diff --git a/src/backend/commands/view.c b/src/backend/commands/view.c
index e73f1d8894..b670cad8b1 100644
--- a/src/backend/commands/view.c
+++ b/src/backend/commands/view.c
@@ -61,6 +61,8 @@ validateWithCheckOption(const char *value)
*
* Create a view relation and use the rules system to store the query
* for the view.
+ *
+ * EventTriggerAlterTableStart must have been called already.
*---------------------------------------------------------------------
*/
static ObjectAddress
@@ -186,6 +188,7 @@ DefineVirtualRelation(RangeVar *relation, List *tlist, bool replace,
atcmds = lappend(atcmds, atcmd);
}
+ /* EventTriggerAlterTableStart called by ProcessUtilitySlow */
AlterTableInternal(viewOid, atcmds, true);
/* Make the new view columns visible */
@@ -217,6 +220,7 @@ DefineVirtualRelation(RangeVar *relation, List *tlist, bool replace,
atcmd->def = (Node *) options;
atcmds = list_make1(atcmd);
+ /* EventTriggerAlterTableStart called by ProcessUtilitySlow */
AlterTableInternal(viewOid, atcmds, true);
ObjectAddressSet(address, RelationRelationId, viewOid);
diff --git a/src/include/catalog/index.h b/src/include/catalog/index.h
index f20c5f789b..35a29f3498 100644
--- a/src/include/catalog/index.h
+++ b/src/include/catalog/index.h
@@ -40,7 +40,8 @@ typedef enum
extern void index_check_primary_key(Relation heapRel,
IndexInfo *indexInfo,
- bool is_alter_table);
+ bool is_alter_table,
+ IndexStmt *stmt);
#define INDEX_CREATE_IS_PRIMARY (1 << 0)
#define INDEX_CREATE_ADD_CONSTRAINT (1 << 1)
diff --git a/src/include/tcop/deparse_utility.h b/src/include/tcop/deparse_utility.h
index 8459463391..766332f6a5 100644
--- a/src/include/tcop/deparse_utility.h
+++ b/src/include/tcop/deparse_utility.h
@@ -44,6 +44,7 @@ typedef struct CollectedATSubcmd
typedef struct CollectedCommand
{
CollectedCommandType type;
+
bool in_extension;
Node *parsetree;
@@ -100,6 +101,8 @@ typedef struct CollectedCommand
ObjectType objtype;
} defprivs;
} d;
+
+ struct CollectedCommand *parent; /* when nested */
} CollectedCommand;
#endif /* DEPARSE_UTILITY_H */
diff --git a/src/test/modules/test_ddl_deparse/expected/alter_table.out b/src/test/modules/test_ddl_deparse/expected/alter_table.out
index e304787bc5..7da847d49e 100644
--- a/src/test/modules/test_ddl_deparse/expected/alter_table.out
+++ b/src/test/modules/test_ddl_deparse/expected/alter_table.out
@@ -16,3 +16,15 @@ NOTICE: DDL test: type simple, tag ALTER TABLE
ALTER TABLE parent ADD CONSTRAINT a_pos CHECK (a > 0);
NOTICE: DDL test: type alter table, tag ALTER TABLE
NOTICE: subcommand: ADD CONSTRAINT (and recurse)
+CREATE TABLE part (
+ a int
+) PARTITION BY RANGE (a);
+NOTICE: DDL test: type simple, tag CREATE TABLE
+CREATE TABLE part1 PARTITION OF part FOR VALUES FROM (1) to (100);
+NOTICE: DDL test: type simple, tag CREATE TABLE
+ALTER TABLE part ADD PRIMARY KEY (a);
+NOTICE: DDL test: type alter table, tag CREATE INDEX
+NOTICE: subcommand: SET NOT NULL
+NOTICE: subcommand: SET NOT NULL
+NOTICE: DDL test: type alter table, tag ALTER TABLE
+NOTICE: subcommand: ADD INDEX
diff --git a/src/test/modules/test_ddl_deparse/sql/alter_table.sql b/src/test/modules/test_ddl_deparse/sql/alter_table.sql
index 6e2cca754e..dec53a0640 100644
--- a/src/test/modules/test_ddl_deparse/sql/alter_table.sql
+++ b/src/test/modules/test_ddl_deparse/sql/alter_table.sql
@@ -11,3 +11,11 @@ ALTER TABLE parent ADD COLUMN b serial;
ALTER TABLE parent RENAME COLUMN b TO c;
ALTER TABLE parent ADD CONSTRAINT a_pos CHECK (a > 0);
+
+CREATE TABLE part (
+ a int
+) PARTITION BY RANGE (a);
+
+CREATE TABLE part1 PARTITION OF part FOR VALUES FROM (1) to (100);
+
+ALTER TABLE part ADD PRIMARY KEY (a);
diff --git a/src/test/regress/expected/event_trigger.out b/src/test/regress/expected/event_trigger.out
index 6175a10d77..548f6d9a3e 100644
--- a/src/test/regress/expected/event_trigger.out
+++ b/src/test/regress/expected/event_trigger.out
@@ -349,6 +349,18 @@ CREATE SCHEMA evttrig
CREATE TABLE one (col_a SERIAL PRIMARY KEY, col_b text DEFAULT 'forty two')
CREATE INDEX one_idx ON one (col_b)
CREATE TABLE two (col_c INTEGER CHECK (col_c > 0) REFERENCES one DEFAULT 42);
+-- Partitioned tables with a partitioned index
+CREATE TABLE evttrig.parted (
+ id int PRIMARY KEY)
+ PARTITION BY RANGE (id);
+CREATE TABLE evttrig.part_1_10 PARTITION OF evttrig.parted (id)
+ FOR VALUES FROM (1) TO (10);
+CREATE TABLE evttrig.part_10_20 PARTITION OF evttrig.parted (id)
+ FOR VALUES FROM (10) TO (20) PARTITION BY RANGE (id);
+CREATE TABLE evttrig.part_10_15 PARTITION OF evttrig.part_10_20 (id)
+ FOR VALUES FROM (10) TO (15);
+CREATE TABLE evttrig.part_15_20 PARTITION OF evttrig.part_10_20 (id)
+ FOR VALUES FROM (15) TO (20);
ALTER TABLE evttrig.two DROP COLUMN col_c;
NOTICE: NORMAL: orig=t normal=f istemp=f type=table column identity=evttrig.two.col_c name={evttrig,two,col_c} args={}
NOTICE: NORMAL: orig=f normal=t istemp=f type=table constraint identity=two_col_c_check on evttrig.two name={evttrig,two,two_col_c_check} args={}
@@ -359,14 +371,20 @@ NOTICE: NORMAL: orig=t normal=f istemp=f type=table constraint identity=one_pke
DROP INDEX evttrig.one_idx;
NOTICE: NORMAL: orig=t normal=f istemp=f type=index identity=evttrig.one_idx name={evttrig,one_idx} args={}
DROP SCHEMA evttrig CASCADE;
-NOTICE: drop cascades to 2 other objects
+NOTICE: drop cascades to 3 other objects
DETAIL: drop cascades to table evttrig.one
drop cascades to table evttrig.two
+drop cascades to table evttrig.parted
NOTICE: NORMAL: orig=t normal=f istemp=f type=schema identity=evttrig name={evttrig} args={}
NOTICE: NORMAL: orig=f normal=t istemp=f type=table identity=evttrig.one name={evttrig,one} args={}
NOTICE: NORMAL: orig=f normal=t istemp=f type=sequence identity=evttrig.one_col_a_seq name={evttrig,one_col_a_seq} args={}
NOTICE: NORMAL: orig=f normal=t istemp=f type=default value identity=for evttrig.one.col_a name={evttrig,one,col_a} args={}
NOTICE: NORMAL: orig=f normal=t istemp=f type=table identity=evttrig.two name={evttrig,two} args={}
+NOTICE: NORMAL: orig=f normal=t istemp=f type=table identity=evttrig.parted name={evttrig,parted} args={}
+NOTICE: NORMAL: orig=f normal=t istemp=f type=table identity=evttrig.part_1_10 name={evttrig,part_1_10} args={}
+NOTICE: NORMAL: orig=f normal=t istemp=f type=table identity=evttrig.part_10_20 name={evttrig,part_10_20} args={}
+NOTICE: NORMAL: orig=f normal=t istemp=f type=table identity=evttrig.part_10_15 name={evttrig,part_10_15} args={}
+NOTICE: NORMAL: orig=f normal=t istemp=f type=table identity=evttrig.part_15_20 name={evttrig,part_15_20} args={}
DROP TABLE a_temp_tbl;
NOTICE: NORMAL: orig=t normal=f istemp=t type=table identity=pg_temp.a_temp_tbl name={pg_temp,a_temp_tbl} args={}
DROP EVENT TRIGGER regress_event_trigger_report_dropped;
diff --git a/src/test/regress/sql/event_trigger.sql b/src/test/regress/sql/event_trigger.sql
index 342aef6449..5220062dd4 100644
--- a/src/test/regress/sql/event_trigger.sql
+++ b/src/test/regress/sql/event_trigger.sql
@@ -274,6 +274,19 @@ CREATE SCHEMA evttrig
CREATE INDEX one_idx ON one (col_b)
CREATE TABLE two (col_c INTEGER CHECK (col_c > 0) REFERENCES one DEFAULT 42);
+-- Partitioned tables with a partitioned index
+CREATE TABLE evttrig.parted (
+ id int PRIMARY KEY)
+ PARTITION BY RANGE (id);
+CREATE TABLE evttrig.part_1_10 PARTITION OF evttrig.parted (id)
+ FOR VALUES FROM (1) TO (10);
+CREATE TABLE evttrig.part_10_20 PARTITION OF evttrig.parted (id)
+ FOR VALUES FROM (10) TO (20) PARTITION BY RANGE (id);
+CREATE TABLE evttrig.part_10_15 PARTITION OF evttrig.part_10_20 (id)
+ FOR VALUES FROM (10) TO (15);
+CREATE TABLE evttrig.part_15_20 PARTITION OF evttrig.part_10_20 (id)
+ FOR VALUES FROM (15) TO (20);
+
ALTER TABLE evttrig.two DROP COLUMN col_c;
ALTER TABLE evttrig.one ALTER COLUMN col_b DROP DEFAULT;
ALTER TABLE evttrig.one DROP CONSTRAINT one_pkey;
--
2.11.0
On 2018-Oct-06, Alvaro Herrera wrote:
here's my proposed patch.
Pushed a few hours ago.
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Sat, Oct 06, 2018 at 09:32:02PM -0300, Alvaro Herrera wrote:
On 2018-Oct-06, Alvaro Herrera wrote:
here's my proposed patch.
Pushed a few hours ago.
Thanks, Alvaro.
--
Michael
Hello,
On 10/6/18 7:50 PM, Alvaro Herrera wrote:
here's my proposed patch.
There is an incorrect assert condition within
EventTriggerCollectAlterTableSubcmd(). Maybe it should be like this?
- Assert(OidIsValid(currentEventTriggerState->currentCommand));
+ Assert(currentEventTriggerState->currentCommand);--
Arthur Zakirov
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company