From 43f29ac13abda3c6ad71143d462c2dcd2c42f521 Mon Sep 17 00:00:00 2001 From: amit Date: Mon, 15 May 2017 14:00:54 +0900 Subject: [PATCH] Fix to make partitioned tables work with event triggers There were a couple of bugs here: 1. When creating range partitioned tables, AlterTableInternal is called which invokes the event trigger code. We must perform EventTriggerAlterTableStart() before calling AlterTableInternal so that the event trigger context information is initialized at all. 2. When creating a partition, CreateStmt.tableElts is likely to become invalid upon return from MergeAttributes(). We must replace the invalid value with the newly constructed correct list value computed by MergeAttributes(). For example, in the caller ProcessUtilitySlow(), EventTriggerCollectSimpleCommand() is called right after returning from DefineRelation(), and it expects the the content of CreateStmt to be valid to perform copyObject on it. First bug was reported by Mark Dilger. Report: https://postgr.es/m/4A9B8269-9771-4FB7-BFA3-84249800917D%40gmail.com --- src/backend/commands/tablecmds.c | 17 ++++++++++++++ src/test/regress/expected/event_trigger.out | 32 ++++++++++++++++++++++++++ src/test/regress/sql/event_trigger.sql | 35 +++++++++++++++++++++++++++++ 3 files changed, 84 insertions(+) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index b94db89b25..c3f489308a 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -620,6 +620,19 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId, stmt->relation->relpersistence, stmt->partbound != NULL, &inheritOids, &old_constraints, &parentOidCount); + /* + * The original value of stmt->tableElts may have been obsoleted by + * MergeAttributes(), especially those for partitions, wherein the + * cells of stmt->tableElts are deleted. Note that in a partition's + * case, stmt->tableElts contains dummy ColumnDef's created to capture + * the partition's own column constraints which are merged into the actual + * column definitions derived from the parent, before deleting the + * aforementioned dummy ones. We don't need to refer to stmt->tableElts + * hereafter in this function, although the caller expects it to contain + * valid elements upon return. So replace the original list with the + * one that's known to be valid. + */ + stmt->tableElts = schema; /* * Create a tuple descriptor from the relation schema. Note that this @@ -853,7 +866,11 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId, * pass true for recurse; ATPrepSetNotNull() complains if we don't */ if (cmds != NIL) + { + EventTriggerAlterTableStart((Node *) stmt); AlterTableInternal(RelationGetRelid(rel), cmds, true); + EventTriggerAlterTableEnd(); + } } } diff --git a/src/test/regress/expected/event_trigger.out b/src/test/regress/expected/event_trigger.out index 906dcb8b31..d5021b648e 100644 --- a/src/test/regress/expected/event_trigger.out +++ b/src/test/regress/expected/event_trigger.out @@ -454,3 +454,35 @@ NOTICE: DROP POLICY - ddl_command_end DROP EVENT TRIGGER start_rls_command; DROP EVENT TRIGGER end_rls_command; DROP EVENT TRIGGER sql_drop_command; +-- check events triggers work correctly in the case of creating partitioned +-- tables, especially range-partitioned ones which invoke +-- AlterTableInternal(). +CREATE OR REPLACE FUNCTION create_partitioned_table_before_proc() RETURNS +event_trigger AS $$ +BEGIN + RAISE WARNING 'preparing to create table: % %', tg_event, tg_tag; +END; +$$ LANGUAGE plpgsql; +CREATE EVENT TRIGGER create_partitioned_table_before_trigger + ON ddl_command_start + WHEN TAG IN ('CREATE TABLE') + EXECUTE PROCEDURE create_partitioned_table_before_proc(); +CREATE OR REPLACE FUNCTION create_partitioned_table_after_proc() +RETURNS event_trigger AS $$ +BEGIN + RAISE WARNING 'finished creating table: % %', tg_event, tg_tag; +END; +$$ LANGUAGE plpgsql; +CREATE EVENT TRIGGER create_partitioned_table_after_trigger + ON ddl_command_end + WHEN TAG IN ('CREATE TABLE') + EXECUTE PROCEDURE create_partitioned_table_after_proc(); +-- create a range-partitioned table +CREATE TABLE range_partitioned (a int) PARTITION BY RANGE (a); +WARNING: preparing to create table: ddl_command_start CREATE TABLE +WARNING: finished creating table: ddl_command_end CREATE TABLE +DROP TABLE range_partitioned; +DROP EVENT TRIGGER create_partitioned_table_before_trigger; +DROP EVENT TRIGGER create_partitioned_table_after_trigger; +DROP FUNCTION create_partitioned_table_before_proc(); +DROP FUNCTION create_partitioned_table_after_proc(); diff --git a/src/test/regress/sql/event_trigger.sql b/src/test/regress/sql/event_trigger.sql index b65bf3ec66..1491fd7f07 100644 --- a/src/test/regress/sql/event_trigger.sql +++ b/src/test/regress/sql/event_trigger.sql @@ -375,3 +375,38 @@ DROP POLICY p2 ON event_trigger_test; DROP EVENT TRIGGER start_rls_command; DROP EVENT TRIGGER end_rls_command; DROP EVENT TRIGGER sql_drop_command; + +-- check events triggers work correctly in the case of creating partitioned +-- tables, especially range-partitioned ones which invoke +-- AlterTableInternal(). +CREATE OR REPLACE FUNCTION create_partitioned_table_before_proc() RETURNS +event_trigger AS $$ +BEGIN + RAISE WARNING 'preparing to create table: % %', tg_event, tg_tag; +END; +$$ LANGUAGE plpgsql; + +CREATE EVENT TRIGGER create_partitioned_table_before_trigger + ON ddl_command_start + WHEN TAG IN ('CREATE TABLE') + EXECUTE PROCEDURE create_partitioned_table_before_proc(); + +CREATE OR REPLACE FUNCTION create_partitioned_table_after_proc() +RETURNS event_trigger AS $$ +BEGIN + RAISE WARNING 'finished creating table: % %', tg_event, tg_tag; +END; +$$ LANGUAGE plpgsql; + +CREATE EVENT TRIGGER create_partitioned_table_after_trigger + ON ddl_command_end + WHEN TAG IN ('CREATE TABLE') + EXECUTE PROCEDURE create_partitioned_table_after_proc(); + +-- create a range-partitioned table +CREATE TABLE range_partitioned (a int) PARTITION BY RANGE (a); +DROP TABLE range_partitioned; +DROP EVENT TRIGGER create_partitioned_table_before_trigger; +DROP EVENT TRIGGER create_partitioned_table_after_trigger; +DROP FUNCTION create_partitioned_table_before_proc(); +DROP FUNCTION create_partitioned_table_after_proc(); -- 2.11.0