Merge pull request #107 from MasahikoSawada/change_to_after_trigger

Change trigger type to AFTER trigger.

bin/pg_repack.c

@@ -186,8 +186,8 @@ typedef struct repack_table
 	Oid				ckid;			/* target: CK OID */
 	const char	   *create_pktype;	/* CREATE TYPE pk */
 	const char	   *create_log;		/* CREATE TABLE log */
-	const char	   *create_trigger;	/* CREATE TRIGGER z_repack_trigger */
+	const char	   *create_trigger;	/* CREATE TRIGGER repack_trigger */
-	const char	   *enable_trigger;	/* ALTER TABLE ENABLE ALWAYS TRIGGER z_repack_trigger */
+	const char	   *enable_trigger;	/* ALTER TABLE ENABLE ALWAYS TRIGGER repack_trigger */
 	const char	   *create_table;	/* CREATE TABLE table AS SELECT */
 	const char	   *drop_columns;	/* ALTER TABLE DROP COLUMNs */
 	const char	   *delete_log;		/* DELETE FROM log */
@@ -1024,7 +1024,7 @@ repack_one_table(repack_table *table, const char *orderby)
 	const char     *appname = getenv("PGAPPNAME");
 
 	/* Keep track of whether we have gotten through setup to install
-	 * the z_repack_trigger, log table, etc. ourselves. We don't want to
+	 * the repack_trigger, log table, etc. ourselves. We don't want to
 	 * go through repack_cleanup() if we didn't actually set up the
 	 * trigger ourselves, lest we be cleaning up another pg_repack's mess,
 	 * or worse, interfering with a still-running pg_repack.
@@ -1126,43 +1126,28 @@ repack_one_table(repack_table *table, const char *orderby)
 
 
 	/*
-	 * Check z_repack_trigger is the trigger executed last so that
+	 * Check if repack_trigger is not conflict with existing trigger. We can
-	 * other before triggers cannot modify triggered tuples.
+	 * find it out later but we check it in advance and go to cleanup if needed.
+	 * In AFTER trigger context, since triggered tuple is not changed by other
+	 * trigger we don't care about the fire order.
 	 */
 	res = execute("SELECT repack.conflicted_triggers($1)", 1, params);
 	if (PQntuples(res) > 0)
 	{
-		if (0 == strcmp("z_repack_trigger", PQgetvalue(res, 0, 0)))
+		ereport(WARNING,
-		{
-			ereport(WARNING,
 				(errcode(E_PG_COMMAND),
 				 errmsg("the table \"%s\" already has a trigger called \"%s\"",
-					table->target_name, PQgetvalue(res, 0, 0)),
+						table->target_name, "repack_trigger"),
 				 errdetail(
-					"The trigger was probably installed during a previous"
+					 "The trigger was probably installed during a previous"
-					" attempt to run pg_repack on the table which was"
+					 " attempt to run pg_repack on the table which was"
-					" interrupted and for some reason failed to clean up"
+					 " interrupted and for some reason failed to clean up"
-					" the temporary objects.  Please drop the trigger or drop"
+					 " the temporary objects.  Please drop the trigger or drop"
 					" and recreate the pg_repack extension altogether"
-					" to remove all the temporary objects left over.")));
+					 " to remove all the temporary objects left over.")));
-		}
-		else
-		{
-			ereport(WARNING,
-				(errcode(E_PG_COMMAND),
-				 errmsg("trigger \"%s\" conflicting on table \"%s\"",
-					PQgetvalue(res, 0, 0), table->target_name),
-				 errdetail(
-					"The trigger \"z_repack_trigger\" must be the last of the"
-					" BEFORE triggers to fire on the table (triggers fire in"
-					" alphabetical order). Please rename the trigger so that"
-					" it sorts before \"z_repack_trigger\": you can use"
-					" \"ALTER TRIGGER %s ON %s RENAME TO newname\".",
-					PQgetvalue(res, 0, 0), table->target_name)));
-		}
-
 		goto cleanup;
 	}
+
 	CLEARPGRES(res);
 
 	command(table->create_pktype, 0, NULL);
@@ -1232,7 +1217,7 @@ repack_one_table(repack_table *table, const char *orderby)
 	 */
 	command("COMMIT", 0, NULL);
 
