mirror of
https://git.postgresql.org/git/postgresql.git
synced 2024-09-01 09:37:25 +02:00
Fix event triggers for partitioned tables
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
This commit is contained in:
parent
6bf278df89
commit
1a852f7c1e
@ -50,6 +50,7 @@
|
|||||||
#include "catalog/pg_type.h"
|
#include "catalog/pg_type.h"
|
||||||
#include "catalog/storage.h"
|
#include "catalog/storage.h"
|
||||||
#include "commands/tablecmds.h"
|
#include "commands/tablecmds.h"
|
||||||
|
#include "commands/event_trigger.h"
|
||||||
#include "commands/trigger.h"
|
#include "commands/trigger.h"
|
||||||
#include "executor/executor.h"
|
#include "executor/executor.h"
|
||||||
#include "miscadmin.h"
|
#include "miscadmin.h"
|
||||||
@ -212,7 +213,8 @@ relationHasPrimaryKey(Relation rel)
|
|||||||
void
|
void
|
||||||
index_check_primary_key(Relation heapRel,
|
index_check_primary_key(Relation heapRel,
|
||||||
IndexInfo *indexInfo,
|
IndexInfo *indexInfo,
|
||||||
bool is_alter_table)
|
bool is_alter_table,
|
||||||
|
IndexStmt *stmt)
|
||||||
{
|
{
|
||||||
List *cmds;
|
List *cmds;
|
||||||
int i;
|
int i;
|
||||||
@ -280,7 +282,11 @@ index_check_primary_key(Relation heapRel,
|
|||||||
* unduly.
|
* unduly.
|
||||||
*/
|
*/
|
||||||
if (cmds)
|
if (cmds)
|
||||||
|
{
|
||||||
|
EventTriggerAlterTableStart((Node *) stmt);
|
||||||
AlterTableInternal(RelationGetRelid(heapRel), cmds, true);
|
AlterTableInternal(RelationGetRelid(heapRel), cmds, true);
|
||||||
|
EventTriggerAlterTableEnd();
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/*
|
/*
|
||||||
|
@ -1696,11 +1696,6 @@ EventTriggerCollectSimpleCommand(ObjectAddress address,
|
|||||||
* Note we don't collect the command immediately; instead we keep it in
|
* 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
|
* currentCommand, and only when we're done processing the subcommands we will
|
||||||
* add it to the command list.
|
* 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
|
void
|
||||||
EventTriggerAlterTableStart(Node *parsetree)
|
EventTriggerAlterTableStart(Node *parsetree)
|
||||||
@ -1725,6 +1720,7 @@ EventTriggerAlterTableStart(Node *parsetree)
|
|||||||
command->d.alterTable.subcmds = NIL;
|
command->d.alterTable.subcmds = NIL;
|
||||||
command->parsetree = copyObject(parsetree);
|
command->parsetree = copyObject(parsetree);
|
||||||
|
|
||||||
|
command->parent = currentEventTriggerState->currentCommand;
|
||||||
currentEventTriggerState->currentCommand = command;
|
currentEventTriggerState->currentCommand = command;
|
||||||
|
|
||||||
MemoryContextSwitchTo(oldcxt);
|
MemoryContextSwitchTo(oldcxt);
|
||||||
@ -1765,6 +1761,7 @@ EventTriggerCollectAlterTableSubcmd(Node *subcmd, ObjectAddress address)
|
|||||||
return;
|
return;
|
||||||
|
|
||||||
Assert(IsA(subcmd, AlterTableCmd));
|
Assert(IsA(subcmd, AlterTableCmd));
|
||||||
|
Assert(OidIsValid(currentEventTriggerState->currentCommand));
|
||||||
Assert(OidIsValid(currentEventTriggerState->currentCommand->d.alterTable.objectId));
|
Assert(OidIsValid(currentEventTriggerState->currentCommand->d.alterTable.objectId));
|
||||||
|
|
||||||
oldcxt = MemoryContextSwitchTo(currentEventTriggerState->cxt);
|
oldcxt = MemoryContextSwitchTo(currentEventTriggerState->cxt);
|
||||||
@ -1790,11 +1787,15 @@ EventTriggerCollectAlterTableSubcmd(Node *subcmd, ObjectAddress address)
|
|||||||
void
|
void
|
||||||
EventTriggerAlterTableEnd(void)
|
EventTriggerAlterTableEnd(void)
|
||||||
{
|
{
|
||||||
|
CollectedCommand *parent;
|
||||||
|
|
||||||
/* ignore if event trigger context not set, or collection disabled */
|
/* ignore if event trigger context not set, or collection disabled */
|
||||||
if (!currentEventTriggerState ||
|
if (!currentEventTriggerState ||
|
||||||
currentEventTriggerState->commandCollectionInhibited)
|
currentEventTriggerState->commandCollectionInhibited)
|
||||||
return;
|
return;
|
||||||
|
|
||||||
|
parent = currentEventTriggerState->currentCommand->parent;
|
||||||
|
|
||||||
/* If no subcommands, don't collect */
|
/* If no subcommands, don't collect */
|
||||||
if (list_length(currentEventTriggerState->currentCommand->d.alterTable.subcmds) != 0)
|
if (list_length(currentEventTriggerState->currentCommand->d.alterTable.subcmds) != 0)
|
||||||
{
|
{
|
||||||
@ -1805,7 +1806,7 @@ EventTriggerAlterTableEnd(void)
|
|||||||
else
|
else
|
||||||
pfree(currentEventTriggerState->currentCommand);
|
pfree(currentEventTriggerState->currentCommand);
|
||||||
|
|
||||||
currentEventTriggerState->currentCommand = NULL;
|
currentEventTriggerState->currentCommand = parent;
|
||||||
}
|
}
|
||||||
|
|
||||||
/*
|
/*
|
||||||
|
@ -34,6 +34,7 @@
|
|||||||
#include "commands/comment.h"
|
#include "commands/comment.h"
|
||||||
#include "commands/dbcommands.h"
|
#include "commands/dbcommands.h"
|
||||||
#include "commands/defrem.h"
|
#include "commands/defrem.h"
|
||||||
|
#include "commands/event_trigger.h"
|
||||||
#include "commands/tablecmds.h"
|
#include "commands/tablecmds.h"
|
||||||
#include "commands/tablespace.h"
|
#include "commands/tablespace.h"
|
||||||
#include "mb/pg_wchar.h"
|
#include "mb/pg_wchar.h"
|
||||||
@ -666,7 +667,7 @@ DefineIndex(Oid relationId,
|
|||||||
* Extra checks when creating a PRIMARY KEY index.
|
* Extra checks when creating a PRIMARY KEY index.
|
||||||
*/
|
*/
|
||||||
if (stmt->primary)
|
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
|
* If this table is partitioned and we're creating a unique index or a
|
||||||
|
@ -7045,7 +7045,7 @@ ATExecAddIndexConstraint(AlteredTableInfo *tab, Relation rel,
|
|||||||
|
|
||||||
/* Extra checks needed if making primary key */
|
/* Extra checks needed if making primary key */
|
||||||
if (stmt->primary)
|
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 */
|
/* Note we currently don't support EXCLUSION constraints here */
|
||||||
if (stmt->primary)
|
if (stmt->primary)
|
||||||
|
@ -61,6 +61,8 @@ validateWithCheckOption(const char *value)
|
|||||||
*
|
*
|
||||||
* Create a view relation and use the rules system to store the query
|
* Create a view relation and use the rules system to store the query
|
||||||
* for the view.
|
* for the view.
|
||||||
|
*
|
||||||
|
* EventTriggerAlterTableStart must have been called already.
|
||||||
*---------------------------------------------------------------------
|
*---------------------------------------------------------------------
|
||||||
*/
|
*/
|
||||||
static ObjectAddress
|
static ObjectAddress
|
||||||
@ -186,6 +188,7 @@ DefineVirtualRelation(RangeVar *relation, List *tlist, bool replace,
|
|||||||
atcmds = lappend(atcmds, atcmd);
|
atcmds = lappend(atcmds, atcmd);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/* EventTriggerAlterTableStart called by ProcessUtilitySlow */
|
||||||
AlterTableInternal(viewOid, atcmds, true);
|
AlterTableInternal(viewOid, atcmds, true);
|
||||||
|
|
||||||
/* Make the new view columns visible */
|
/* Make the new view columns visible */
|
||||||
@ -217,6 +220,7 @@ DefineVirtualRelation(RangeVar *relation, List *tlist, bool replace,
|
|||||||
atcmd->def = (Node *) options;
|
atcmd->def = (Node *) options;
|
||||||
atcmds = list_make1(atcmd);
|
atcmds = list_make1(atcmd);
|
||||||
|
|
||||||
|
/* EventTriggerAlterTableStart called by ProcessUtilitySlow */
|
||||||
AlterTableInternal(viewOid, atcmds, true);
|
AlterTableInternal(viewOid, atcmds, true);
|
||||||
|
|
||||||
ObjectAddressSet(address, RelationRelationId, viewOid);
|
ObjectAddressSet(address, RelationRelationId, viewOid);
|
||||||
|
@ -40,7 +40,8 @@ typedef enum
|
|||||||
|
|
||||||
extern void index_check_primary_key(Relation heapRel,
|
extern void index_check_primary_key(Relation heapRel,
|
||||||
IndexInfo *indexInfo,
|
IndexInfo *indexInfo,
|
||||||
bool is_alter_table);
|
bool is_alter_table,
|
||||||
|
IndexStmt *stmt);
|
||||||
|
|
||||||
#define INDEX_CREATE_IS_PRIMARY (1 << 0)
|
#define INDEX_CREATE_IS_PRIMARY (1 << 0)
|
||||||
#define INDEX_CREATE_ADD_CONSTRAINT (1 << 1)
|
#define INDEX_CREATE_ADD_CONSTRAINT (1 << 1)
|
||||||
|
@ -44,6 +44,7 @@ typedef struct CollectedATSubcmd
|
|||||||
typedef struct CollectedCommand
|
typedef struct CollectedCommand
|
||||||
{
|
{
|
||||||
CollectedCommandType type;
|
CollectedCommandType type;
|
||||||
|
|
||||||
bool in_extension;
|
bool in_extension;
|
||||||
Node *parsetree;
|
Node *parsetree;
|
||||||
|
|
||||||
@ -100,6 +101,8 @@ typedef struct CollectedCommand
|
|||||||
ObjectType objtype;
|
ObjectType objtype;
|
||||||
} defprivs;
|
} defprivs;
|
||||||
} d;
|
} d;
|
||||||
|
|
||||||
|
struct CollectedCommand *parent; /* when nested */
|
||||||
} CollectedCommand;
|
} CollectedCommand;
|
||||||
|
|
||||||
#endif /* DEPARSE_UTILITY_H */
|
#endif /* DEPARSE_UTILITY_H */
|
||||||
|
@ -16,3 +16,15 @@ NOTICE: DDL test: type simple, tag ALTER TABLE
|
|||||||
ALTER TABLE parent ADD CONSTRAINT a_pos CHECK (a > 0);
|
ALTER TABLE parent ADD CONSTRAINT a_pos CHECK (a > 0);
|
||||||
NOTICE: DDL test: type alter table, tag ALTER TABLE
|
NOTICE: DDL test: type alter table, tag ALTER TABLE
|
||||||
NOTICE: subcommand: ADD CONSTRAINT (and recurse)
|
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
|
||||||
|
@ -11,3 +11,11 @@ ALTER TABLE parent ADD COLUMN b serial;
|
|||||||
ALTER TABLE parent RENAME COLUMN b TO c;
|
ALTER TABLE parent RENAME COLUMN b TO c;
|
||||||
|
|
||||||
ALTER TABLE parent ADD CONSTRAINT a_pos CHECK (a > 0);
|
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);
|
||||||
|
@ -349,6 +349,18 @@ CREATE SCHEMA evttrig
|
|||||||
CREATE TABLE one (col_a SERIAL PRIMARY KEY, col_b text DEFAULT 'forty two')
|
CREATE TABLE one (col_a SERIAL PRIMARY KEY, col_b text DEFAULT 'forty two')
|
||||||
CREATE INDEX one_idx ON one (col_b)
|
CREATE INDEX one_idx ON one (col_b)
|
||||||
CREATE TABLE two (col_c INTEGER CHECK (col_c > 0) REFERENCES one DEFAULT 42);
|
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.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=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={}
|
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;
|
DROP INDEX evttrig.one_idx;
|
||||||
NOTICE: NORMAL: orig=t normal=f istemp=f type=index identity=evttrig.one_idx name={evttrig,one_idx} args={}
|
NOTICE: NORMAL: orig=t normal=f istemp=f type=index identity=evttrig.one_idx name={evttrig,one_idx} args={}
|
||||||
DROP SCHEMA evttrig CASCADE;
|
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
|
DETAIL: drop cascades to table evttrig.one
|
||||||
drop cascades to table evttrig.two
|
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=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=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=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=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.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;
|
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={}
|
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;
|
DROP EVENT TRIGGER regress_event_trigger_report_dropped;
|
||||||
|
@ -274,6 +274,19 @@ CREATE SCHEMA evttrig
|
|||||||
CREATE INDEX one_idx ON one (col_b)
|
CREATE INDEX one_idx ON one (col_b)
|
||||||
CREATE TABLE two (col_c INTEGER CHECK (col_c > 0) REFERENCES one DEFAULT 42);
|
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.two DROP COLUMN col_c;
|
||||||
ALTER TABLE evttrig.one ALTER COLUMN col_b DROP DEFAULT;
|
ALTER TABLE evttrig.one ALTER COLUMN col_b DROP DEFAULT;
|
||||||
ALTER TABLE evttrig.one DROP CONSTRAINT one_pkey;
|
ALTER TABLE evttrig.one DROP CONSTRAINT one_pkey;
|
||||||
|
Loading…
Reference in New Issue
Block a user