pgsql: Fix event triggers for partitioned tables
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: /messages/by-id/CANNhMLCpi+HQ7M36uPfGbJZEQLyTy7XvX=5EFkpR-b1bo0uJew@mail.gmail.com
Branch
------
master
Details
-------
https://git.postgresql.org/pg/commitdiff/ad08006ba03797fed431af87de6e66c6c0985b7a
Modified Files
--------------
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(-)
Hi Alvaro,
On Sat, Oct 06, 2018 at 10:18:46PM +0000, Alvaro Herrera wrote:
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.
This commit is producing a warning with my compiler:
event_trigger.c:1764:9: note: in expansion of macro ‘OidIsValid’
Assert(OidIsValid(currentEventTriggerState->currentCommand));
The fix is obvious because currentCommand is a pointer and not an Oid.
Please see attached. Should I fix it myself?
--
Michael
Attachments:
event-trigger-warning.patchtext/x-diff; charset=us-asciiDownload
diff --git a/src/backend/commands/event_trigger.c b/src/backend/commands/event_trigger.c
index 2c1dc47541..9a702e4097 100644
--- a/src/backend/commands/event_trigger.c
+++ b/src/backend/commands/event_trigger.c
@@ -1761,7 +1761,7 @@ EventTriggerCollectAlterTableSubcmd(Node *subcmd, ObjectAddress address)
return;
Assert(IsA(subcmd, AlterTableCmd));
- Assert(OidIsValid(currentEventTriggerState->currentCommand));
+ Assert(currentEventTriggerState->currentCommand != NULL);
Assert(OidIsValid(currentEventTriggerState->currentCommand->d.alterTable.objectId));
oldcxt = MemoryContextSwitchTo(currentEventTriggerState->cxt);
On 07/10/2018 00:18, Alvaro Herrera wrote:
Fix event triggers for partitioned tables
It looks like this is causing compiler warnings:
event_trigger.c: In function 'EventTriggerCollectAlterTableSubcmd':
../../../src/include/c.h:605:51: error: comparison between pointer and
zero character constant [-Werror=pointer-compare]
#define OidIsValid(objectId) ((bool) ((objectId) != InvalidOid))
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 2018-Oct-08, Michael Paquier wrote:
Hi Alvaro,
On Sat, Oct 06, 2018 at 10:18:46PM +0000, Alvaro Herrera wrote:
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.This commit is producing a warning with my compiler:
event_trigger.c:1764:9: note: in expansion of macro ‘OidIsValid’
Assert(OidIsValid(currentEventTriggerState->currentCommand));The fix is obvious because currentCommand is a pointer and not an Oid.
Please see attached. Should I fix it myself?
If you have a commit ready, please do, thanks.
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 2018-Oct-08, Alvaro Herrera wrote:
On 2018-Oct-08, Michael Paquier wrote:
The fix is obvious because currentCommand is a pointer and not an Oid.
Please see attached. Should I fix it myself?
Pushed now, thanks.
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services