-	/* The main connection has now committed its z_repack_trigger,
+	/* The main connection has now committed its repack_trigger,
 	 * log table, and temp. table. If any error occurs from this point
 	 * on and we bail out, we should try to clean those up.
 	 */

doc/pg_repack.rst

@@ -362,7 +362,7 @@ ERROR: query failed: ERROR: column "col" does not exist
 
     Specify existing columns.
 
-WARNING: the table "tbl" already has a trigger called z_repack_trigger
+WARNING: the table "tbl" already has a trigger called repack_trigger
     The trigger was probably installed during a previous attempt to run
     pg_repack on the table which was interrupted and for some reason failed
     to clean up the temporary objects.
@@ -370,16 +370,6 @@ WARNING: the table "tbl" already has a trigger called z_repack_trigger
     You can remove all the temporary objects by dropping and re-creating the
     extension: see the installation_ section for the details.
 
-WARNING: trigger "trg" conflicting on table "tbl"
-    The target table has a trigger whose name follows ``z_repack_trigger``
-    in alphabetical order.
-
-    The ``z_repack_trigger`` should be the last BEFORE trigger to fire.
-    Please rename your trigger so that it sorts alphabetically before
-    pg_repack's one; you can use::
-
-        ALTER TRIGGER zzz_my_trigger ON sometable RENAME TO yyy_my_trigger;
-
 ERROR: Another pg_repack command may be running on the table. Please try again
     later.
 

doc/pg_repack_jp.rst

@@ -651,7 +651,7 @@ ERROR: query failed: ERROR: column "col" does not exist
     対象のテーブルが  ``--order-by`` オプションで指定したカラムを持っていない場合に表示されます。
     存在しているカラムを指定してください。
 
-.. WARNING: the table "tbl" already has a trigger called z_repack_trigger
+.. WARNING: the table "tbl" already has a trigger called a_repack_trigger
     The trigger was probably installed during a previous attempt to run
     pg_repack on the table which was interrupted and for some reason failed
     to clean up the temporary objects.
@@ -661,37 +661,21 @@ ERROR: query failed: ERROR: column "col" does not exist
 
 .. class:: diag
 
-WARNING: the table "tbl" already has a trigger called z_repack_trigger
+WARNING: the table "tbl" already has a trigger called repack_trigger
     以前に実行したが何らかの理由で中断したか、あるいは失敗したpg_repackコマンドにより、
     対象テーブルにpg_repackが利用するトリガが残存している場合に表示されます。
     pg_repackを一度削除して、再度登録することで、こうした一時オブジェクトを削除できます。
     `インストール`_ を参照してください。
     
 .. WARNING: trigger "trg" conflicting on table "tbl"
-    The target table has a trigger whose name follows ``z_repack_trigger``
+    The target table has a trigger whose name follows ``repack_trigger``
     in alphabetical order.
   
-    The ``z_repack_trigger`` should be the last BEFORE trigger to fire.
+    The ``repack_trigger`` should be the first AFTER trigger to fire.
     Please rename your trigger so that it sorts alphabetically before
     pg_repack's one; you can use::
   
-        ALTER TRIGGER zzz_my_trigger ON sometable RENAME TO yyy_my_trigger;
+        ALTER TRIGGER aaa_my_trigger ON sometable RENAME TO bbb_my_trigger;
-
-.. class:: diag
-
-WARNING: trigger "trg" conflicting on table "tbl"
-    対象のテーブルが、pg_repackが利用する ``z_repack_trigger`` という名前のトリガ
-    よりもアルファベット順で後ろになるような名前のトリガを持っている場合に表示されます。
-    ``z_repack_trigger`` トリガは最後に実行されるBEFOREトリガになる必要があります。
-    該当のトリガ名称を変更してください。::
-
-        ALTER TRIGGER zzz_my_trigger ON sometable RENAME TO yyy_my_trigger;
-
-.. ERROR: Another pg_repack command may be running on the table. Please try again
-    later.
-  
-   There is a chance of deadlock when two concurrent pg_repack commands are run
-   on the same table. So, try to run the command after some time.
 
 .. class:: diag
 

lib/pg_repack.sql.in

@@ -68,8 +68,8 @@ LANGUAGE sql STABLE STRICT;
 CREATE FUNCTION repack.get_create_trigger(relid oid, pkid oid)
   RETURNS text AS
 $$
