Adding the optional clause 'AS' in CREATE TRIGGER
Hi,
I would like to add the following support for a trigger.
This implementation enables to create a trigger efficiently
in single command.
It had been discussed before. The link is as shown below.
/messages/by-id/CAA-aLv4m=f9cc1zcUzM49pE8+2NpytUDraTgfBmkTOkMN_wO2w@mail.gmail.com
Currently, PostgreSQL requires two steps to create a trigger.
1. to create a function.
2. to define a trigger with action specified via already created function.
Supporting 'AS' clause in CREATE TRIGGER syntax will enable the option of
defining the trigger in single command.
As a bonus, it will be compatible with oracle.
Also, the optional clause 'OR REPLACE' is required as below.
/messages/by-id/CAA-aLv6KYgVt2CwaRdcnptzWVngEm72Cp4mUFnF-MfeH0gS91g@mail.gmail.com
Currently, to change the definition of a trigger, trigger needs to
be dropped first before creating it again with new definition.
To change the definition of a function in CREATE TRIGGER syntax,
trigger needs to be dropped first before creating it again with new definition, too!
So, we need to add the optional clause 'OR REPLACE'.
Adding the optional clauses 'AS' and 'OR REPLACE' in CREATE TRIGGER syntax gives
the comfort of defining the trigger or redefining the trigger definition
which contains the function definition in single command.
Here is the syntax based on the previous discussion.
CREATE [ OR REPLACE ] [ CONSTRAINT ] TRIGGER name { BEFORE | AFTER | INSTEAD OF }
{ event [ OR ... ] }
ON table_name
[ FROM referenced_table_name ]
[ NOT DEFERRABLE | [ DEFERRABLE ] { INITIALLY IMMEDIATE | INITIALLY DEFERRED } ]
[ FOR [ EACH ] { ROW | STATEMENT } ]
[ WHEN ( condition ) ]
{ EXECUTE PROCEDURE function_name ( arguments )
| AS 'trigger function definition' [ LANGUAGE lang_name ]
[ SET configuration_parameter { TO value | = value | FROM CURRENT }]
}
If you have your opinion on this concept, please give me it.
Regards,
Okano Naoki
Fujitsu
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
"Okano, Naoki" <okano.naoki@jp.fujitsu.com> writes:
I would like to add the following support for a trigger.
This implementation enables to create a trigger efficiently
in single command.
It had been discussed before. The link is as shown below.
/messages/by-id/CAA-aLv4m=f9cc1zcUzM49pE8+2NpytUDraTgfBmkTOkMN_wO2w@mail.gmail.com
I think people pretty much lost interest in that proposal, which is
why it never got finished. Aside from the definitional issues discussed
in that thread, I can think of a few other problems:
1. The impression I have is that most people write trigger functions
so that they can be shared across multiple uses. That'd be impossible
with anonymous trigger function blocks. So you'd be trading off merging
two commands into one, versus having to write out the whole body of the
trigger multiple times, which wouldn't be much of a win.
2. I am concerned that every time we add any syntax to CREATE FUNCTION,
we'll have to think about whether we need to add it to CREATE TRIGGER.
(Or more likely, we'll forget and then users will complain.)
3. There's a lot of infrastructure that's built up around CREATE FUNCTION,
such as psql's \ef and \sf commands. We'd soon find ourselves having to
duplicate all that for triggers.
So personally I feel that the value-for-work-expended ratio here is
pretty low.
But in any case it would be a serious mistake to do this without first
implementing CREATE OR REPLACE TRIGGER. I think that's an entirely
separate proposal and you would be well advised to treat it as such.
It's far from trivial because of locking considerations. You probably
have to take an exclusive lock on the owning relation in order to change
trigger properties. (An advantage of the separate-function-definition
approach is that you can replace the function body with a much weaker
lock.)
Supporting 'AS' clause in CREATE TRIGGER syntax will enable the option of
defining the trigger in single command.
As a bonus, it will be compatible with oracle.
It certainly would not match Oracle's syntax for trigger bodies. It might
slightly reduce the conversion pain, but only slightly.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
But in any case it would be a serious mistake to do this without first
implementing CREATE OR REPLACE TRIGGER. I think that's an entirely separate
proposal and you would be well advised to treat it as such.
I see. There are more problems than I expected...
Let me start with 'OR REPLACE' clause.
At least, adding only 'OR REPLACE' clause has the following advantages.
* It enables users to redefine a trigger in single command.
* The trigger can always be referenced when redefining a trigger.
# But it is not so when using 'DROP' and 'CREATE' commands.
It is useful when users change the function or change the condition of 'WHEN' clause.
Regard,
Okano Naoki
Fujitsu
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Wednesday, November 16, 2016 4:31 PM Okano Naoki wrote:
But in any case it would be a serious mistake to do this without first
implementing CREATE OR REPLACE TRIGGER. I think that's an entirely separate
proposal and you would be well advised to treat it as such.I see. There are more problems than I expected...
Let me start with 'OR REPLACE' clause.
I tried to cretae a patch for CREATE OR REPLACE TRIGGER.
An example of execution is shown below.
example)
1.define a new trigger
CREATE TRIGGER regular_trigger
AFTER UPDATE ON table_name
REFERENCING OLD TABLE AS oldtable_1 NEW TABLE AS newtable_1
FOR EACH STATEMENT
EXECUTE PROCEDURE func_1();
2.redinfe a trigger in single command
CREATE OR REPLACE TRIGGER regular_trigger
AFTER UPDATE OR DELETE ON table_name
REFERENCING OLD TABLE AS oldtable_2 NEW TABLE AS newtable_2
FOR EACH ROW
EXECUTE PROCEDURE func_2();
If the named trigger does not exist.
a new trigger is also created by using OR REPLACE clause.
A regular trigger cannot be replaced by a constraint trigger and vice varsa.
because a constraint trigger has a different role from regular triger.
Please give me feedback.
Regards,
Okano Naoki
Fujitsu
Attachments:
OR_REPLACE_for_CREATE_TRIGGER_v1.patchapplication/octet-stream; name=OR_REPLACE_for_CREATE_TRIGGER_v1.patchDownload
diff --git a/doc/src/sgml/ref/create_trigger.sgml b/doc/src/sgml/ref/create_trigger.sgml
index 8590e22..40b695e 100644
--- a/doc/src/sgml/ref/create_trigger.sgml
+++ b/doc/src/sgml/ref/create_trigger.sgml
@@ -21,7 +21,7 @@ PostgreSQL documentation
<refsynopsisdiv>
<synopsis>
-CREATE [ CONSTRAINT ] TRIGGER <replaceable class="PARAMETER">name</replaceable> { BEFORE | AFTER | INSTEAD OF } { <replaceable class="PARAMETER">event</replaceable> [ OR ... ] }
+CREATE [ OR REPLACE ] [ CONSTRAINT ] TRIGGER <replaceable class="PARAMETER">name</replaceable> { BEFORE | AFTER | INSTEAD OF } { <replaceable class="PARAMETER">event</replaceable> [ OR ... ] }
ON <replaceable class="PARAMETER">table_name</replaceable>
[ FROM <replaceable class="parameter">referenced_table_name</replaceable> ]
[ NOT DEFERRABLE | [ DEFERRABLE ] [ INITIALLY IMMEDIATE | INITIALLY DEFERRED ] ]
@@ -43,11 +43,12 @@ CREATE [ CONSTRAINT ] TRIGGER <replaceable class="PARAMETER">name</replaceable>
<title>Description</title>
<para>
- <command>CREATE TRIGGER</command> creates a new trigger. The
+ <command>CREATE TRIGGER</command> creates a new trigger. The
trigger will be associated with the specified table, view, or foreign table
and will execute the specified
function <replaceable class="parameter">function_name</replaceable> when
- certain events occur.
+ certain events occur. <command>CREATE OR REPLACE TRIGGER</command> will
+ either create a new trigger, or replace an existing definition.
</para>
<para>
@@ -68,6 +69,17 @@ CREATE [ CONSTRAINT ] TRIGGER <replaceable class="PARAMETER">name</replaceable>
</para>
<para>
+ To replace the current definition of an existing trigger,
+ use <command>CREATE OR REPLACE TRIGGER</command>.
+ It is not possible to change <replaceable class="parameter">name</replaceable>
+ or <replaceable class="parameter">table_name</replaceable> this way
+ (if you tried, you would actually be creating a new, distinct trigger).
+ Also, <command>CREATE OR REPLACE TRIGGER</command> will not let you
+ change the type (regular/constraint) of an existing trigger.
+ To do that, you must drop and recreate the trigger.
+ </para>
+
+ <para>
A trigger that is marked <literal>FOR EACH ROW</literal> is called
once for every row that the operation modifies. For example, a
<command>DELETE</command> that affects 10 rows will cause any
diff --git a/src/backend/catalog/pg_constraint.c b/src/backend/catalog/pg_constraint.c
index 62be80d..96955fd 100644
--- a/src/backend/catalog/pg_constraint.c
+++ b/src/backend/catalog/pg_constraint.c
@@ -76,7 +76,7 @@ CreateConstraintEntry(const char *constraintName,
bool is_internal)
{
Relation conDesc;
- Oid conOid;
+ Oid conOid = InvalidOid;
HeapTuple tup;
bool nulls[Natts_pg_constraint];
Datum values[Natts_pg_constraint];
@@ -224,9 +224,70 @@ CreateConstraintEntry(const char *constraintName,
else
nulls[Anum_pg_constraint_consrc - 1] = true;
- tup = heap_form_tuple(RelationGetDescr(conDesc), values, nulls);
+ /*
+ * Since 'OR REPLACE' is supported in CREATE TRIGGER command,there are
+ * chances for replacing existing constraint table entry. So a new check
+ * is needed to know whether new constraint entry should be created
+ * or existing entry should be replaced.
+ */
+ if (constraintType == CONSTRAINT_TRIGGER &&
+ ConstraintNameIsUsed(CONSTRAINT_RELATION,
+ relId,
+ constraintNamespace,
+ constraintName))
+ {
+ SysScanDesc conscan;
+ ScanKeyData skey[2];
+ HeapTuple oldtup;
+
+ ScanKeyInit(&skey[0],
+ Anum_pg_constraint_conname,
+ BTEqualStrategyNumber, F_NAMEEQ,
+ CStringGetDatum(constraintName));
+
+ ScanKeyInit(&skey[1],
+ Anum_pg_constraint_connamespace,
+ BTEqualStrategyNumber, F_OIDEQ,
+ ObjectIdGetDatum(constraintNamespace));
+
+ conscan = systable_beginscan(conDesc, ConstraintNameNspIndexId, true,
+ NULL, 2, skey);
+
+ while (HeapTupleIsValid(oldtup = systable_getnext(conscan)))
+ {
+ Form_pg_constraint con = (Form_pg_constraint) GETSTRUCT(oldtup);
+
+ if (namestrcmp(&(con->conname), constraintName) == 0 &&
+ con->conrelid == relId)
+ {
- conOid = CatalogTupleInsert(conDesc, tup);
+ bool replaces[Natts_pg_constraint];
+ conOid = HeapTupleGetOid(oldtup);
+
+ memset(replaces, true, sizeof(replaces));
+ replaces[Anum_pg_constraint_conname - 1] = false;
+ replaces[Anum_pg_constraint_confrelid - 1] = false;
+
+ /* Modify the existing constraint entry */
+ tup = heap_modify_tuple(oldtup, RelationGetDescr(conDesc), values, nulls, replaces);
+ CatalogTupleUpdate(conDesc, &oldtup->t_self, tup);
+ heap_freetuple(tup);
+
+ /* Remove all old dependencies before registering new ones */
+ deleteDependencyRecordsFor(ConstraintRelationId, conOid, true);
+ break;
+ }
+ }
+
+ systable_endscan(conscan);
+
+ }
+ else
+ {
+ tup = heap_form_tuple(RelationGetDescr(conDesc), values, nulls);
+ conOid = CatalogTupleInsert(conDesc, tup);
+ heap_freetuple(tup);
+ }
conobject.classId = ConstraintRelationId;
conobject.objectId = conOid;
diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index d80bff6..2c02029 100644
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -166,6 +166,7 @@ CreateTrigger(CreateTrigStmt *stmt, const char *queryString,
referenced;
char *oldtablename = NULL;
char *newtablename = NULL;
+ bool is_update = false;
if (OidIsValid(relOid))
rel = heap_open(relOid, ShareRowExclusiveLock);
@@ -616,37 +617,6 @@ CreateTrigger(CreateTrigStmt *stmt, const char *queryString,
}
/*
- * Scan pg_trigger for existing triggers on relation. We do this only to
- * give a nice error message if there's already a trigger of the same
- * name. (The unique index on tgrelid/tgname would complain anyway.) We
- * can skip this for internally generated triggers, since the name
- * modification above should be sufficient.
- *
- * NOTE that this is cool only because we have ShareRowExclusiveLock on
- * the relation, so the trigger set won't be changing underneath us.
- */
- if (!isInternal)
- {
- ScanKeyInit(&key,
- Anum_pg_trigger_tgrelid,
- BTEqualStrategyNumber, F_OIDEQ,
- ObjectIdGetDatum(RelationGetRelid(rel)));
- tgscan = systable_beginscan(tgrel, TriggerRelidNameIndexId, true,
- NULL, 1, &key);
- while (HeapTupleIsValid(tuple = systable_getnext(tgscan)))
- {
- Form_pg_trigger pg_trigger = (Form_pg_trigger) GETSTRUCT(tuple);
-
- if (namestrcmp(&(pg_trigger->tgname), trigname) == 0)
- ereport(ERROR,
- (errcode(ERRCODE_DUPLICATE_OBJECT),
- errmsg("trigger \"%s\" for relation \"%s\" already exists",
- trigname, RelationGetRelationName(rel))));
- }
- systable_endscan(tgscan);
- }
-
- /*
* Build the new pg_trigger tuple.
*/
memset(nulls, false, sizeof(nulls));
@@ -765,17 +735,108 @@ CreateTrigger(CreateTrigStmt *stmt, const char *queryString,
else
nulls[Anum_pg_trigger_tgnewtable - 1] = true;
- tuple = heap_form_tuple(tgrel->rd_att, values, nulls);
-
- /* force tuple to have the desired OID */
- HeapTupleSetOid(tuple, trigoid);
-
/*
- * Insert tuple into pg_trigger.
+ * Scan pg_trigger for existing triggers on relation. We do this only to
+ * give a nice error message if there's already a trigger of the same
+ * name and no 'OR REPLACE' is specified. (The unique index on tgrelid/tgname
+ * would complain anyway.). And, in case of replace trigger,
+ * existing trigger definition will be updated.
+ * We can skip this for internally generated triggers,
+ * since the name modification above should be sufficient.
+
+ * NOTE that this is cool only because we have ShareRowExclusiveLock on
+ * the relation, so the trigger set won't be changing underneath us.
*/
- CatalogTupleInsert(tgrel, tuple);
- heap_freetuple(tuple);
+ if (!isInternal)
+ {
+ ScanKeyInit(&key,
+ Anum_pg_trigger_tgrelid,
+ BTEqualStrategyNumber, F_OIDEQ,
+ ObjectIdGetDatum(RelationGetRelid(rel)));
+ tgscan = systable_beginscan(tgrel, TriggerRelidNameIndexId, true,
+ NULL, 1, &key);
+ while (HeapTupleIsValid(tuple = systable_getnext(tgscan)))
+ {
+ Form_pg_trigger pg_trigger = (Form_pg_trigger) GETSTRUCT(tuple);
+
+ if (namestrcmp(&(pg_trigger->tgname), trigname) == 0)
+ {
+ /* If 'OR REPLACE' is specified, then update the existing pg_trigger tuple. */
+ if(stmt->replace)
+ {
+
+ bool constraint_exists;
+ bool replaces[Natts_pg_trigger];
+
+ constraint_exists = ConstraintNameIsUsed(CONSTRAINT_RELATION,
+ RelationGetRelid(rel),
+ RelationGetNamespace(rel),
+ stmt->trigname);
+ memset(replaces, true, sizeof(replaces));
+
+ /* Replacement between normal triggers and constraint triggers are restricted */
+ if((stmt->isconstraint && constraint_exists) ||
+ (!stmt->isconstraint && !constraint_exists))
+ {
+ HeapTuple newtup;
+ TupleDesc tupDesc;
+
+ tupDesc = RelationGetDescr(tgrel);
+ replaces[Anum_pg_trigger_tgrelid - 1] = false;
+ replaces[Anum_pg_trigger_tgname - 1] = false;
+ trigoid = HeapTupleGetOid(tuple);
+ newtup = heap_modify_tuple(tuple, tupDesc, values, nulls, replaces);
+
+ /* Update tuple in pg_trigger */
+ CatalogTupleUpdate(tgrel, &tuple->t_self, newtup);
+ heap_freetuple(newtup);
+ is_update = true;
+ break;
+ }
+ else
+ {
+ if(stmt->isconstraint)
+ ereport(ERROR,
+ (errcode(ERRCODE_DUPLICATE_OBJECT),
+ errmsg("Trigger \"%s\" for relation \"%s\" cannot be replaced with constraint trigger",
+ trigname, RelationGetRelationName(rel))));
+ else
+ ereport(ERROR,
+ (errcode(ERRCODE_DUPLICATE_OBJECT),
+ errmsg("Constraint trigger \"%s\" for relation \"%s\" cannot be replaced with non-constraint trigger",
+ trigname, RelationGetRelationName(rel))));
+ }
+ }
+ else
+ {
+ ereport(ERROR,
+ (errcode(ERRCODE_DUPLICATE_OBJECT),
+ errmsg("trigger \"%s\" for relation \"%s\" already exists",
+ trigname, RelationGetRelationName(rel))));
+ }
+ }
+ }
+ systable_endscan(tgscan);
+ }
+
+ if(!is_update)
+ {
+
+ tuple = heap_form_tuple(tgrel->rd_att, values, nulls);
+
+ /* force tuple to have the desired OID */
+ HeapTupleSetOid(tuple, trigoid);
+
+ /*
+ * Insert tuple into pg_trigger.
+ */
+ CatalogTupleInsert(tgrel, tuple);
+
+ heap_freetuple(tuple);
+
+ }
+
heap_close(tgrel, RowExclusiveLock);
pfree(DatumGetPointer(values[Anum_pg_trigger_tgname - 1]));
@@ -819,6 +880,15 @@ CreateTrigger(CreateTrigStmt *stmt, const char *queryString,
myself.objectId = trigoid;
myself.objectSubId = 0;
+ /*
+ * In case of replace trigger, trigger should no-more dependent on old
+ * referenced objects. Always remove the old dependencies and then register
+ * new ones.In that way, even if the old referenced object gets dropped,
+ * trigger will remain in the database.
+ */
+ if (is_update)
+ deleteDependencyRecordsFor(myself.classId, myself.objectId, true);
+
referenced.classId = ProcedureRelationId;
referenced.objectId = funcoid;
referenced.objectSubId = 0;
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index cf97be5..bc6183f 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -5118,48 +5118,50 @@ CreateAmStmt: CREATE ACCESS METHOD name TYPE_P INDEX HANDLER handler_name
*****************************************************************************/
CreateTrigStmt:
- CREATE TRIGGER name TriggerActionTime TriggerEvents ON
+ CREATE opt_or_replace TRIGGER name TriggerActionTime TriggerEvents ON
qualified_name TriggerReferencing TriggerForSpec TriggerWhen
EXECUTE PROCEDURE func_name '(' TriggerFuncArgs ')'
{
CreateTrigStmt *n = makeNode(CreateTrigStmt);
- n->trigname = $3;
- n->relation = $7;
- n->funcname = $13;
- n->args = $15;
- n->row = $9;
- n->timing = $4;
- n->events = intVal(linitial($5));
- n->columns = (List *) lsecond($5);
- n->whenClause = $10;
- n->transitionRels = $8;
+ n->replace = $2;
+ n->trigname = $4;
+ n->relation = $8;
+ n->funcname = $14;
+ n->args = $16;
+ n->row = $10;
+ n->timing = $5;
+ n->events = intVal(linitial($6));
+ n->columns = (List *) lsecond($6);
+ n->whenClause = $11;
+ n->transitionRels = $9;
n->isconstraint = FALSE;
n->deferrable = FALSE;
n->initdeferred = FALSE;
n->constrrel = NULL;
$$ = (Node *)n;
}
- | CREATE CONSTRAINT TRIGGER name AFTER TriggerEvents ON
+ | CREATE opt_or_replace CONSTRAINT TRIGGER name AFTER TriggerEvents ON
qualified_name OptConstrFromTable ConstraintAttributeSpec
FOR EACH ROW TriggerWhen
EXECUTE PROCEDURE func_name '(' TriggerFuncArgs ')'
{
CreateTrigStmt *n = makeNode(CreateTrigStmt);
- n->trigname = $4;
- n->relation = $8;
- n->funcname = $17;
- n->args = $19;
+ n->replace = $2;
+ n->trigname = $5;
+ n->relation = $9;
+ n->funcname = $18;
+ n->args = $20;
n->row = TRUE;
n->timing = TRIGGER_TYPE_AFTER;
- n->events = intVal(linitial($6));
- n->columns = (List *) lsecond($6);
- n->whenClause = $14;
+ n->events = intVal(linitial($7));
+ n->columns = (List *) lsecond($7);
+ n->whenClause = $15;
n->transitionRels = NIL;
n->isconstraint = TRUE;
- processCASbits($10, @10, "TRIGGER",
+ processCASbits($11, @11, "TRIGGER",
&n->deferrable, &n->initdeferred, NULL,
NULL, yyscanner);
- n->constrrel = $9;
+ n->constrrel = $10;
$$ = (Node *)n;
}
;
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index f355954..2c7be2e 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -853,7 +853,7 @@ pg_get_triggerdef_worker(Oid trigid, bool pretty)
initStringInfo(&buf);
tgname = NameStr(trigrec->tgname);
- appendStringInfo(&buf, "CREATE %sTRIGGER %s ",
+ appendStringInfo(&buf, "CREATE OR REPLACE %sTRIGGER %s ",
OidIsValid(trigrec->tgconstraint) ? "CONSTRAINT " : "",
quote_identifier(tgname));
diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl
index 242d3c0..f13ef67 100644
--- a/src/bin/pg_dump/t/002_pg_dump.pl
+++ b/src/bin/pg_dump/t/002_pg_dump.pl
@@ -1486,7 +1486,7 @@ qr/CREATE CAST \(timestamp with time zone AS interval\) WITH FUNCTION pg_catalog
FOR EACH ROW WHEN (NEW.col1 > 10)
EXECUTE PROCEDURE dump_test.trigger_func();',
regexp => qr/^
- \QCREATE TRIGGER test_trigger BEFORE INSERT ON test_table \E
+ \QCREATE OR REPLACE TRIGGER test_trigger BEFORE INSERT ON test_table \E
\QFOR EACH ROW WHEN ((new.col1 > 10)) \E
\QEXECUTE PROCEDURE trigger_func();\E
/xm,
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 07a8436..648f6b4 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -2250,6 +2250,7 @@ typedef struct CreateTrigStmt
List *columns; /* column names, or NIL for all columns */
Node *whenClause; /* qual expression, or NULL if none */
bool isconstraint; /* This is a constraint trigger */
+ bool replace; /* T => replace if already exists */
/* explicitly named transition data */
List *transitionRels; /* TriggerTransition nodes, or NIL if none */
/* The remaining fields are only used for constraint triggers */
diff --git a/src/test/regress/expected/triggers.out b/src/test/regress/expected/triggers.out
index f408475..0efb218 100644
--- a/src/test/regress/expected/triggers.out
+++ b/src/test/regress/expected/triggers.out
@@ -385,21 +385,21 @@ SELECT * FROM main_table ORDER BY a, b;
(8 rows)
SELECT pg_get_triggerdef(oid, true) FROM pg_trigger WHERE tgrelid = 'main_table'::regclass AND tgname = 'modified_a';
- pg_get_triggerdef
---------------------------------------------------------------------------------------------------------------------------------------------
- CREATE TRIGGER modified_a BEFORE UPDATE OF a ON main_table FOR EACH ROW WHEN (old.a <> new.a) EXECUTE PROCEDURE trigger_func('modified_a')
+ pg_get_triggerdef
+-------------------------------------------------------------------------------------------------------------------------------------------------------
+ CREATE OR REPLACE TRIGGER modified_a BEFORE UPDATE OF a ON main_table FOR EACH ROW WHEN (old.a <> new.a) EXECUTE PROCEDURE trigger_func('modified_a')
(1 row)
SELECT pg_get_triggerdef(oid, false) FROM pg_trigger WHERE tgrelid = 'main_table'::regclass AND tgname = 'modified_a';
- pg_get_triggerdef
-----------------------------------------------------------------------------------------------------------------------------------------------
- CREATE TRIGGER modified_a BEFORE UPDATE OF a ON main_table FOR EACH ROW WHEN ((old.a <> new.a)) EXECUTE PROCEDURE trigger_func('modified_a')
+ pg_get_triggerdef
+---------------------------------------------------------------------------------------------------------------------------------------------------------
+ CREATE OR REPLACE TRIGGER modified_a BEFORE UPDATE OF a ON main_table FOR EACH ROW WHEN ((old.a <> new.a)) EXECUTE PROCEDURE trigger_func('modified_a')
(1 row)
SELECT pg_get_triggerdef(oid, true) FROM pg_trigger WHERE tgrelid = 'main_table'::regclass AND tgname = 'modified_any';
- pg_get_triggerdef
---------------------------------------------------------------------------------------------------------------------------------------------------------------
- CREATE TRIGGER modified_any BEFORE UPDATE OF a ON main_table FOR EACH ROW WHEN (old.* IS DISTINCT FROM new.*) EXECUTE PROCEDURE trigger_func('modified_any')
+ pg_get_triggerdef
+-------------------------------------------------------------------------------------------------------------------------------------------------------------------------
+ CREATE OR REPLACE TRIGGER modified_any BEFORE UPDATE OF a ON main_table FOR EACH ROW WHEN (old.* IS DISTINCT FROM new.*) EXECUTE PROCEDURE trigger_func('modified_any')
(1 row)
DROP TRIGGER modified_a ON main_table;
@@ -421,9 +421,9 @@ FOR EACH STATEMENT EXECUTE PROCEDURE trigger_func('before_upd_a_stmt');
CREATE TRIGGER after_upd_b_stmt_trig AFTER UPDATE OF b ON main_table
FOR EACH STATEMENT EXECUTE PROCEDURE trigger_func('after_upd_b_stmt');
SELECT pg_get_triggerdef(oid) FROM pg_trigger WHERE tgrelid = 'main_table'::regclass AND tgname = 'after_upd_a_b_row_trig';
- pg_get_triggerdef
--------------------------------------------------------------------------------------------------------------------------------------------
- CREATE TRIGGER after_upd_a_b_row_trig AFTER UPDATE OF a, b ON main_table FOR EACH ROW EXECUTE PROCEDURE trigger_func('after_upd_a_b_row')
+ pg_get_triggerdef
+------------------------------------------------------------------------------------------------------------------------------------------------------
+ CREATE OR REPLACE TRIGGER after_upd_a_b_row_trig AFTER UPDATE OF a, b ON main_table FOR EACH ROW EXECUTE PROCEDURE trigger_func('after_upd_a_b_row')
(1 row)
UPDATE main_table SET a = 50;
@@ -1674,6 +1674,156 @@ drop table self_ref_trigger;
drop function self_ref_trigger_ins_func();
drop function self_ref_trigger_del_func();
--
+-- test triggers with OR REPLACE clause
+--
+CREATE TABLE my_table1 (id integer, name text);
+CREATE TABLE my_table2 (id integer);
+CREATE FUNCTION my_proc1() RETURNS trigger AS $$
+begin
+ RETURN NULL;
+end;$$ LANGUAGE plpgsql;
+CREATE FUNCTION my_updateproc1() RETURNS trigger AS $$
+begin
+ UPDATE my_table2 SET id=NEW.id WHERE id=OLD.id;
+ RETURN NULL;
+end;$$ LANGUAGE plpgsql;
+CREATE FUNCTION my_deleteproc1() RETURNS trigger AS $$
+begin
+ DELETE FROM my_table2 WHERE id=OLD.id;
+ RETURN NULL;
+end;$$ LANGUAGE plpgsql;
+CREATE FUNCTION my_insertproc1() RETURNS trigger AS $$
+begin
+ INSERT INTO my_table2 VALUES(NEW.id);
+ RETURN NULL;
+end;$$ LANGUAGE plpgsql;
+INSERT INTO my_table1 VALUES(323, 'Alex');
+INSERT INTO my_table1 VALUES(23, 'Teddy');
+INSERT INTO my_table1 VALUES(38, 'Bob');
+INSERT INTO my_table2 VALUES(323);
+INSERT INTO my_table2 VALUES(23);
+INSERT INTO my_table2 VALUES(38);
+--With OR REPALCE clause, create a new trigger with the name my_regular_trigger.
+CREATE OR REPLACE TRIGGER my_regular_trigger AFTER DELETE ON my_table1
+FOR EACH ROW
+EXECUTE PROCEDURE my_deleteproc1();
+DELETE FROM my_table1 WHERE id=323;
+SELECT * FROM my_table1;
+ id | name
+----+-------
+ 23 | Teddy
+ 38 | Bob
+(2 rows)
+
+SELECT * FROM my_table2;
+ id
+----
+ 23
+ 38
+(2 rows)
+
+--With OR REPALCE clause, create a new constraint trigger with the name my_constraint_trigger.
+CREATE OR REPLACE CONSTRAINT TRIGGER my_constraint_trigger AFTER DELETE ON my_table1
+FOR EACH ROW
+EXECUTE PROCEDURE my_deleteproc1();
+DELETE FROM my_table1 WHERE id=23;
+SELECT * FROM my_table1;
+ id | name
+----+------
+ 38 | Bob
+(1 row)
+
+SELECT * FROM my_table2;
+ id
+----
+ 38
+(1 row)
+
+-- Replace my_regular_trigger definition with new definition.
+CREATE OR REPLACE TRIGGER my_regular_trigger AFTER INSERT ON my_table1
+FOR EACH ROW
+EXECUTE PROCEDURE my_insertproc1();
+INSERT INTO my_table1 VALUES(323, 'Alex');
+SELECT * FROM my_table1;
+ id | name
+-----+------
+ 38 | Bob
+ 323 | Alex
+(2 rows)
+
+SELECT * FROM my_table2;
+ id
+-----
+ 38
+ 323
+(2 rows)
+
+-- Replace my_constraint_trigger definition with new definition.
+CREATE OR REPLACE CONSTRAINT TRIGGER my_constraint_trigger AFTER INSERT ON my_table1
+DEFERRABLE INITIALLY DEFERRED
+FOR EACH ROW
+EXECUTE PROCEDURE my_insertproc1();
+INSERT INTO my_table1 VALUES(23, 'Teddy');
+SELECT * FROM my_table1;
+ id | name
+-----+-------
+ 38 | Bob
+ 323 | Alex
+ 23 | Teddy
+(3 rows)
+
+SELECT * FROM my_table2;
+ id
+-----
+ 38
+ 323
+ 23
+ 23
+(4 rows)
+
+--Without OR REPALCE clause, Cannot create a new constraint trigger with same name of regular trigger already exisiting.
+CREATE CONSTRAINT TRIGGER my_regular_trigger AFTER DELETE ON my_table1
+FOR EACH ROW
+EXECUTE PROCEDURE my_proc1();
+ERROR: trigger "my_regular_trigger" for relation "my_table1" already exists
+--With OR REPALCE clause, Cannot repalce a regular trigger with a constraint trigger.
+CREATE OR REPLACE CONSTRAINT TRIGGER my_regular_trigger AFTER DELETE ON my_table1
+FOR EACH ROW
+EXECUTE PROCEDURE my_proc1();
+ERROR: Trigger "my_regular_trigger" for relation "my_table1" cannot be replaced with constraint trigger
+--Without OR REPALCE clause, Cannot create a new constraint trigger with same name of constraint trigger already exisiting.
+CREATE TRIGGER my_constraint_trigger AFTER DELETE ON my_table1
+FOR EACH ROW
+EXECUTE PROCEDURE my_proc1();
+ERROR: trigger "my_constraint_trigger" for relation "my_table1" already exists
+--With OR REPALCE clause, Cannot replace a regular triiger with a constraint trigger.
+CREATE OR REPLACE TRIGGER my_constraint_trigger AFTER DELETE ON my_table1
+FOR EACH ROW
+EXECUTE PROCEDURE my_proc1();
+ERROR: Constraint trigger "my_constraint_trigger" for relation "my_table1" cannot be replaced with non-constraint trigger
+--Without OR REPALCE clause, cannot replace my_regular_trigger definition with new definition.
+CREATE TRIGGER my_regular_trigger AFTER DELETE ON my_table1
+EXECUTE PROCEDURE my_proc1();
+ERROR: trigger "my_regular_trigger" for relation "my_table1" already exists
+--Without OR REPALCE clause, cannot replace my_constraint_trigger definition with new definition.
+CREATE CONSTRAINT TRIGGER my_constraint_trigger AFTER DELETE ON my_table1
+FOR EACH ROW
+EXECUTE PROCEDURE my_proc1();
+ERROR: trigger "my_constraint_trigger" for relation "my_table1" already exists
+-- Delete the dependency when replaces are executed.
+CREATE OR REPLACE TRIGGER my_trigger2 AFTER UPDATE OF NAME ON my_table1
+FOR EACH ROW
+EXECUTE PROCEDURE my_updateproc1();
+CREATE OR REPLACE TRIGGER my_trigger2 AFTER DELETE ON my_table1
+EXECUTE PROCEDURE my_proc1();
+ALTER TABLE my_table1 DROP COLUMN name;
+DROP FUNCTION my_updateproc1();
+DROP TABLE my_table1;
+DROP TABLE my_table2;
+DROP FUNCTION my_deleteproc1();
+DROP FUNCTION my_insertproc1();
+DROP FUNCTION my_proc1();
+--
-- Verify behavior of before and after triggers with INSERT...ON CONFLICT
-- DO UPDATE
--
diff --git a/src/test/regress/sql/triggers.sql b/src/test/regress/sql/triggers.sql
index b6de1b3..7c5e228 100644
--- a/src/test/regress/sql/triggers.sql
+++ b/src/test/regress/sql/triggers.sql
@@ -1182,6 +1182,121 @@ select * from self_ref_trigger;
drop table self_ref_trigger;
drop function self_ref_trigger_ins_func();
drop function self_ref_trigger_del_func();
+--
+-- test triggers with OR REPLACE clause
+--
+
+CREATE TABLE my_table1 (id integer, name text);
+CREATE TABLE my_table2 (id integer);
+CREATE FUNCTION my_proc1() RETURNS trigger AS $$
+begin
+ RETURN NULL;
+end;$$ LANGUAGE plpgsql;
+CREATE FUNCTION my_updateproc1() RETURNS trigger AS $$
+begin
+ UPDATE my_table2 SET id=NEW.id WHERE id=OLD.id;
+ RETURN NULL;
+end;$$ LANGUAGE plpgsql;
+CREATE FUNCTION my_deleteproc1() RETURNS trigger AS $$
+begin
+ DELETE FROM my_table2 WHERE id=OLD.id;
+ RETURN NULL;
+end;$$ LANGUAGE plpgsql;
+CREATE FUNCTION my_insertproc1() RETURNS trigger AS $$
+begin
+ INSERT INTO my_table2 VALUES(NEW.id);
+ RETURN NULL;
+end;$$ LANGUAGE plpgsql;
+INSERT INTO my_table1 VALUES(323, 'Alex');
+INSERT INTO my_table1 VALUES(23, 'Teddy');
+INSERT INTO my_table1 VALUES(38, 'Bob');
+INSERT INTO my_table2 VALUES(323);
+INSERT INTO my_table2 VALUES(23);
+INSERT INTO my_table2 VALUES(38);
+
+--With OR REPALCE clause, create a new trigger with the name my_regular_trigger.
+CREATE OR REPLACE TRIGGER my_regular_trigger AFTER DELETE ON my_table1
+FOR EACH ROW
+EXECUTE PROCEDURE my_deleteproc1();
+
+DELETE FROM my_table1 WHERE id=323;
+SELECT * FROM my_table1;
+SELECT * FROM my_table2;
+
+--With OR REPALCE clause, create a new constraint trigger with the name my_constraint_trigger.
+CREATE OR REPLACE CONSTRAINT TRIGGER my_constraint_trigger AFTER DELETE ON my_table1
+FOR EACH ROW
+EXECUTE PROCEDURE my_deleteproc1();
+
+DELETE FROM my_table1 WHERE id=23;
+SELECT * FROM my_table1;
+SELECT * FROM my_table2;
+
+-- Replace my_regular_trigger definition with new definition.
+CREATE OR REPLACE TRIGGER my_regular_trigger AFTER INSERT ON my_table1
+FOR EACH ROW
+EXECUTE PROCEDURE my_insertproc1();
+
+INSERT INTO my_table1 VALUES(323, 'Alex');
+SELECT * FROM my_table1;
+SELECT * FROM my_table2;
+
+-- Replace my_constraint_trigger definition with new definition.
+CREATE OR REPLACE CONSTRAINT TRIGGER my_constraint_trigger AFTER INSERT ON my_table1
+DEFERRABLE INITIALLY DEFERRED
+FOR EACH ROW
+EXECUTE PROCEDURE my_insertproc1();
+
+INSERT INTO my_table1 VALUES(23, 'Teddy');
+SELECT * FROM my_table1;
+SELECT * FROM my_table2;
+
+--Without OR REPALCE clause, Cannot create a new constraint trigger with same name of regular trigger already exisiting.
+CREATE CONSTRAINT TRIGGER my_regular_trigger AFTER DELETE ON my_table1
+FOR EACH ROW
+EXECUTE PROCEDURE my_proc1();
+
+--With OR REPALCE clause, Cannot repalce a regular trigger with a constraint trigger.
+CREATE OR REPLACE CONSTRAINT TRIGGER my_regular_trigger AFTER DELETE ON my_table1
+FOR EACH ROW
+EXECUTE PROCEDURE my_proc1();
+
+--Without OR REPALCE clause, Cannot create a new constraint trigger with same name of constraint trigger already exisiting.
+CREATE TRIGGER my_constraint_trigger AFTER DELETE ON my_table1
+FOR EACH ROW
+EXECUTE PROCEDURE my_proc1();
+
+--With OR REPALCE clause, Cannot replace a regular triiger with a constraint trigger.
+CREATE OR REPLACE TRIGGER my_constraint_trigger AFTER DELETE ON my_table1
+FOR EACH ROW
+EXECUTE PROCEDURE my_proc1();
+
+--Without OR REPALCE clause, cannot replace my_regular_trigger definition with new definition.
+CREATE TRIGGER my_regular_trigger AFTER DELETE ON my_table1
+EXECUTE PROCEDURE my_proc1();
+
+--Without OR REPALCE clause, cannot replace my_constraint_trigger definition with new definition.
+CREATE CONSTRAINT TRIGGER my_constraint_trigger AFTER DELETE ON my_table1
+FOR EACH ROW
+EXECUTE PROCEDURE my_proc1();
+
+-- Delete the dependency when replaces are executed.
+CREATE OR REPLACE TRIGGER my_trigger2 AFTER UPDATE OF NAME ON my_table1
+FOR EACH ROW
+EXECUTE PROCEDURE my_updateproc1();
+
+CREATE OR REPLACE TRIGGER my_trigger2 AFTER DELETE ON my_table1
+EXECUTE PROCEDURE my_proc1();
+
+ALTER TABLE my_table1 DROP COLUMN name;
+DROP FUNCTION my_updateproc1();
+
+DROP TABLE my_table1;
+DROP TABLE my_table2;
+DROP FUNCTION my_deleteproc1();
+DROP FUNCTION my_insertproc1();
+DROP FUNCTION my_proc1();
+
--
-- Verify behavior of before and after triggers with INSERT...ON CONFLICT
On Tue, Feb 7, 2017 at 3:11 AM, Okano, Naoki <okano.naoki@jp.fujitsu.com> wrote:
On Wednesday, November 16, 2016 4:31 PM Okano Naoki wrote:
But in any case it would be a serious mistake to do this without first
implementing CREATE OR REPLACE TRIGGER. I think that's an entirely separate
proposal and you would be well advised to treat it as such.I see. There are more problems than I expected...
Let me start with 'OR REPLACE' clause.I tried to cretae a patch for CREATE OR REPLACE TRIGGER.
An example of execution is shown below.example)
1.define a new trigger
CREATE TRIGGER regular_trigger
AFTER UPDATE ON table_name
REFERENCING OLD TABLE AS oldtable_1 NEW TABLE AS newtable_1
FOR EACH STATEMENT
EXECUTE PROCEDURE func_1();
2.redinfe a trigger in single command
CREATE OR REPLACE TRIGGER regular_trigger
AFTER UPDATE OR DELETE ON table_name
REFERENCING OLD TABLE AS oldtable_2 NEW TABLE AS newtable_2
FOR EACH ROW
EXECUTE PROCEDURE func_2();If the named trigger does not exist.
a new trigger is also created by using OR REPLACE clause.
A regular trigger cannot be replaced by a constraint trigger and vice varsa.
because a constraint trigger has a different role from regular triger.Please give me feedback.
You should add this to the next CommitFest so it doesn't get missed.
https://commitfest.postgresql.org/
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Thursday, February 9, 2017 4:41 AM Robert Haas wrote:
You should add this to the next CommitFest so it doesn't get missed.
Thank you for your kind advice!
I have added this patch to the CommitFest 2017-03.
Regards,
Okano Naoki
Fujitsu
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 11/14/16 8:51 PM, Tom Lane wrote:
1. The impression I have is that most people write trigger functions
so that they can be shared across multiple uses. That'd be impossible
with anonymous trigger function blocks. So you'd be trading off merging
two commands into one, versus having to write out the whole body of the
trigger multiple times, which wouldn't be much of a win.
I've fairly frequently needed one-off triggers, usually to do some kind
of data validation that wasn't suitable in a CHECK constraint. Enough so
that twice now[1] I've created generic trigger functions that allow you
to specify an arbitrary expression to test against NEW and OLD.
1: Second, WIP example:
https://github.com/decibel/tg_sanity/blob/master/sql/tg_sanity.sql
--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com
855-TREBLE2 (855-873-2532)
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 2/7/17 03:11, Okano, Naoki wrote:
I tried to cretae a patch for CREATE OR REPLACE TRIGGER.
I have a feeling that this was proposed a few times in the ancient past
but did not go through because of locking issues. I can't find any
emails about it through. Does anyone remember? Have you thought about
locking issues?
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Peter Eisentraut wrote:
I have a feeling that this was proposed a few times in the ancient past
but did not go through because of locking issues. I can't find any
emails about it through. Does anyone remember? Have you thought about
locking issues?
Is this e-mail you are finding?
/messages/by-id/20140916124537.GH25887@awork2.anarazel.de
I am considering to add 'OR REPLACE' clause as a first step.
At least, I think there is no need to change the locking level when replacing a trigger with 'EXECUTE PROCEDURE' clause.
In PostgreSQL, we currently have ShareRowExclusiveLock lock on relation on which trigger is created. ShareRowExclusiveLock is enough to replace a trigger.
Also, we currently have RowExclusiveLock on pg_trigger. RowExclusiveLock is enough to replace a trigger, too.
Regards,
Okano Naoki
Fujitsu
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 3/8/17 04:12, Okano, Naoki wrote:
Peter Eisentraut wrote:
I have a feeling that this was proposed a few times in the ancient past
but did not go through because of locking issues. I can't find any
emails about it through. Does anyone remember? Have you thought about
locking issues?Is this e-mail you are finding?
/messages/by-id/20140916124537.GH25887@awork2.anarazel.de
No, that's not the one I had in mind.
I am considering to add 'OR REPLACE' clause as a first step.
At least, I think there is no need to change the locking level when replacing a trigger with 'EXECUTE PROCEDURE' clause.
In PostgreSQL, we currently have ShareRowExclusiveLock lock on relation on which trigger is created. ShareRowExclusiveLock is enough to replace a trigger.
Also, we currently have RowExclusiveLock on pg_trigger. RowExclusiveLock is enough to replace a trigger, too.
I'm not saying it's not correct. I was just wondering.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Peter Eisentraut wrote:
On 3/8/17 04:12, Okano, Naoki wrote:
Peter Eisentraut wrote:
I have a feeling that this was proposed a few times in the ancient past
but did not go through because of locking issues. I can't find any
emails about it through. Does anyone remember? Have you thought about
locking issues?Is this e-mail you are finding?
/messages/by-id/20140916124537.GH25887@awork2.anarazel.deNo, that's not the one I had in mind.
I see. But I could only find it.
Would anyone know e-mails discussed about locking issues?
I am considering to add 'OR REPLACE' clause as a first step.
At least, I think there is no need to change the locking level when replacing a trigger with 'EXECUTE PROCEDURE' clause.
In PostgreSQL, we currently have ShareRowExclusiveLock lock on relation on which trigger is created. ShareRowExclusiveLock is enough to replace a trigger.
Also, we currently have RowExclusiveLock on pg_trigger. RowExclusiveLock is enough to replace a trigger, too.I'm not saying it's not correct. I was just wondering.
Thank you for your opinion!
I think we do not need to change locking level in this case.
If someone notice a mistake in my understanding, please point out it.
Regards,
Okano Naoki
Fujitsu
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
"Okano, Naoki" <okano.naoki@jp.fujitsu.com> writes:
Peter Eisentraut wrote:
I have a feeling that this was proposed a few times in the ancient past
but did not go through because of locking issues. I can't find any
emails about it through. Does anyone remember? Have you thought about
locking issues?
Is this e-mail you are finding?
/messages/by-id/20140916124537.GH25887@awork2.anarazel.de
No, that's not the one I had in mind.
That's still a thread well worth studying in detail. It does touch on
locking issues, in that it points out that we allow you to replace a
trigger function's body with CREATE OR REPLACE FUNCTION with no lock at
all on the relation(s) it's a trigger for. Even with very lax assumptions
about what lock level CREATE OR REPLACE TRIGGER needs, it can't match
"none". Now you could certainly argue that we're not being very safe by
allowing trigger functions to be changed that way, but that's the current
state of affairs.
Another thread that you really need to absorb in its entirety is
/messages/by-id/5447578C.2050807@proxel.se
and you might also want to read the older threads that Robert Haas
links to early in that thread.
The locking-related point that strikes me most forcefully in that thread
is the concerns about whether a concurrent pg_dump run will produce a
consistent view of the table's triggers. This is problematic mainly
because pg_dump itself will see only the catalog entries that were current
when it started, but it relies heavily on ruleutils.c which will tend to
see committed changes immediately. Now, the existing behavior here is
probably not at all perfect, but that doesn't mean it's okay to make
things worse with CREATE OR REPLACE TRIGGER. A conservative conclusion
would be that C.O.R.T. needs to take AccessExclusiveLock so that it
can't run in parallel with pg_dump. Maybe that can be relaxed but it
requires some study. (CREATE TRIGGER doesn't have this issue because
pg_dump wouldn't see the new trigger at all and thus would never ask
ruleutils about it.)
Even if you don't care about pg_dump, I think that the question of whether
concurrent DML operations would always see (and act upon) instantaneously-
consistent versions of a table's trigger state is worth worrying about.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
I am sending the review of this patch
I found the following
v Use <optional> tage in documentation
v Don’t modified existing test case add new one instead
v Comment in pg_constraint.c is extended make it short
v Error message can be more guider if it tells about general rule
v Wrong result in concurrency case
Step to produce the result
1. build with with --enable-cassert and with
-DCATCACHE_FORCE_RELEASE=1
2. Run these commands as setup:
CREATE TABLE my_table1 (id integer, name text);
CREATE TABLE my_table2 (id integer);
3. CREATE FUNCTION my_deleteproc1() RETURNS trigger AS $$
begin
DELETE FROM my_table2 WHERE id=OLD.id;
RETURN NULL;
end;$$ LANGUAGE plpgsql;
4. INSERT INTO my_table1 VALUES(323, 'Alex');
INSERT INTO my_table1 VALUES(23, 'Teddy');
INSERT INTO my_table1 VALUES(38, 'Bob');
INSERT INTO my_table2 VALUES(323);
INSERT INTO my_table2 VALUES(23);
INSERT INTO my_table2 VALUES(38);
5. CREATE OR REPLACE TRIGGER my_regular_trigger AFTER DELETE ON
my_table1
FOR EACH ROW
EXECUTE PROCEDURE my_deleteproc1();
6. Attach a debugger to your session set a breakpoint at
ExecARDeleteTriggers
7. Run this in your session
DELETE FROM my_table1 WHERE id=323;
8. start another session and run:
CREATE OR REPLACE TRIGGER my_regular_trigger
AFTER INSERT ON my_table1
FOR EACH ROW
EXECUTE PROCEDURE my_deleteproc1();
9. exite the debugger to release the first session
and the result
postgres=# SELECT * FROM my_table1;
id | name
----+-------
23 | Teddy
38 | Bob
(2 rows)
postgres=# SELECT * FROM my_table2;
id
-----
323
23
38
(3 rows)
Id number 323 should not be there in my_table2;
Regards
Surafel
Hi Naoki,
On 3/17/17 11:58 AM, Surafel Temesgen wrote:
I am sending the review of this patch
This thread has been idle for a week. Please respond and/or post a new
patch by 2017-03-28 00:00 AoE (UTC-12) or this submission will be marked
"Returned with Feedback".
--
-David
david@pgmasters.net
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Hi Surafel,
Thank you for your review!
But I have not finish fixing a patch yet.
v Use <optional> tage in documentation
ok.
v Don’t modified existing test case add new one instead
These existing tests I modified are the results of commands "SELECT
pg_get_triggerdef(oid, true) ... ".
I modified them in order to match with pg_get_functiondef().
The keyword 'OR REPLACE' is always added in CREATE FUNCTION.
Does not it make sense?
v Comment in pg_constraint.c is extended make it short
v Error message can be more guider if it tells about general rule
ok.
v Wrong result in concurrency case
Thank you for the detailed example case. I appreciate it.
The issue does not seem to be resolved easily because I have to
modify pg_get_constraintdef_worker() in ruleutils.c.
And it takes some more time to modify pg_get_constraintdef_worker().
So, I am moving this patch to "Returned with Feedback".
Regards,
Okano Naoki
Fujitsu
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers