Hi,

BTW, the Postgres convention is to submit patches in context diff format (diff -c).

Satoshi Nagayasu wrote:
+       while (HeapTupleIsValid(tuple = systable_getnext(tgscan)))
+       {
+               HeapTuple newtup = heap_copytuple(tuple);
+               Form_pg_trigger pg_trigger = (Form_pg_trigger) 
GETSTRUCT(newtup);
+
+               if ( pg_trigger->tgenabled==true && enable==false )
+               {
+                       pg_trigger->tgenabled = false;
+               }
+               else if ( pg_trigger->tgenabled==false && enable==true )
+               {
+                       pg_trigger->tgenabled = true;
+               }
+
+               simple_heap_update(tgrel, &newtup->t_self, newtup);

Wouldn't it just be easier to set pg_trigger->tgenabled = enable ?

Also, you should probably skip the simple_heap_update() if the user tries to disable an already-disabled trigger, to avoid pointless MVCC bloat (and same for enabling an already-enabled trigger, of course).

--- pgsql.orig/src/backend/parser/gram.y        2005-06-30 05:34:13.000000000 
+0900
+++ pgsql/src/backend/parser/gram.y     2005-07-01 14:21:25.000000000 +0900
@@ -348,9 +348,9 @@
DATABASE DAY_P DEALLOCATE DEC DECIMAL_P DECLARE DEFAULT DEFAULTS
        DEFERRABLE DEFERRED DEFINER DELETE_P DELIMITER DELIMITERS
-       DESC DISTINCT DO DOMAIN_P DOUBLE_P DROP
+       DESC DISABLE DISTINCT DO DOMAIN_P DOUBLE_P DROP
- EACH ELSE ENCODING ENCRYPTED END_P ESCAPE EXCEPT EXCLUDING
+       EACH ELSE ENABLE ENCODING ENCRYPTED END_P ESCAPE EXCEPT EXCLUDING
        EXCLUSIVE EXECUTE EXISTS EXPLAIN EXTERNAL EXTRACT

You also need to add these to unreserved_keywords (see gram.y:7903).

--- pgsql.orig/src/include/commands/trigger.h   2005-05-30 16:20:58.000000000 
+0900
+++ pgsql/src/include/commands/trigger.h        2005-07-01 15:50:09.000000000 
+0900
@@ -164,6 +164,7 @@
extern void AfterTriggerSetState(ConstraintsSetStmt *stmt); +void EnableDisableTrigger(Relation rel, const char *tgname, bool enable);

The Postgres convention is to use the "extern" keyword in declarations of global functions.

If someone pg_dump's a table with a disabled trigger, should the dump enable or disable the trigger? I'd be inclined to say pg_dump should preserve the state of the database it is dumping, and so the trigger should be disabled. In that case I believe pg_dump needs to be updated.

The patch needs to update the documentation and add regression tests. psql tab completion might also be worth adding.

-Neil

---------------------------(end of broadcast)---------------------------
TIP 2: you can get off all lists at once with the unregister command
   (send "unregister YourEmailAddressHere" to [EMAIL PROTECTED])

Reply via email to