-  SELECT 'CREATE TRIGGER z_repack_trigger' ||
+  SELECT 'CREATE TRIGGER repack_trigger' ||
-         ' BEFORE INSERT OR DELETE OR UPDATE ON ' || repack.oid2text($1) ||
+         ' AFTER INSERT OR DELETE OR UPDATE ON ' || repack.oid2text($1) ||
          ' FOR EACH ROW EXECUTE PROCEDURE repack.repack_trigger(' ||
          '''INSERT INTO repack.log_' || $1 || '(pk, row) VALUES(' ||
          ' CASE WHEN $1 IS NULL THEN NULL ELSE (ROW($1.' ||
@@ -82,7 +82,7 @@ CREATE FUNCTION repack.get_enable_trigger(relid oid)
   RETURNS text AS
 $$
   SELECT 'ALTER TABLE ' || repack.oid2text($1) ||
-    ' ENABLE ALWAYS TRIGGER z_repack_trigger';
+    ' ENABLE ALWAYS TRIGGER repack_trigger';
 $$
 LANGUAGE sql STABLE STRICT;
 
@@ -223,8 +223,7 @@ LANGUAGE C VOLATILE STRICT SECURITY DEFINER;
 CREATE FUNCTION repack.conflicted_triggers(oid) RETURNS SETOF name AS
 $$
 SELECT tgname FROM pg_trigger
- WHERE tgrelid = $1 AND tgname >= 'z_repack_trigger'
+ WHERE tgrelid = $1 AND tgname = 'repack_trigger'
- AND (tgtype & 2) = 2      -- BEFORE trigger
  ORDER BY tgname;
 $$
 LANGUAGE sql STABLE STRICT;

lib/repack.c

@@ -151,7 +151,7 @@ repack_trigger(PG_FUNCTION_ARGS)
 
 	/* make sure it's called as a trigger at all */
 	if (!CALLED_AS_TRIGGER(fcinfo) ||
-		!TRIGGER_FIRED_BEFORE(trigdata->tg_event) ||
+		!TRIGGER_FIRED_AFTER(trigdata->tg_event) ||
 		!TRIGGER_FIRED_FOR_ROW(trigdata->tg_event) ||
 		trigdata->tg_trigger->tgnargs != 1)
 		elog(ERROR, "repack_trigger: invalid trigger call");
@@ -921,7 +921,7 @@ repack_swap(PG_FUNCTION_ARGS)
 	/* drop repack trigger */
 	execute_with_format(
 		SPI_OK_UTILITY,
-		"DROP TRIGGER IF EXISTS z_repack_trigger ON %s.%s CASCADE",
+		"DROP TRIGGER IF EXISTS repack_trigger ON %s.%s CASCADE",
 		nspname, relname);
 
 	SPI_finish();
@@ -962,7 +962,7 @@ repack_drop(PG_FUNCTION_ARGS)
 	 * To prevent concurrent lockers of the repack target table from causing
 	 * deadlocks, take an exclusive lock on it. Consider that the following
 	 * commands take exclusive lock on tables log_xxx and the target table
-	 * itself when deleting the z_repack_trigger on it, while concurrent
+	 * itself when deleting the repack_trigger on it, while concurrent
 	 * updaters require row exclusive lock on the target table and in
 	 * addition, on the log_xxx table, because of the trigger.
 	 *
@@ -1011,7 +1011,7 @@ repack_drop(PG_FUNCTION_ARGS)
 	{
 		execute_with_format(
 			SPI_OK_UTILITY,
-			"DROP TRIGGER IF EXISTS z_repack_trigger ON %s.%s CASCADE",
+			"DROP TRIGGER IF EXISTS repack_trigger ON %s.%s CASCADE",
 			nspname, relname);
 		--numobj;
 	}

regress/expected/repack.out

@@ -338,25 +338,19 @@ CREATE FUNCTION trgtest() RETURNS trigger AS
 $$BEGIN RETURN NEW; END$$
 LANGUAGE plpgsql;
 CREATE TABLE trg1 (id integer PRIMARY KEY);
-CREATE TRIGGER z_repack_triggeq BEFORE UPDATE ON trg1 FOR EACH ROW EXECUTE PROCEDURE trgtest();
+CREATE TRIGGER repack_trigger_1 AFTER UPDATE ON trg1 FOR EACH ROW EXECUTE PROCEDURE trgtest();
 \! pg_repack --dbname=contrib_regression --table=trg1
 INFO: repacking table "trg1"
 CREATE TABLE trg2 (id integer PRIMARY KEY);
-CREATE TRIGGER z_repack_trigger BEFORE UPDATE ON trg2 FOR EACH ROW EXECUTE PROCEDURE trgtest();
+CREATE TRIGGER repack_trigger AFTER UPDATE ON trg2 FOR EACH ROW EXECUTE PROCEDURE trgtest();
 \! pg_repack --dbname=contrib_regression --table=trg2
 INFO: repacking table "trg2"
-WARNING: the table "trg2" already has a trigger called "z_repack_trigger"
+WARNING: the table "trg2" already has a trigger called "repack_trigger"
 DETAIL: The trigger was probably installed during a previous attempt to run pg_repack on the table which was interrupted and for some reason failed to clean up the temporary objects.  Please drop the trigger or drop and recreate the pg_repack extension altogether to remove all the temporary objects left over.
 CREATE TABLE trg3 (id integer PRIMARY KEY);
-CREATE TRIGGER z_repack_trigges BEFORE UPDATE ON trg3 FOR EACH ROW EXECUTE PROCEDURE trgtest();
+CREATE TRIGGER repack_trigger_1 BEFORE UPDATE ON trg3 FOR EACH ROW EXECUTE PROCEDURE trgtest();
 \! pg_repack --dbname=contrib_regression --table=trg3
 INFO: repacking table "trg3"
-WARNING: trigger "z_repack_trigges" conflicting on table "trg3"
-DETAIL: The trigger "z_repack_trigger" must be the last of the BEFORE triggers to fire on the table (triggers fire in alphabetical order). Please rename the trigger so that it sorts before "z_repack_trigger": you can use "ALTER TRIGGER z_repack_trigges ON trg3 RENAME TO newname".
-CREATE TABLE trg4 (id integer PRIMARY KEY);
-CREATE TRIGGER zzzzzz AFTER UPDATE ON trg4 FOR EACH ROW EXECUTE PROCEDURE trgtest();
-\! pg_repack --dbname=contrib_regression --table=trg4
-INFO: repacking table "trg4"
 --
 -- Dry run
 --

regress/sql/repack.sql

@@ -197,17 +197,14 @@ CREATE FUNCTION trgtest() RETURNS trigger AS
 $$BEGIN RETURN NEW; END$$
 LANGUAGE plpgsql;
 CREATE TABLE trg1 (id integer PRIMARY KEY);
-CREATE TRIGGER z_repack_triggeq BEFORE UPDATE ON trg1 FOR EACH ROW EXECUTE PROCEDURE trgtest();
+CREATE TRIGGER repack_trigger_1 AFTER UPDATE ON trg1 FOR EACH ROW EXECUTE PROCEDURE trgtest();
 \! pg_repack --dbname=contrib_regression --table=trg1
 CREATE TABLE trg2 (id integer PRIMARY KEY);
-CREATE TRIGGER z_repack_trigger BEFORE UPDATE ON trg2 FOR EACH ROW EXECUTE PROCEDURE trgtest();
+CREATE TRIGGER repack_trigger AFTER UPDATE ON trg2 FOR EACH ROW EXECUTE PROCEDURE trgtest();
 \! pg_repack --dbname=contrib_regression --table=trg2
 CREATE TABLE trg3 (id integer PRIMARY KEY);
-CREATE TRIGGER z_repack_trigges BEFORE UPDATE ON trg3 FOR EACH ROW EXECUTE PROCEDURE trgtest();
+CREATE TRIGGER repack_trigger_1 BEFORE UPDATE ON trg3 FOR EACH ROW EXECUTE PROCEDURE trgtest();
 \! pg_repack --dbname=contrib_regression --table=trg3
-CREATE TABLE trg4 (id integer PRIMARY KEY);
-CREATE TRIGGER zzzzzz AFTER UPDATE ON trg4 FOR EACH ROW EXECUTE PROCEDURE trgtest();
-\! pg_repack --dbname=contrib_regression --table=trg4
 
 --
 -- Dry run