Thread: enable/disable trigger (Re: Fwd: [HACKERS] Open items)

enable/disable trigger (Re: Fwd: [HACKERS] Open items)

From
Satoshi Nagayasu
Date:
Hi all,

Here is a first patch to allow these commands.

> ALTER TABLE <table> ENABLE TRIGGER <trigname>
> ALTER TABLE <table> DISABLE TRIGGER <trigname>

Bruce said to allow them only super-user,
but currently this patch allows also the table owner.

If we need to restrict these operations,
we have to add more user checks.

> From: Bruce Momjian <pgman@candle.pha.pa.us>
> Date: 2005/06/29 20:49
> Subject: Re: [HACKERS] Open items
> To: Satoshi Nagayasu <nagayasus@nttdata.co.jp>
> Cc: PostgreSQL-development <pgsql-hackers@postgresql.org>
>
>
> Satoshi Nagayasu wrote:
>
>>How about enable/disable triggers?
>>
>>>From TODO:
>>
>>>Allow triggers to be disabled.
>>
>>http://momjian.postgresql.org/cgi-bin/pgtodo?trigger
>>
>>I think this is good for COPY performance improvement.
>>
>>Now I have user functions to enable/disable triggers, not DDL.
>>It modifies system tables.
>>But I can rewrite this as a DDL. (ALTER TABLE?)
>
>
> Yea, it is a TODO item, and should be pretty straight-forward to code,
> so sure, go ahead.
>
> It has to be something that is super-user-only.
>
> --
>  Bruce Momjian                        |  http://candle.pha.pa.us
>  pgman@candle.pha.pa.us               |  (610) 359-1001
>  +  If your life is a hard drive,     |  13 Roberts Road
>  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073
>
> ---------------------------(end of broadcast)---------------------------
> TIP 4: Don't 'kill -9' the postmaster
>


--
NAGAYASU Satoshi <nagayasus@nttdata.co.jp>
diff -ru pgsql.orig/src/backend/commands/tablecmds.c pgsql/src/backend/commands/tablecmds.c
--- pgsql.orig/src/backend/commands/tablecmds.c    2005-06-28 14:08:54.000000000 +0900
+++ pgsql/src/backend/commands/tablecmds.c    2005-07-01 15:50:27.000000000 +0900
@@ -236,6 +236,8 @@
                                               Oid newOwnerId);
 static void ATExecClusterOn(Relation rel, const char *indexName);
 static void ATExecDropCluster(Relation rel);
+static void ATExecEnableDisableTrigger(Relation rel, char *trigname,
+                                       bool enable);
 static void ATPrepSetTableSpace(AlteredTableInfo *tab, Relation rel,
                     char *tablespacename);
 static void ATExecSetTableSpace(Oid tableOid, Oid newTableSpace);
@@ -1993,6 +1995,11 @@
             }
             pass = AT_PASS_DROP;
             break;
+        case AT_EnableTrig:        /* ENABLE TRIGGER */
+        case AT_DisableTrig:    /* DISABLE TRIGGER */
+            ATSimplePermissions(rel, false);
+            pass = AT_PASS_MISC;
+            break;
         case AT_SetTableSpace:    /* SET TABLESPACE */
             /* This command never recurses */
             ATPrepSetTableSpace(tab, rel, cmd->name);
@@ -2155,6 +2162,12 @@
              * Nothing to do here; Phase 3 does the work
              */
             break;
+        case AT_EnableTrig:        /* ENABLE TRIGGER */
+            ATExecEnableDisableTrigger(rel, cmd->name, true);
+            break;
+        case AT_DisableTrig:    /* DISABLE TRIGGER */
+            ATExecEnableDisableTrigger(rel, cmd->name, false);
+            break;
         default:                /* oops */
             elog(ERROR, "unrecognized alter table type: %d",
                  (int) cmd->subtype);
@@ -5465,6 +5478,15 @@
 }

 /*
+ * ALTER TABLE ENABLE/DISABLE TRIGGER
+ */
+static void
+ATExecEnableDisableTrigger(Relation rel, char *trigname, bool enable)
+{
+    EnableDisableTrigger(rel, trigname, enable);
+}
+
+/*
  * ALTER TABLE SET TABLESPACE
  */
 static void
diff -ru pgsql.orig/src/backend/commands/trigger.c pgsql/src/backend/commands/trigger.c
--- pgsql.orig/src/backend/commands/trigger.c    2005-05-30 16:20:58.000000000 +0900
+++ pgsql/src/backend/commands/trigger.c    2005-07-01 15:53:45.000000000 +0900
@@ -3063,3 +3063,74 @@
         afterTriggerAddEvent(new_event);
     }
 }
+
+/* ----------
+ * EnableDisableTrigger()
+ *
+ *    Called by ALTER TABLE ENABLE/DISABLE TRIGGER
+ *  to change 'tgenabled' flag in the pg_trigger.
+ * ----------
+ */
+void
+EnableDisableTrigger(Relation rel, const char *tgname, bool enable)
+{
+    Relation tgrel;
+    SysScanDesc tgscan;
+    ScanKeyData keys[2];
+    HeapTuple tuple;
+
+    /* Permissions checks */
+    if (!pg_class_ownercheck(RelationGetRelid(rel), GetUserId()))
+        aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_CLASS,
+                       RelationGetRelationName(rel));
+
+    if (!allowSystemTableMods && IsSystemRelation(rel))
+        ereport(ERROR,
+                (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+                 errmsg("permission denied: \"%s\" is a system catalog",
+                        RelationGetRelationName(rel))));
+
+    tgrel = heap_open(TriggerRelationId, RowExclusiveLock);
+
+    ScanKeyInit(&keys[0],
+                Anum_pg_trigger_tgrelid,
+                BTEqualStrategyNumber, F_OIDEQ,
+                ObjectIdGetDatum(RelationGetRelid(rel)));
+    ScanKeyInit(&keys[1],
+                Anum_pg_trigger_tgname,
+                BTEqualStrategyNumber, F_NAMEEQ,
+                CStringGetDatum(tgname));
+
+    tgscan = systable_beginscan(tgrel, TriggerRelidNameIndexId, true,
+                                SnapshotNow, 1, keys);
+
+    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);
+
+        /* Keep catalog indexes current */
+        CatalogUpdateIndexes(tgrel, newtup);
+
+        heap_freetuple(newtup);
+    }
+    systable_endscan(tgscan);
+
+    heap_close(tgrel, RowExclusiveLock);
+
+    CommandCounterIncrement();
+
+    FreeTriggerDesc(rel->trigdesc);
+    RelationBuildTriggers(rel);
+}
diff -ru pgsql.orig/src/backend/parser/gram.y pgsql/src/backend/parser/gram.y
--- 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

     FALSE_P FETCH FIRST_P FLOAT_P FOR FORCE FOREIGN FORWARD
@@ -1389,6 +1389,22 @@
                     n->name = NULL;
                     $$ = (Node *)n;
                 }
+            /* ALTER TABLE <name> ENABLE TRIGGER <trig> */
+            | ENABLE TRIGGER name
+                {
+                    AlterTableCmd *n = makeNode(AlterTableCmd);
+                    n->subtype = AT_EnableTrig;
+                    n->name = $3;
+                    $$ = (Node *)n;
+                }
+            /* ALTER TABLE <name> DISABLE TRIGGER <trig> */
+            | DISABLE TRIGGER name
+                {
+                    AlterTableCmd *n = makeNode(AlterTableCmd);
+                    n->subtype = AT_DisableTrig;
+                    n->name = $3;
+                    $$ = (Node *)n;
+                }
             | alter_rel_cmd
                 {
                     $$ = $1;
diff -ru pgsql.orig/src/backend/parser/keywords.c pgsql/src/backend/parser/keywords.c
--- pgsql.orig/src/backend/parser/keywords.c    2005-06-30 05:34:14.000000000 +0900
+++ pgsql/src/backend/parser/keywords.c    2005-07-01 14:38:13.000000000 +0900
@@ -116,6 +116,7 @@
     {"delimiter", DELIMITER},
     {"delimiters", DELIMITERS},
     {"desc", DESC},
+    {"disable", DISABLE},
     {"distinct", DISTINCT},
     {"do", DO},
     {"domain", DOMAIN_P},
@@ -123,6 +124,7 @@
     {"drop", DROP},
     {"each", EACH},
     {"else", ELSE},
+    {"enable", ENABLE},
     {"encoding", ENCODING},
     {"encrypted", ENCRYPTED},
     {"end", END_P},
diff -ru pgsql.orig/src/include/commands/trigger.h pgsql/src/include/commands/trigger.h
--- 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);

 /*
  * in utils/adt/ri_triggers.c
diff -ru pgsql.orig/src/include/nodes/parsenodes.h pgsql/src/include/nodes/parsenodes.h
--- pgsql.orig/src/include/nodes/parsenodes.h    2005-06-29 04:51:24.000000000 +0900
+++ pgsql/src/include/nodes/parsenodes.h    2005-07-01 14:20:14.000000000 +0900
@@ -822,6 +822,8 @@
     AT_ClusterOn,                /* CLUSTER ON */
     AT_DropCluster,                /* SET WITHOUT CLUSTER */
     AT_DropOids,                /* SET WITHOUT OIDS */
+    AT_EnableTrig,                /* ENABLE TRIGGER */
+    AT_DisableTrig,                /* DISABLE TRIGGER */
     AT_SetTableSpace            /* SET TABLESPACE */
 } AlterTableType;


Re: enable/disable trigger (Re: Fwd: [HACKERS] Open items)

From
Christopher Kings-Lynne
Date:
>>ALTER TABLE <table> ENABLE TRIGGER <trigname>
>>ALTER TABLE <table> DISABLE TRIGGER <trigname>
>
>
> Bruce said to allow them only super-user,
> but currently this patch allows also the table owner.

The table owner can drop and create triggers - so why shouldn't they be
able to enable and disable them?

Chris


Re: enable/disable trigger (Re: Fwd: [HACKERS] Open items)

From
Satoshi Nagayasu
Date:
> The table owner can drop and create triggers - so why shouldn't they be
> able to enable and disable them?

For convenience or easy operation.

I believe the user doesn't like to create same triggers again and again.

Christopher Kings-Lynne wrote:
>>>ALTER TABLE <table> ENABLE TRIGGER <trigname>
>>>ALTER TABLE <table> DISABLE TRIGGER <trigname>
>>
>>
>>Bruce said to allow them only super-user,
>>but currently this patch allows also the table owner.
>
>
> The table owner can drop and create triggers - so why shouldn't they be
> able to enable and disable them?
>
> Chris
>
>


--
NAGAYASU Satoshi <nagayasus@nttdata.co.jp>

Re: enable/disable trigger (Re: Fwd: [HACKERS] Open items)

From
Christopher Kings-Lynne
Date:

Satoshi Nagayasu wrote:
>>The table owner can drop and create triggers - so why shouldn't they be
>>able to enable and disable them?
>
>
> For convenience or easy operation.
>
> I believe the user doesn't like to create same triggers again and again.

I said why _shouldn't_.  I was agreeing with you.

Chris


Re: enable/disable trigger (Re: Fwd: [HACKERS] Open items)

From
Satoshi Nagayasu
Date:
> I said why _shouldn't_.  I was agreeing with you.

Oops. Sorry.

I don't know why only super-user shold be able to enable/disable tirggers.

I believe the table owner want to enable/disable triggers,
because I always need it.

Loading huge data set into the table with triggers(or fk) is very painful.

Christopher Kings-Lynne wrote:
>
> Satoshi Nagayasu wrote:
>
>>>The table owner can drop and create triggers - so why shouldn't they be
>>>able to enable and disable them?
>>
>>
>>For convenience or easy operation.
>>
>>I believe the user doesn't like to create same triggers again and again.
>
>
> I said why _shouldn't_.  I was agreeing with you.
>
> Chris
>
>


--
NAGAYASU Satoshi <nagayasus@nttdata.co.jp>

Re: enable/disable trigger (Re: Fwd: [HACKERS] Open items)

From
Neil Conway
Date:
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

Re: enable/disable trigger (Re: Fwd: [HACKERS] Open items)

From
Satoshi Nagayasu
Date:
Thanks for comments, Neil.

Some are fixed.

Neil Conway wrote:
> 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).

Do we need some log/warning message for the user in these cases?

> 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.

I think so too.

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

I'll dive into pg_dump and psql this weekend...

--
NAGAYASU Satoshi <nagayasus@nttdata.co.jp>
diff -cr pgsql.orig/src/backend/commands/tablecmds.c pgsql/src/backend/commands/tablecmds.c
*** pgsql.orig/src/backend/commands/tablecmds.c    2005-06-28 14:08:54.000000000 +0900
--- pgsql/src/backend/commands/tablecmds.c    2005-07-01 15:50:27.000000000 +0900
***************
*** 236,241 ****
--- 236,243 ----
                                                Oid newOwnerId);
  static void ATExecClusterOn(Relation rel, const char *indexName);
  static void ATExecDropCluster(Relation rel);
+ static void ATExecEnableDisableTrigger(Relation rel, char *trigname,
+                                        bool enable);
  static void ATPrepSetTableSpace(AlteredTableInfo *tab, Relation rel,
                      char *tablespacename);
  static void ATExecSetTableSpace(Oid tableOid, Oid newTableSpace);
***************
*** 1993,1998 ****
--- 1995,2005 ----
              }
              pass = AT_PASS_DROP;
              break;
+         case AT_EnableTrig:        /* ENABLE TRIGGER */
+         case AT_DisableTrig:    /* DISABLE TRIGGER */
+             ATSimplePermissions(rel, false);
+             pass = AT_PASS_MISC;
+             break;
          case AT_SetTableSpace:    /* SET TABLESPACE */
              /* This command never recurses */
              ATPrepSetTableSpace(tab, rel, cmd->name);
***************
*** 2155,2160 ****
--- 2162,2173 ----
               * Nothing to do here; Phase 3 does the work
               */
              break;
+         case AT_EnableTrig:        /* ENABLE TRIGGER */
+             ATExecEnableDisableTrigger(rel, cmd->name, true);
+             break;
+         case AT_DisableTrig:    /* DISABLE TRIGGER */
+             ATExecEnableDisableTrigger(rel, cmd->name, false);
+             break;
          default:                /* oops */
              elog(ERROR, "unrecognized alter table type: %d",
                   (int) cmd->subtype);
***************
*** 5465,5470 ****
--- 5478,5492 ----
  }

  /*
+  * ALTER TABLE ENABLE/DISABLE TRIGGER
+  */
+ static void
+ ATExecEnableDisableTrigger(Relation rel, char *trigname, bool enable)
+ {
+     EnableDisableTrigger(rel, trigname, enable);
+ }
+
+ /*
   * ALTER TABLE SET TABLESPACE
   */
  static void
diff -cr pgsql.orig/src/backend/commands/trigger.c pgsql/src/backend/commands/trigger.c
*** pgsql.orig/src/backend/commands/trigger.c    2005-05-30 16:20:58.000000000 +0900
--- pgsql/src/backend/commands/trigger.c    2005-07-01 17:21:44.000000000 +0900
***************
*** 3063,3065 ****
--- 3063,3132 ----
          afterTriggerAddEvent(new_event);
      }
  }
+
+ /* ----------
+  * EnableDisableTrigger()
+  *
+  *    Called by ALTER TABLE ENABLE/DISABLE TRIGGER
+  *  to change 'tgenabled' flag in the pg_trigger.
+  * ----------
+  */
+ void
+ EnableDisableTrigger(Relation rel, const char *tgname, bool enable)
+ {
+     Relation tgrel;
+     SysScanDesc tgscan;
+     ScanKeyData keys[2];
+     HeapTuple tuple;
+
+     /* Permissions checks */
+     if (!pg_class_ownercheck(RelationGetRelid(rel), GetUserId()))
+         aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_CLASS,
+                        RelationGetRelationName(rel));
+
+     if (!allowSystemTableMods && IsSystemRelation(rel))
+         ereport(ERROR,
+                 (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+                  errmsg("permission denied: \"%s\" is a system catalog",
+                         RelationGetRelationName(rel))));
+
+     tgrel = heap_open(TriggerRelationId, RowExclusiveLock);
+
+     ScanKeyInit(&keys[0],
+                 Anum_pg_trigger_tgrelid,
+                 BTEqualStrategyNumber, F_OIDEQ,
+                 ObjectIdGetDatum(RelationGetRelid(rel)));
+     ScanKeyInit(&keys[1],
+                 Anum_pg_trigger_tgname,
+                 BTEqualStrategyNumber, F_NAMEEQ,
+                 CStringGetDatum(tgname));
+
+     tgscan = systable_beginscan(tgrel, TriggerRelidNameIndexId, true,
+                                 SnapshotNow, 1, keys);
+
+     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 != enable )
+         {
+             pg_trigger->tgenabled = enable;
+
+             simple_heap_update(tgrel, &newtup->t_self, newtup);
+
+             /* Keep catalog indexes current */
+             CatalogUpdateIndexes(tgrel, newtup);
+         }
+
+         heap_freetuple(newtup);
+     }
+     systable_endscan(tgscan);
+
+     heap_close(tgrel, RowExclusiveLock);
+
+     CommandCounterIncrement();
+
+     FreeTriggerDesc(rel->trigdesc);
+     RelationBuildTriggers(rel);
+ }
diff -cr pgsql.orig/src/backend/parser/gram.y pgsql/src/backend/parser/gram.y
*** pgsql.orig/src/backend/parser/gram.y    2005-06-30 05:34:13.000000000 +0900
--- pgsql/src/backend/parser/gram.y    2005-07-01 17:16:32.000000000 +0900
***************
*** 348,356 ****

      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

!     EACH ELSE ENCODING ENCRYPTED END_P ESCAPE EXCEPT EXCLUDING
      EXCLUSIVE EXECUTE EXISTS EXPLAIN EXTERNAL EXTRACT

      FALSE_P FETCH FIRST_P FLOAT_P FOR FORCE FOREIGN FORWARD
--- 348,356 ----

      DATABASE DAY_P DEALLOCATE DEC DECIMAL_P DECLARE DEFAULT DEFAULTS
      DEFERRABLE DEFERRED DEFINER DELETE_P DELIMITER DELIMITERS
!     DESC DISABLE DISTINCT DO DOMAIN_P DOUBLE_P DROP

!     EACH ELSE ENABLE ENCODING ENCRYPTED END_P ESCAPE EXCEPT EXCLUDING
      EXCLUSIVE EXECUTE EXISTS EXPLAIN EXTERNAL EXTRACT

      FALSE_P FETCH FIRST_P FLOAT_P FOR FORCE FOREIGN FORWARD
***************
*** 1389,1394 ****
--- 1389,1410 ----
                      n->name = NULL;
                      $$ = (Node *)n;
                  }
+             /* ALTER TABLE <name> ENABLE TRIGGER <trig> */
+             | ENABLE TRIGGER name
+                 {
+                     AlterTableCmd *n = makeNode(AlterTableCmd);
+                     n->subtype = AT_EnableTrig;
+                     n->name = $3;
+                     $$ = (Node *)n;
+                 }
+             /* ALTER TABLE <name> DISABLE TRIGGER <trig> */
+             | DISABLE TRIGGER name
+                 {
+                     AlterTableCmd *n = makeNode(AlterTableCmd);
+                     n->subtype = AT_DisableTrig;
+                     n->name = $3;
+                     $$ = (Node *)n;
+                 }
              | alter_rel_cmd
                  {
                      $$ = $1;
***************
*** 7960,7969 ****
--- 7976,7987 ----
              | DELETE_P
              | DELIMITER
              | DELIMITERS
+             | DISABLE
              | DOMAIN_P
              | DOUBLE_P
              | DROP
              | EACH
+             | ENABLE
              | ENCODING
              | ENCRYPTED
              | ESCAPE
diff -cr pgsql.orig/src/backend/parser/keywords.c pgsql/src/backend/parser/keywords.c
*** pgsql.orig/src/backend/parser/keywords.c    2005-06-30 05:34:14.000000000 +0900
--- pgsql/src/backend/parser/keywords.c    2005-07-01 14:38:13.000000000 +0900
***************
*** 116,121 ****
--- 116,122 ----
      {"delimiter", DELIMITER},
      {"delimiters", DELIMITERS},
      {"desc", DESC},
+     {"disable", DISABLE},
      {"distinct", DISTINCT},
      {"do", DO},
      {"domain", DOMAIN_P},
***************
*** 123,128 ****
--- 124,130 ----
      {"drop", DROP},
      {"each", EACH},
      {"else", ELSE},
+     {"enable", ENABLE},
      {"encoding", ENCODING},
      {"encrypted", ENCRYPTED},
      {"end", END_P},
diff -cr pgsql.orig/src/include/commands/trigger.h pgsql/src/include/commands/trigger.h
*** pgsql.orig/src/include/commands/trigger.h    2005-05-30 16:20:58.000000000 +0900
--- pgsql/src/include/commands/trigger.h    2005-07-01 17:14:37.000000000 +0900
***************
*** 164,169 ****
--- 164,172 ----

  extern void AfterTriggerSetState(ConstraintsSetStmt *stmt);

+ extern void EnableDisableTrigger(Relation rel,
+                                  const char *tgname,
+                                  bool enable);

  /*
   * in utils/adt/ri_triggers.c
diff -cr pgsql.orig/src/include/nodes/parsenodes.h pgsql/src/include/nodes/parsenodes.h
*** pgsql.orig/src/include/nodes/parsenodes.h    2005-06-29 04:51:24.000000000 +0900
--- pgsql/src/include/nodes/parsenodes.h    2005-07-01 14:20:14.000000000 +0900
***************
*** 822,827 ****
--- 822,829 ----
      AT_ClusterOn,                /* CLUSTER ON */
      AT_DropCluster,                /* SET WITHOUT CLUSTER */
      AT_DropOids,                /* SET WITHOUT OIDS */
+     AT_EnableTrig,                /* ENABLE TRIGGER */
+     AT_DisableTrig,                /* DISABLE TRIGGER */
      AT_SetTableSpace            /* SET TABLESPACE */
  } AlterTableType;


Re: enable/disable trigger (Re: Fwd: [HACKERS] Open items)

From
Satoshi Nagayasu
Date:
There is one more fix...

> +     tgscan = systable_beginscan(tgrel, TriggerRelidNameIndexId, true,
> +                                 SnapshotNow, 1, keys);

'1' was incorrect, must be '2'.

> +     tgscan = systable_beginscan(tgrel, TriggerRelidNameIndexId, true,
> +                                 SnapshotNow, 2, keys);

--
NAGAYASU Satoshi <nagayasus@nttdata.co.jp>
diff -cr pgsql.orig/src/backend/commands/tablecmds.c pgsql/src/backend/commands/tablecmds.c
*** pgsql.orig/src/backend/commands/tablecmds.c    2005-06-28 14:08:54.000000000 +0900
--- pgsql/src/backend/commands/tablecmds.c    2005-07-01 15:50:27.000000000 +0900
***************
*** 236,241 ****
--- 236,243 ----
                                                Oid newOwnerId);
  static void ATExecClusterOn(Relation rel, const char *indexName);
  static void ATExecDropCluster(Relation rel);
+ static void ATExecEnableDisableTrigger(Relation rel, char *trigname,
+                                        bool enable);
  static void ATPrepSetTableSpace(AlteredTableInfo *tab, Relation rel,
                      char *tablespacename);
  static void ATExecSetTableSpace(Oid tableOid, Oid newTableSpace);
***************
*** 1993,1998 ****
--- 1995,2005 ----
              }
              pass = AT_PASS_DROP;
              break;
+         case AT_EnableTrig:        /* ENABLE TRIGGER */
+         case AT_DisableTrig:    /* DISABLE TRIGGER */
+             ATSimplePermissions(rel, false);
+             pass = AT_PASS_MISC;
+             break;
          case AT_SetTableSpace:    /* SET TABLESPACE */
              /* This command never recurses */
              ATPrepSetTableSpace(tab, rel, cmd->name);
***************
*** 2155,2160 ****
--- 2162,2173 ----
               * Nothing to do here; Phase 3 does the work
               */
              break;
+         case AT_EnableTrig:        /* ENABLE TRIGGER */
+             ATExecEnableDisableTrigger(rel, cmd->name, true);
+             break;
+         case AT_DisableTrig:    /* DISABLE TRIGGER */
+             ATExecEnableDisableTrigger(rel, cmd->name, false);
+             break;
          default:                /* oops */
              elog(ERROR, "unrecognized alter table type: %d",
                   (int) cmd->subtype);
***************
*** 5465,5470 ****
--- 5478,5492 ----
  }

  /*
+  * ALTER TABLE ENABLE/DISABLE TRIGGER
+  */
+ static void
+ ATExecEnableDisableTrigger(Relation rel, char *trigname, bool enable)
+ {
+     EnableDisableTrigger(rel, trigname, enable);
+ }
+
+ /*
   * ALTER TABLE SET TABLESPACE
   */
  static void
diff -cr pgsql.orig/src/backend/commands/trigger.c pgsql/src/backend/commands/trigger.c
*** pgsql.orig/src/backend/commands/trigger.c    2005-05-30 16:20:58.000000000 +0900
--- pgsql/src/backend/commands/trigger.c    2005-07-01 17:21:44.000000000 +0900
***************
*** 3063,3065 ****
--- 3063,3132 ----
          afterTriggerAddEvent(new_event);
      }
  }
+
+ /* ----------
+  * EnableDisableTrigger()
+  *
+  *    Called by ALTER TABLE ENABLE/DISABLE TRIGGER
+  *  to change 'tgenabled' flag in the pg_trigger.
+  * ----------
+  */
+ void
+ EnableDisableTrigger(Relation rel, const char *tgname, bool enable)
+ {
+     Relation tgrel;
+     SysScanDesc tgscan;
+     ScanKeyData keys[2];
+     HeapTuple tuple;
+
+     /* Permissions checks */
+     if (!pg_class_ownercheck(RelationGetRelid(rel), GetUserId()))
+         aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_CLASS,
+                        RelationGetRelationName(rel));
+
+     if (!allowSystemTableMods && IsSystemRelation(rel))
+         ereport(ERROR,
+                 (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+                  errmsg("permission denied: \"%s\" is a system catalog",
+                         RelationGetRelationName(rel))));
+
+     tgrel = heap_open(TriggerRelationId, RowExclusiveLock);
+
+     ScanKeyInit(&keys[0],
+                 Anum_pg_trigger_tgrelid,
+                 BTEqualStrategyNumber, F_OIDEQ,
+                 ObjectIdGetDatum(RelationGetRelid(rel)));
+     ScanKeyInit(&keys[1],
+                 Anum_pg_trigger_tgname,
+                 BTEqualStrategyNumber, F_NAMEEQ,
+                 CStringGetDatum(tgname));
+
+     tgscan = systable_beginscan(tgrel, TriggerRelidNameIndexId, true,
+                                 SnapshotNow, 2, keys);
+
+     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 != enable )
+         {
+             pg_trigger->tgenabled = enable;
+
+             simple_heap_update(tgrel, &newtup->t_self, newtup);
+
+             /* Keep catalog indexes current */
+             CatalogUpdateIndexes(tgrel, newtup);
+         }
+
+         heap_freetuple(newtup);
+     }
+     systable_endscan(tgscan);
+
+     heap_close(tgrel, RowExclusiveLock);
+
+     CommandCounterIncrement();
+
+     FreeTriggerDesc(rel->trigdesc);
+     RelationBuildTriggers(rel);
+ }
diff -cr pgsql.orig/src/backend/parser/gram.y pgsql/src/backend/parser/gram.y
*** pgsql.orig/src/backend/parser/gram.y    2005-06-30 05:34:13.000000000 +0900
--- pgsql/src/backend/parser/gram.y    2005-07-01 17:16:32.000000000 +0900
***************
*** 348,356 ****

      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

!     EACH ELSE ENCODING ENCRYPTED END_P ESCAPE EXCEPT EXCLUDING
      EXCLUSIVE EXECUTE EXISTS EXPLAIN EXTERNAL EXTRACT

      FALSE_P FETCH FIRST_P FLOAT_P FOR FORCE FOREIGN FORWARD
--- 348,356 ----

      DATABASE DAY_P DEALLOCATE DEC DECIMAL_P DECLARE DEFAULT DEFAULTS
      DEFERRABLE DEFERRED DEFINER DELETE_P DELIMITER DELIMITERS
!     DESC DISABLE DISTINCT DO DOMAIN_P DOUBLE_P DROP

!     EACH ELSE ENABLE ENCODING ENCRYPTED END_P ESCAPE EXCEPT EXCLUDING
      EXCLUSIVE EXECUTE EXISTS EXPLAIN EXTERNAL EXTRACT

      FALSE_P FETCH FIRST_P FLOAT_P FOR FORCE FOREIGN FORWARD
***************
*** 1389,1394 ****
--- 1389,1410 ----
                      n->name = NULL;
                      $$ = (Node *)n;
                  }
+             /* ALTER TABLE <name> ENABLE TRIGGER <trig> */
+             | ENABLE TRIGGER name
+                 {
+                     AlterTableCmd *n = makeNode(AlterTableCmd);
+                     n->subtype = AT_EnableTrig;
+                     n->name = $3;
+                     $$ = (Node *)n;
+                 }
+             /* ALTER TABLE <name> DISABLE TRIGGER <trig> */
+             | DISABLE TRIGGER name
+                 {
+                     AlterTableCmd *n = makeNode(AlterTableCmd);
+                     n->subtype = AT_DisableTrig;
+                     n->name = $3;
+                     $$ = (Node *)n;
+                 }
              | alter_rel_cmd
                  {
                      $$ = $1;
***************
*** 7960,7969 ****
--- 7976,7987 ----
              | DELETE_P
              | DELIMITER
              | DELIMITERS
+             | DISABLE
              | DOMAIN_P
              | DOUBLE_P
              | DROP
              | EACH
+             | ENABLE
              | ENCODING
              | ENCRYPTED
              | ESCAPE
diff -cr pgsql.orig/src/backend/parser/keywords.c pgsql/src/backend/parser/keywords.c
*** pgsql.orig/src/backend/parser/keywords.c    2005-06-30 05:34:14.000000000 +0900
--- pgsql/src/backend/parser/keywords.c    2005-07-01 14:38:13.000000000 +0900
***************
*** 116,121 ****
--- 116,122 ----
      {"delimiter", DELIMITER},
      {"delimiters", DELIMITERS},
      {"desc", DESC},
+     {"disable", DISABLE},
      {"distinct", DISTINCT},
      {"do", DO},
      {"domain", DOMAIN_P},
***************
*** 123,128 ****
--- 124,130 ----
      {"drop", DROP},
      {"each", EACH},
      {"else", ELSE},
+     {"enable", ENABLE},
      {"encoding", ENCODING},
      {"encrypted", ENCRYPTED},
      {"end", END_P},
diff -cr pgsql.orig/src/include/commands/trigger.h pgsql/src/include/commands/trigger.h
*** pgsql.orig/src/include/commands/trigger.h    2005-05-30 16:20:58.000000000 +0900
--- pgsql/src/include/commands/trigger.h    2005-07-01 17:14:37.000000000 +0900
***************
*** 164,169 ****
--- 164,172 ----

  extern void AfterTriggerSetState(ConstraintsSetStmt *stmt);

+ extern void EnableDisableTrigger(Relation rel,
+                                  const char *tgname,
+                                  bool enable);

  /*
   * in utils/adt/ri_triggers.c
diff -cr pgsql.orig/src/include/nodes/parsenodes.h pgsql/src/include/nodes/parsenodes.h
*** pgsql.orig/src/include/nodes/parsenodes.h    2005-06-29 04:51:24.000000000 +0900
--- pgsql/src/include/nodes/parsenodes.h    2005-07-01 14:20:14.000000000 +0900
***************
*** 822,827 ****
--- 822,829 ----
      AT_ClusterOn,                /* CLUSTER ON */
      AT_DropCluster,                /* SET WITHOUT CLUSTER */
      AT_DropOids,                /* SET WITHOUT OIDS */
+     AT_EnableTrig,                /* ENABLE TRIGGER */
+     AT_DisableTrig,                /* DISABLE TRIGGER */
      AT_SetTableSpace            /* SET TABLESPACE */
  } AlterTableType;


Re: enable/disable trigger (Re: Fwd: [HACKERS] Open items)

From
Gavin Sherry
Date:
On Fri, 1 Jul 2005, Satoshi Nagayasu wrote:

> Hi all,
>
> Here is a first patch to allow these commands.
>
> > ALTER TABLE <table> ENABLE TRIGGER <trigname>
> > ALTER TABLE <table> DISABLE TRIGGER <trigname>

There are three other areas which are worth looking at:

a) We may defer the execution of some triggers to the end of the
transaction. Do we execute those if a they were later disabled?

b) There is a bug in how we execute triggers. For example, in
ExecDelete():

        bool        dodelete;

        dodelete = ExecBRDeleteTriggers(estate, resultRelInfo, tupleid,
                                        estate->es_snapshot->curcid);

        if (!dodelete)          /* "do nothing" */
            return;


This means that if a before trigger returned NULL, we short circuit and do
not delete the tuple. Consider the following in ExecBRDeleteTriggers()

    HeapTuple   newtuple = NULL;

...

   for (i = 0; i < ntrigs; i++)
    {
        Trigger    *trigger = &trigdesc->triggers[tgindx[i]];

        if (!trigger->tgenabled)
            continue;
        LocTriggerData.tg_trigtuple = trigtuple;
        LocTriggerData.tg_trigtuplebuf = InvalidBuffer;
        LocTriggerData.tg_trigger = trigger;
        newtuple = ExecCallTriggerFunc(&LocTriggerData,
                                       tgindx[i],
                                       relinfo->ri_TrigFunctions,
                                       relinfo->ri_TrigInstrument,
                                       GetPerTupleMemoryContext(estate));
        if (newtuple == NULL)
            break;
        if (newtuple != trigtuple)
            heap_freetuple(newtuple);
    }

...

    return (newtuple == NULL) ? false : true;

This means that if all triggers on a table are disabled, we tell the
caller that a trigger returned NULL and that we should short circuit. This
does not seem to be the case for the other DML statements.

c) There has been a push over previous releases to make dumps generated by
pg_dump look like ANSI SQL. Now, ALTER TABLE ... DISABLE trigger is useful
for pg_dump but not compliant. Others have suggested something like:

SET enable_triggers = off

This would turn all triggers off in the current session. It has the added
benefit that it does not affect other sessions. It does introduce some
issues with permissions -- I wouldn't want users turning off data
validation before triggers in my database, but that's me. I'm not
enamoured of the idea but it is worth discussing, I believe.

Also, a final patch will also need documentation and regression tests :-)

Thanks,

Gavin

Re: enable/disable trigger (Re: Fwd: [HACKERS] Open items)

From
Gavin Sherry
Date:
On Fri, 1 Jul 2005, Satoshi Nagayasu wrote:

> Hi all,
>
> Here is a first patch to allow these commands.
>
> > ALTER TABLE <table> ENABLE TRIGGER <trigname>
> > ALTER TABLE <table> DISABLE TRIGGER <trigname>
>

Hmmm.. just thinking about it for a second. I wonder if we should also
support:

ALTER TABLE DISABLE TRIGGERS

which would disable all triggers on the table. We would have a
complimentary ENABLE TRIGGERS as well, obviously. The reason I say this is
that the common case will be that people are doing a bulk load and want to
disable all triggers. However, this will be very useful for debugging
interactions between triggers on a table so a user might want to disable
only one of many triggers -- as your current grammar does.

Perhaps a way of making the grammar a little less ambiguous would be to
have the following to disable all triggers:

ALTER TABLE <table> DISABLE TRIGGERS

and the following to disable one:

ALTER TRIGGER <trigger> DISABLE


Just an idea.

Thanks,

Gavin

Re: enable/disable trigger (Re: Fwd: [HACKERS] Open items)

From
Satoshi Nagayasu
Date:
Hi,

Gavin Sherry wrote:
> Hmmm.. just thinking about it for a second. I wonder if we should also
> support:
>
> ALTER TABLE DISABLE TRIGGERS

I found some RDBMSes are supporting 'DISABLE TRIGGER ALL TRIGGERS' command.

Does anyone know about the SQL99 spec?

--
NAGAYASU Satoshi <nagayasus@nttdata.co.jp>

Re: enable/disable trigger (Re: Fwd: [HACKERS] Open items)

From
Gavin Sherry
Date:
On Fri, 1 Jul 2005, Satoshi Nagayasu wrote:

> Hi,
>
> Gavin Sherry wrote:
> > Hmmm.. just thinking about it for a second. I wonder if we should also
> > support:
> >
> > ALTER TABLE DISABLE TRIGGERS
>
> I found some RDBMSes are supporting 'DISABLE TRIGGER ALL TRIGGERS' command.
>
> Does anyone know about the SQL99 spec?

The spec says nothing about disabling triggers.

Gavin

Re: enable/disable trigger (Re: Fwd: [HACKERS] Open items)

From
Tom Lane
Date:
Christopher Kings-Lynne <chriskl@familyhealth.com.au> writes:
> The table owner can drop and create triggers - so why shouldn't they be
> able to enable and disable them?

Not triggers belonging to RI constraints on other tables.

            regards, tom lane

Re: enable/disable trigger (Re: Fwd: [HACKERS] Open items)

From
Andreas Pflug
Date:
Satoshi Nagayasu wrote:

>Hi all,
>
>Here is a first patch to allow these commands.
>
>
>
>>ALTER TABLE <table> ENABLE TRIGGER <trigname>
>>ALTER TABLE <table> DISABLE TRIGGER <trigname>
>>
>>
>
>Bruce said to allow them only super-user,
>but currently this patch allows also the table owner.
>
>
>
It would be convenient if all triggers could be disabled with a single
command. More precise:
option 1: All triggers except for RI triggers (EN/DISABLE TRIGGER ALL)
option 2: really all triggers including RI triggers (superuser only)

Regards,
Andreas




Re: enable/disable trigger (Re: Fwd: [HACKERS] Open items)

From
Bruce Momjian
Date:
Gavin Sherry wrote:
> On Fri, 1 Jul 2005, Satoshi Nagayasu wrote:
>
> > Hi all,
> >
> > Here is a first patch to allow these commands.
> >
> > > ALTER TABLE <table> ENABLE TRIGGER <trigname>
> > > ALTER TABLE <table> DISABLE TRIGGER <trigname>
> >
>
> Hmmm.. just thinking about it for a second. I wonder if we should also
> support:
>
> ALTER TABLE DISABLE TRIGGERS
>
> which would disable all triggers on the table. We would have a
> complimentary ENABLE TRIGGERS as well, obviously. The reason I say this is
> that the common case will be that people are doing a bulk load and want to
> disable all triggers. However, this will be very useful for debugging
> interactions between triggers on a table so a user might want to disable
> only one of many triggers -- as your current grammar does.
>
> Perhaps a way of making the grammar a little less ambiguous would be to
> have the following to disable all triggers:
>
> ALTER TABLE <table> DISABLE TRIGGERS
>
> and the following to disable one:
>
> ALTER TRIGGER <trigger> DISABLE

The proposed syntax in TODO.detail/trigger is:

    ALTER TABLE foo {DISABLE|ENABLE} TRIGGER { ALL | trigger_name [ ,... ] };

which would cleanly allow all triggers of a table to be disabled.

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Re: enable/disable trigger (Re: Fwd: [HACKERS] Open items)

From
Bruce Momjian
Date:
I am not sure what to do with this patch.  It is missing dump
capability, there is no clause to disable all triggers on a table, and
it uses a table owner check when a super user check is required (because
of referential integrity).

Satoshi, are you still working on it?  If not does someone else want to
complete it?  I realized you just started on it but the deadline is
soon.

---------------------------------------------------------------------------

Satoshi Nagayasu wrote:
> There is one more fix...
>
> > +     tgscan = systable_beginscan(tgrel, TriggerRelidNameIndexId, true,
> > +                                 SnapshotNow, 1, keys);
>
> '1' was incorrect, must be '2'.
>
> > +     tgscan = systable_beginscan(tgrel, TriggerRelidNameIndexId, true,
> > +                                 SnapshotNow, 2, keys);
>
> --
> NAGAYASU Satoshi <nagayasus@nttdata.co.jp>

> diff -cr pgsql.orig/src/backend/commands/tablecmds.c pgsql/src/backend/commands/tablecmds.c
> *** pgsql.orig/src/backend/commands/tablecmds.c    2005-06-28 14:08:54.000000000 +0900
> --- pgsql/src/backend/commands/tablecmds.c    2005-07-01 15:50:27.000000000 +0900
> ***************
> *** 236,241 ****
> --- 236,243 ----
>                                                 Oid newOwnerId);
>   static void ATExecClusterOn(Relation rel, const char *indexName);
>   static void ATExecDropCluster(Relation rel);
> + static void ATExecEnableDisableTrigger(Relation rel, char *trigname,
> +                                        bool enable);
>   static void ATPrepSetTableSpace(AlteredTableInfo *tab, Relation rel,
>                       char *tablespacename);
>   static void ATExecSetTableSpace(Oid tableOid, Oid newTableSpace);
> ***************
> *** 1993,1998 ****
> --- 1995,2005 ----
>               }
>               pass = AT_PASS_DROP;
>               break;
> +         case AT_EnableTrig:        /* ENABLE TRIGGER */
> +         case AT_DisableTrig:    /* DISABLE TRIGGER */
> +             ATSimplePermissions(rel, false);
> +             pass = AT_PASS_MISC;
> +             break;
>           case AT_SetTableSpace:    /* SET TABLESPACE */
>               /* This command never recurses */
>               ATPrepSetTableSpace(tab, rel, cmd->name);
> ***************
> *** 2155,2160 ****
> --- 2162,2173 ----
>                * Nothing to do here; Phase 3 does the work
>                */
>               break;
> +         case AT_EnableTrig:        /* ENABLE TRIGGER */
> +             ATExecEnableDisableTrigger(rel, cmd->name, true);
> +             break;
> +         case AT_DisableTrig:    /* DISABLE TRIGGER */
> +             ATExecEnableDisableTrigger(rel, cmd->name, false);
> +             break;
>           default:                /* oops */
>               elog(ERROR, "unrecognized alter table type: %d",
>                    (int) cmd->subtype);
> ***************
> *** 5465,5470 ****
> --- 5478,5492 ----
>   }
>
>   /*
> +  * ALTER TABLE ENABLE/DISABLE TRIGGER
> +  */
> + static void
> + ATExecEnableDisableTrigger(Relation rel, char *trigname, bool enable)
> + {
> +     EnableDisableTrigger(rel, trigname, enable);
> + }
> +
> + /*
>    * ALTER TABLE SET TABLESPACE
>    */
>   static void
> diff -cr pgsql.orig/src/backend/commands/trigger.c pgsql/src/backend/commands/trigger.c
> *** pgsql.orig/src/backend/commands/trigger.c    2005-05-30 16:20:58.000000000 +0900
> --- pgsql/src/backend/commands/trigger.c    2005-07-01 17:21:44.000000000 +0900
> ***************
> *** 3063,3065 ****
> --- 3063,3132 ----
>           afterTriggerAddEvent(new_event);
>       }
>   }
> +
> + /* ----------
> +  * EnableDisableTrigger()
> +  *
> +  *    Called by ALTER TABLE ENABLE/DISABLE TRIGGER
> +  *  to change 'tgenabled' flag in the pg_trigger.
> +  * ----------
> +  */
> + void
> + EnableDisableTrigger(Relation rel, const char *tgname, bool enable)
> + {
> +     Relation tgrel;
> +     SysScanDesc tgscan;
> +     ScanKeyData keys[2];
> +     HeapTuple tuple;
> +
> +     /* Permissions checks */
> +     if (!pg_class_ownercheck(RelationGetRelid(rel), GetUserId()))
> +         aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_CLASS,
> +                        RelationGetRelationName(rel));
> +
> +     if (!allowSystemTableMods && IsSystemRelation(rel))
> +         ereport(ERROR,
> +                 (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
> +                  errmsg("permission denied: \"%s\" is a system catalog",
> +                         RelationGetRelationName(rel))));
> +
> +     tgrel = heap_open(TriggerRelationId, RowExclusiveLock);
> +
> +     ScanKeyInit(&keys[0],
> +                 Anum_pg_trigger_tgrelid,
> +                 BTEqualStrategyNumber, F_OIDEQ,
> +                 ObjectIdGetDatum(RelationGetRelid(rel)));
> +     ScanKeyInit(&keys[1],
> +                 Anum_pg_trigger_tgname,
> +                 BTEqualStrategyNumber, F_NAMEEQ,
> +                 CStringGetDatum(tgname));
> +
> +     tgscan = systable_beginscan(tgrel, TriggerRelidNameIndexId, true,
> +                                 SnapshotNow, 2, keys);
> +
> +     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 != enable )
> +         {
> +             pg_trigger->tgenabled = enable;
> +
> +             simple_heap_update(tgrel, &newtup->t_self, newtup);
> +
> +             /* Keep catalog indexes current */
> +             CatalogUpdateIndexes(tgrel, newtup);
> +         }
> +
> +         heap_freetuple(newtup);
> +     }
> +     systable_endscan(tgscan);
> +
> +     heap_close(tgrel, RowExclusiveLock);
> +
> +     CommandCounterIncrement();
> +
> +     FreeTriggerDesc(rel->trigdesc);
> +     RelationBuildTriggers(rel);
> + }
> diff -cr pgsql.orig/src/backend/parser/gram.y pgsql/src/backend/parser/gram.y
> *** pgsql.orig/src/backend/parser/gram.y    2005-06-30 05:34:13.000000000 +0900
> --- pgsql/src/backend/parser/gram.y    2005-07-01 17:16:32.000000000 +0900
> ***************
> *** 348,356 ****
>
>       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
>
> !     EACH ELSE ENCODING ENCRYPTED END_P ESCAPE EXCEPT EXCLUDING
>       EXCLUSIVE EXECUTE EXISTS EXPLAIN EXTERNAL EXTRACT
>
>       FALSE_P FETCH FIRST_P FLOAT_P FOR FORCE FOREIGN FORWARD
> --- 348,356 ----
>
>       DATABASE DAY_P DEALLOCATE DEC DECIMAL_P DECLARE DEFAULT DEFAULTS
>       DEFERRABLE DEFERRED DEFINER DELETE_P DELIMITER DELIMITERS
> !     DESC DISABLE DISTINCT DO DOMAIN_P DOUBLE_P DROP
>
> !     EACH ELSE ENABLE ENCODING ENCRYPTED END_P ESCAPE EXCEPT EXCLUDING
>       EXCLUSIVE EXECUTE EXISTS EXPLAIN EXTERNAL EXTRACT
>
>       FALSE_P FETCH FIRST_P FLOAT_P FOR FORCE FOREIGN FORWARD
> ***************
> *** 1389,1394 ****
> --- 1389,1410 ----
>                       n->name = NULL;
>                       $$ = (Node *)n;
>                   }
> +             /* ALTER TABLE <name> ENABLE TRIGGER <trig> */
> +             | ENABLE TRIGGER name
> +                 {
> +                     AlterTableCmd *n = makeNode(AlterTableCmd);
> +                     n->subtype = AT_EnableTrig;
> +                     n->name = $3;
> +                     $$ = (Node *)n;
> +                 }
> +             /* ALTER TABLE <name> DISABLE TRIGGER <trig> */
> +             | DISABLE TRIGGER name
> +                 {
> +                     AlterTableCmd *n = makeNode(AlterTableCmd);
> +                     n->subtype = AT_DisableTrig;
> +                     n->name = $3;
> +                     $$ = (Node *)n;
> +                 }
>               | alter_rel_cmd
>                   {
>                       $$ = $1;
> ***************
> *** 7960,7969 ****
> --- 7976,7987 ----
>               | DELETE_P
>               | DELIMITER
>               | DELIMITERS
> +             | DISABLE
>               | DOMAIN_P
>               | DOUBLE_P
>               | DROP
>               | EACH
> +             | ENABLE
>               | ENCODING
>               | ENCRYPTED
>               | ESCAPE
> diff -cr pgsql.orig/src/backend/parser/keywords.c pgsql/src/backend/parser/keywords.c
> *** pgsql.orig/src/backend/parser/keywords.c    2005-06-30 05:34:14.000000000 +0900
> --- pgsql/src/backend/parser/keywords.c    2005-07-01 14:38:13.000000000 +0900
> ***************
> *** 116,121 ****
> --- 116,122 ----
>       {"delimiter", DELIMITER},
>       {"delimiters", DELIMITERS},
>       {"desc", DESC},
> +     {"disable", DISABLE},
>       {"distinct", DISTINCT},
>       {"do", DO},
>       {"domain", DOMAIN_P},
> ***************
> *** 123,128 ****
> --- 124,130 ----
>       {"drop", DROP},
>       {"each", EACH},
>       {"else", ELSE},
> +     {"enable", ENABLE},
>       {"encoding", ENCODING},
>       {"encrypted", ENCRYPTED},
>       {"end", END_P},
> diff -cr pgsql.orig/src/include/commands/trigger.h pgsql/src/include/commands/trigger.h
> *** pgsql.orig/src/include/commands/trigger.h    2005-05-30 16:20:58.000000000 +0900
> --- pgsql/src/include/commands/trigger.h    2005-07-01 17:14:37.000000000 +0900
> ***************
> *** 164,169 ****
> --- 164,172 ----
>
>   extern void AfterTriggerSetState(ConstraintsSetStmt *stmt);
>
> + extern void EnableDisableTrigger(Relation rel,
> +                                  const char *tgname,
> +                                  bool enable);
>
>   /*
>    * in utils/adt/ri_triggers.c
> diff -cr pgsql.orig/src/include/nodes/parsenodes.h pgsql/src/include/nodes/parsenodes.h
> *** pgsql.orig/src/include/nodes/parsenodes.h    2005-06-29 04:51:24.000000000 +0900
> --- pgsql/src/include/nodes/parsenodes.h    2005-07-01 14:20:14.000000000 +0900
> ***************
> *** 822,827 ****
> --- 822,829 ----
>       AT_ClusterOn,                /* CLUSTER ON */
>       AT_DropCluster,                /* SET WITHOUT CLUSTER */
>       AT_DropOids,                /* SET WITHOUT OIDS */
> +     AT_EnableTrig,                /* ENABLE TRIGGER */
> +     AT_DisableTrig,                /* DISABLE TRIGGER */
>       AT_SetTableSpace            /* SET TABLESPACE */
>   } AlterTableType;
>
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 7: don't forget to increase your free space map settings

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Re: enable/disable trigger (Re: Fwd: [HACKERS] Open items)

From
Andreas Pflug
Date:
Bruce Momjian wrote:

>I am not sure what to do with this patch.  It is missing dump
>capability, there is no clause to disable all triggers on a table, and
>it uses a table owner check when a super user check is required (because
>of referential integrity).
>
>
 From a user's view, a trigger implementing RI isn't a trigger but an
implementation detail he shouldn't need to care about. So for std
triggers, owner check should be sufficient, requiring superuser for RI
triggers only.
This impacts EN/DISABLE TRIGGER ALL too. To touch RI triggers as well,
an additional keyword is needed.

Regards,
Andreas


Re: enable/disable trigger (Re: Fwd: [HACKERS] Open items)

From
Satoshi Nagayasu
Date:
Bruce Momjian wrote:
> I am not sure what to do with this patch.  It is missing dump
> capability, there is no clause to disable all triggers on a table, and
> it uses a table owner check when a super user check is required (because
> of referential integrity).
>
> Satoshi, are you still working on it?  If not does someone else want to
> complete it?  I realized you just started on it but the deadline is
> soon.

I've already implemented 'ENABLE/DISABLE TRIGGER ALL',
and the superuser check has been added.  Please check it.

And, I'm going to have a business trip to Sydney this weekend,
so I'll complete pg_dump stuffs while my flight.

Thank you.
--
NAGAYASU Satoshi <nagayasus@nttdata.co.jp>
diff -cr pgsql.orig/src/backend/commands/tablecmds.c pgsql/src/backend/commands/tablecmds.c
*** pgsql.orig/src/backend/commands/tablecmds.c    2005-06-28 14:08:54.000000000 +0900
--- pgsql/src/backend/commands/tablecmds.c    2005-07-01 15:50:27.000000000 +0900
***************
*** 236,241 ****
--- 236,243 ----
                                                Oid newOwnerId);
  static void ATExecClusterOn(Relation rel, const char *indexName);
  static void ATExecDropCluster(Relation rel);
+ static void ATExecEnableDisableTrigger(Relation rel, char *trigname,
+                                        bool enable);
  static void ATPrepSetTableSpace(AlteredTableInfo *tab, Relation rel,
                      char *tablespacename);
  static void ATExecSetTableSpace(Oid tableOid, Oid newTableSpace);
***************
*** 1993,1998 ****
--- 1995,2005 ----
              }
              pass = AT_PASS_DROP;
              break;
+         case AT_EnableTrig:        /* ENABLE TRIGGER */
+         case AT_DisableTrig:    /* DISABLE TRIGGER */
+             ATSimplePermissions(rel, false);
+             pass = AT_PASS_MISC;
+             break;
          case AT_SetTableSpace:    /* SET TABLESPACE */
              /* This command never recurses */
              ATPrepSetTableSpace(tab, rel, cmd->name);
***************
*** 2155,2160 ****
--- 2162,2173 ----
               * Nothing to do here; Phase 3 does the work
               */
              break;
+         case AT_EnableTrig:        /* ENABLE TRIGGER */
+             ATExecEnableDisableTrigger(rel, cmd->name, true);
+             break;
+         case AT_DisableTrig:    /* DISABLE TRIGGER */
+             ATExecEnableDisableTrigger(rel, cmd->name, false);
+             break;
          default:                /* oops */
              elog(ERROR, "unrecognized alter table type: %d",
                   (int) cmd->subtype);
***************
*** 5465,5470 ****
--- 5478,5492 ----
  }

  /*
+  * ALTER TABLE ENABLE/DISABLE TRIGGER
+  */
+ static void
+ ATExecEnableDisableTrigger(Relation rel, char *trigname, bool enable)
+ {
+     EnableDisableTrigger(rel, trigname, enable);
+ }
+
+ /*
   * ALTER TABLE SET TABLESPACE
   */
  static void
diff -cr pgsql.orig/src/backend/commands/trigger.c pgsql/src/backend/commands/trigger.c
*** pgsql.orig/src/backend/commands/trigger.c    2005-05-30 16:20:58.000000000 +0900
--- pgsql/src/backend/commands/trigger.c    2005-07-04 10:40:27.000000000 +0900
***************
*** 3063,3065 ****
--- 3063,3158 ----
          afterTriggerAddEvent(new_event);
      }
  }
+
+ /* ----------
+  * EnableDisableTrigger()
+  *
+  *    Called by ALTER TABLE ENABLE/DISABLE TRIGGER
+  *  to change 'tgenabled' flag in the pg_trigger.
+  * ----------
+  */
+ void
+ EnableDisableTrigger(Relation rel, const char *tgname, bool enable)
+ {
+     Relation tgrel;
+     SysScanDesc tgscan;
+     ScanKeyData keys[2];
+     HeapTuple tuple;
+     int nkeys;
+     int changed;
+
+     /* Permissions checks */
+     if (!pg_class_ownercheck(RelationGetRelid(rel), GetUserId()))
+         aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_CLASS,
+                        RelationGetRelationName(rel));
+
+     if (!allowSystemTableMods && IsSystemRelation(rel))
+         ereport(ERROR,
+                 (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+                  errmsg("permission denied: \"%s\" is a system catalog",
+                         RelationGetRelationName(rel))));
+
+     tgrel = heap_open(TriggerRelationId, RowExclusiveLock);
+
+     nkeys = 2;
+     changed = 0;
+     if ( strcmp(tgname, "*")==0 )
+     {
+         if ( !superuser() )
+             elog(ERROR, "ENABLE/DISABLE TRIGGER ALL is allowed superuser only.");
+
+         nkeys = 1;
+     }
+
+     ScanKeyInit(&keys[0],
+                 Anum_pg_trigger_tgrelid,
+                 BTEqualStrategyNumber, F_OIDEQ,
+                 ObjectIdGetDatum(RelationGetRelid(rel)));
+     ScanKeyInit(&keys[1],
+                 Anum_pg_trigger_tgname,
+                 BTEqualStrategyNumber, F_NAMEEQ,
+                 CStringGetDatum(tgname));
+
+     tgscan = systable_beginscan(tgrel, TriggerRelidNameIndexId, true,
+                                 SnapshotNow, nkeys, keys);
+
+     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 != enable )
+         {
+             if ( !superuser() && pg_trigger->tgisconstraint )
+             {
+                 elog(NOTICE, "Constraint trigger can be enabled/disabled "
+                      "only by superuser.");
+                 continue;
+             }
+
+             pg_trigger->tgenabled = enable;
+
+             simple_heap_update(tgrel, &newtup->t_self, newtup);
+
+             /* Keep catalog indexes current */
+             CatalogUpdateIndexes(tgrel, newtup);
+
+             changed++;
+         }
+
+         heap_freetuple(newtup);
+     }
+     systable_endscan(tgscan);
+
+     heap_close(tgrel, RowExclusiveLock);
+
+     CommandCounterIncrement();
+
+     FreeTriggerDesc(rel->trigdesc);
+     RelationBuildTriggers(rel);
+
+     elog(NOTICE, "%d trigger(s) on %s %s.",
+          changed,
+          NameStr(rel->rd_rel->relname),
+          enable ? "enabled" : "disabled");
+ }
diff -cr pgsql.orig/src/backend/parser/gram.y pgsql/src/backend/parser/gram.y
*** pgsql.orig/src/backend/parser/gram.y    2005-06-30 05:34:13.000000000 +0900
--- pgsql/src/backend/parser/gram.y    2005-07-04 10:06:08.000000000 +0900
***************
*** 348,356 ****

      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

!     EACH ELSE ENCODING ENCRYPTED END_P ESCAPE EXCEPT EXCLUDING
      EXCLUSIVE EXECUTE EXISTS EXPLAIN EXTERNAL EXTRACT

      FALSE_P FETCH FIRST_P FLOAT_P FOR FORCE FOREIGN FORWARD
--- 348,356 ----

      DATABASE DAY_P DEALLOCATE DEC DECIMAL_P DECLARE DEFAULT DEFAULTS
      DEFERRABLE DEFERRED DEFINER DELETE_P DELIMITER DELIMITERS
!     DESC DISABLE DISTINCT DO DOMAIN_P DOUBLE_P DROP

!     EACH ELSE ENABLE ENCODING ENCRYPTED END_P ESCAPE EXCEPT EXCLUDING
      EXCLUSIVE EXECUTE EXISTS EXPLAIN EXTERNAL EXTRACT

      FALSE_P FETCH FIRST_P FLOAT_P FOR FORCE FOREIGN FORWARD
***************
*** 1389,1394 ****
--- 1389,1426 ----
                      n->name = NULL;
                      $$ = (Node *)n;
                  }
+             /* ALTER TABLE <name> ENABLE TRIGGER <trig> */
+             | ENABLE TRIGGER name
+                 {
+                     AlterTableCmd *n = makeNode(AlterTableCmd);
+                     n->subtype = AT_EnableTrig;
+                     n->name = $3;
+                     $$ = (Node *)n;
+                 }
+             /* ALTER TABLE <name> ENABLE TRIGGER ALL */
+             | ENABLE TRIGGER ALL
+                 {
+                     AlterTableCmd *n = makeNode(AlterTableCmd);
+                     n->subtype = AT_EnableTrig;
+                     n->name = pstrdup("*");
+                     $$ = (Node *)n;
+                 }
+             /* ALTER TABLE <name> DISABLE TRIGGER <trig> */
+             | DISABLE TRIGGER name
+                 {
+                     AlterTableCmd *n = makeNode(AlterTableCmd);
+                     n->subtype = AT_DisableTrig;
+                     n->name = $3;
+                     $$ = (Node *)n;
+                 }
+             /* ALTER TABLE <name> DISABLE TRIGGER ALL */
+             | DISABLE TRIGGER ALL
+                 {
+                     AlterTableCmd *n = makeNode(AlterTableCmd);
+                     n->subtype = AT_DisableTrig;
+                     n->name = pstrdup("*");
+                     $$ = (Node *)n;
+                 }
              | alter_rel_cmd
                  {
                      $$ = $1;
***************
*** 7960,7969 ****
--- 7992,8003 ----
              | DELETE_P
              | DELIMITER
              | DELIMITERS
+             | DISABLE
              | DOMAIN_P
              | DOUBLE_P
              | DROP
              | EACH
+             | ENABLE
              | ENCODING
              | ENCRYPTED
              | ESCAPE
diff -cr pgsql.orig/src/backend/parser/keywords.c pgsql/src/backend/parser/keywords.c
*** pgsql.orig/src/backend/parser/keywords.c    2005-06-30 05:34:14.000000000 +0900
--- pgsql/src/backend/parser/keywords.c    2005-07-01 14:38:13.000000000 +0900
***************
*** 116,121 ****
--- 116,122 ----
      {"delimiter", DELIMITER},
      {"delimiters", DELIMITERS},
      {"desc", DESC},
+     {"disable", DISABLE},
      {"distinct", DISTINCT},
      {"do", DO},
      {"domain", DOMAIN_P},
***************
*** 123,128 ****
--- 124,130 ----
      {"drop", DROP},
      {"each", EACH},
      {"else", ELSE},
+     {"enable", ENABLE},
      {"encoding", ENCODING},
      {"encrypted", ENCRYPTED},
      {"end", END_P},
diff -cr pgsql.orig/src/include/commands/trigger.h pgsql/src/include/commands/trigger.h
*** pgsql.orig/src/include/commands/trigger.h    2005-05-30 16:20:58.000000000 +0900
--- pgsql/src/include/commands/trigger.h    2005-07-01 17:14:37.000000000 +0900
***************
*** 164,169 ****
--- 164,172 ----

  extern void AfterTriggerSetState(ConstraintsSetStmt *stmt);

+ extern void EnableDisableTrigger(Relation rel,
+                                  const char *tgname,
+                                  bool enable);

  /*
   * in utils/adt/ri_triggers.c
diff -cr pgsql.orig/src/include/nodes/parsenodes.h pgsql/src/include/nodes/parsenodes.h
*** pgsql.orig/src/include/nodes/parsenodes.h    2005-06-29 04:51:24.000000000 +0900
--- pgsql/src/include/nodes/parsenodes.h    2005-07-01 14:20:14.000000000 +0900
***************
*** 822,827 ****
--- 822,829 ----
      AT_ClusterOn,                /* CLUSTER ON */
      AT_DropCluster,                /* SET WITHOUT CLUSTER */
      AT_DropOids,                /* SET WITHOUT OIDS */
+     AT_EnableTrig,                /* ENABLE TRIGGER */
+     AT_DisableTrig,                /* DISABLE TRIGGER */
      AT_SetTableSpace            /* SET TABLESPACE */
  } AlterTableType;


Re: enable/disable trigger (Re: Fwd: [HACKERS] Open items)

From
Bruce Momjian
Date:
Satoshi Nagayasu wrote:
> Bruce Momjian wrote:
> > I am not sure what to do with this patch.  It is missing dump
> > capability, there is no clause to disable all triggers on a table, and
> > it uses a table owner check when a super user check is required (because
> > of referential integrity).
> >
> > Satoshi, are you still working on it?  If not does someone else want to
> > complete it?  I realized you just started on it but the deadline is
> > soon.
>
> I've already implemented 'ENABLE/DISABLE TRIGGER ALL',
> and the superuser check has been added.  Please check it.
>
> And, I'm going to have a business trip to Sydney this weekend,
> so I'll complete pg_dump stuffs while my flight.

OK, but once our patch queue is empty, we will need to process your
patch.  My guess is that we are several days away from that.

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Re: enable/disable trigger (Re: Fwd: [HACKERS] Open items)

From
Bruce Momjian
Date:
I am waiting for pg_dump support for this patch.

---------------------------------------------------------------------------

Satoshi Nagayasu wrote:
> Bruce Momjian wrote:
> > I am not sure what to do with this patch.  It is missing dump
> > capability, there is no clause to disable all triggers on a table, and
> > it uses a table owner check when a super user check is required (because
> > of referential integrity).
> >
> > Satoshi, are you still working on it?  If not does someone else want to
> > complete it?  I realized you just started on it but the deadline is
> > soon.
>
> I've already implemented 'ENABLE/DISABLE TRIGGER ALL',
> and the superuser check has been added.  Please check it.
>
> And, I'm going to have a business trip to Sydney this weekend,
> so I'll complete pg_dump stuffs while my flight.
>
> Thank you.
> --
> NAGAYASU Satoshi <nagayasus@nttdata.co.jp>

> diff -cr pgsql.orig/src/backend/commands/tablecmds.c pgsql/src/backend/commands/tablecmds.c
> *** pgsql.orig/src/backend/commands/tablecmds.c    2005-06-28 14:08:54.000000000 +0900
> --- pgsql/src/backend/commands/tablecmds.c    2005-07-01 15:50:27.000000000 +0900
> ***************
> *** 236,241 ****
> --- 236,243 ----
>                                                 Oid newOwnerId);
>   static void ATExecClusterOn(Relation rel, const char *indexName);
>   static void ATExecDropCluster(Relation rel);
> + static void ATExecEnableDisableTrigger(Relation rel, char *trigname,
> +                                        bool enable);
>   static void ATPrepSetTableSpace(AlteredTableInfo *tab, Relation rel,
>                       char *tablespacename);
>   static void ATExecSetTableSpace(Oid tableOid, Oid newTableSpace);
> ***************
> *** 1993,1998 ****
> --- 1995,2005 ----
>               }
>               pass = AT_PASS_DROP;
>               break;
> +         case AT_EnableTrig:        /* ENABLE TRIGGER */
> +         case AT_DisableTrig:    /* DISABLE TRIGGER */
> +             ATSimplePermissions(rel, false);
> +             pass = AT_PASS_MISC;
> +             break;
>           case AT_SetTableSpace:    /* SET TABLESPACE */
>               /* This command never recurses */
>               ATPrepSetTableSpace(tab, rel, cmd->name);
> ***************
> *** 2155,2160 ****
> --- 2162,2173 ----
>                * Nothing to do here; Phase 3 does the work
>                */
>               break;
> +         case AT_EnableTrig:        /* ENABLE TRIGGER */
> +             ATExecEnableDisableTrigger(rel, cmd->name, true);
> +             break;
> +         case AT_DisableTrig:    /* DISABLE TRIGGER */
> +             ATExecEnableDisableTrigger(rel, cmd->name, false);
> +             break;
>           default:                /* oops */
>               elog(ERROR, "unrecognized alter table type: %d",
>                    (int) cmd->subtype);
> ***************
> *** 5465,5470 ****
> --- 5478,5492 ----
>   }
>
>   /*
> +  * ALTER TABLE ENABLE/DISABLE TRIGGER
> +  */
> + static void
> + ATExecEnableDisableTrigger(Relation rel, char *trigname, bool enable)
> + {
> +     EnableDisableTrigger(rel, trigname, enable);
> + }
> +
> + /*
>    * ALTER TABLE SET TABLESPACE
>    */
>   static void
> diff -cr pgsql.orig/src/backend/commands/trigger.c pgsql/src/backend/commands/trigger.c
> *** pgsql.orig/src/backend/commands/trigger.c    2005-05-30 16:20:58.000000000 +0900
> --- pgsql/src/backend/commands/trigger.c    2005-07-04 10:40:27.000000000 +0900
> ***************
> *** 3063,3065 ****
> --- 3063,3158 ----
>           afterTriggerAddEvent(new_event);
>       }
>   }
> +
> + /* ----------
> +  * EnableDisableTrigger()
> +  *
> +  *    Called by ALTER TABLE ENABLE/DISABLE TRIGGER
> +  *  to change 'tgenabled' flag in the pg_trigger.
> +  * ----------
> +  */
> + void
> + EnableDisableTrigger(Relation rel, const char *tgname, bool enable)
> + {
> +     Relation tgrel;
> +     SysScanDesc tgscan;
> +     ScanKeyData keys[2];
> +     HeapTuple tuple;
> +     int nkeys;
> +     int changed;
> +
> +     /* Permissions checks */
> +     if (!pg_class_ownercheck(RelationGetRelid(rel), GetUserId()))
> +         aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_CLASS,
> +                        RelationGetRelationName(rel));
> +
> +     if (!allowSystemTableMods && IsSystemRelation(rel))
> +         ereport(ERROR,
> +                 (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
> +                  errmsg("permission denied: \"%s\" is a system catalog",
> +                         RelationGetRelationName(rel))));
> +
> +     tgrel = heap_open(TriggerRelationId, RowExclusiveLock);
> +
> +     nkeys = 2;
> +     changed = 0;
> +     if ( strcmp(tgname, "*")==0 )
> +     {
> +         if ( !superuser() )
> +             elog(ERROR, "ENABLE/DISABLE TRIGGER ALL is allowed superuser only.");
> +
> +         nkeys = 1;
> +     }
> +
> +     ScanKeyInit(&keys[0],
> +                 Anum_pg_trigger_tgrelid,
> +                 BTEqualStrategyNumber, F_OIDEQ,
> +                 ObjectIdGetDatum(RelationGetRelid(rel)));
> +     ScanKeyInit(&keys[1],
> +                 Anum_pg_trigger_tgname,
> +                 BTEqualStrategyNumber, F_NAMEEQ,
> +                 CStringGetDatum(tgname));
> +
> +     tgscan = systable_beginscan(tgrel, TriggerRelidNameIndexId, true,
> +                                 SnapshotNow, nkeys, keys);
> +
> +     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 != enable )
> +         {
> +             if ( !superuser() && pg_trigger->tgisconstraint )
> +             {
> +                 elog(NOTICE, "Constraint trigger can be enabled/disabled "
> +                      "only by superuser.");
> +                 continue;
> +             }
> +
> +             pg_trigger->tgenabled = enable;
> +
> +             simple_heap_update(tgrel, &newtup->t_self, newtup);
> +
> +             /* Keep catalog indexes current */
> +             CatalogUpdateIndexes(tgrel, newtup);
> +
> +             changed++;
> +         }
> +
> +         heap_freetuple(newtup);
> +     }
> +     systable_endscan(tgscan);
> +
> +     heap_close(tgrel, RowExclusiveLock);
> +
> +     CommandCounterIncrement();
> +
> +     FreeTriggerDesc(rel->trigdesc);
> +     RelationBuildTriggers(rel);
> +
> +     elog(NOTICE, "%d trigger(s) on %s %s.",
> +          changed,
> +          NameStr(rel->rd_rel->relname),
> +          enable ? "enabled" : "disabled");
> + }
> diff -cr pgsql.orig/src/backend/parser/gram.y pgsql/src/backend/parser/gram.y
> *** pgsql.orig/src/backend/parser/gram.y    2005-06-30 05:34:13.000000000 +0900
> --- pgsql/src/backend/parser/gram.y    2005-07-04 10:06:08.000000000 +0900
> ***************
> *** 348,356 ****
>
>       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
>
> !     EACH ELSE ENCODING ENCRYPTED END_P ESCAPE EXCEPT EXCLUDING
>       EXCLUSIVE EXECUTE EXISTS EXPLAIN EXTERNAL EXTRACT
>
>       FALSE_P FETCH FIRST_P FLOAT_P FOR FORCE FOREIGN FORWARD
> --- 348,356 ----
>
>       DATABASE DAY_P DEALLOCATE DEC DECIMAL_P DECLARE DEFAULT DEFAULTS
>       DEFERRABLE DEFERRED DEFINER DELETE_P DELIMITER DELIMITERS
> !     DESC DISABLE DISTINCT DO DOMAIN_P DOUBLE_P DROP
>
> !     EACH ELSE ENABLE ENCODING ENCRYPTED END_P ESCAPE EXCEPT EXCLUDING
>       EXCLUSIVE EXECUTE EXISTS EXPLAIN EXTERNAL EXTRACT
>
>       FALSE_P FETCH FIRST_P FLOAT_P FOR FORCE FOREIGN FORWARD
> ***************
> *** 1389,1394 ****
> --- 1389,1426 ----
>                       n->name = NULL;
>                       $$ = (Node *)n;
>                   }
> +             /* ALTER TABLE <name> ENABLE TRIGGER <trig> */
> +             | ENABLE TRIGGER name
> +                 {
> +                     AlterTableCmd *n = makeNode(AlterTableCmd);
> +                     n->subtype = AT_EnableTrig;
> +                     n->name = $3;
> +                     $$ = (Node *)n;
> +                 }
> +             /* ALTER TABLE <name> ENABLE TRIGGER ALL */
> +             | ENABLE TRIGGER ALL
> +                 {
> +                     AlterTableCmd *n = makeNode(AlterTableCmd);
> +                     n->subtype = AT_EnableTrig;
> +                     n->name = pstrdup("*");
> +                     $$ = (Node *)n;
> +                 }
> +             /* ALTER TABLE <name> DISABLE TRIGGER <trig> */
> +             | DISABLE TRIGGER name
> +                 {
> +                     AlterTableCmd *n = makeNode(AlterTableCmd);
> +                     n->subtype = AT_DisableTrig;
> +                     n->name = $3;
> +                     $$ = (Node *)n;
> +                 }
> +             /* ALTER TABLE <name> DISABLE TRIGGER ALL */
> +             | DISABLE TRIGGER ALL
> +                 {
> +                     AlterTableCmd *n = makeNode(AlterTableCmd);
> +                     n->subtype = AT_DisableTrig;
> +                     n->name = pstrdup("*");
> +                     $$ = (Node *)n;
> +                 }
>               | alter_rel_cmd
>                   {
>                       $$ = $1;
> ***************
> *** 7960,7969 ****
> --- 7992,8003 ----
>               | DELETE_P
>               | DELIMITER
>               | DELIMITERS
> +             | DISABLE
>               | DOMAIN_P
>               | DOUBLE_P
>               | DROP
>               | EACH
> +             | ENABLE
>               | ENCODING
>               | ENCRYPTED
>               | ESCAPE
> diff -cr pgsql.orig/src/backend/parser/keywords.c pgsql/src/backend/parser/keywords.c
> *** pgsql.orig/src/backend/parser/keywords.c    2005-06-30 05:34:14.000000000 +0900
> --- pgsql/src/backend/parser/keywords.c    2005-07-01 14:38:13.000000000 +0900
> ***************
> *** 116,121 ****
> --- 116,122 ----
>       {"delimiter", DELIMITER},
>       {"delimiters", DELIMITERS},
>       {"desc", DESC},
> +     {"disable", DISABLE},
>       {"distinct", DISTINCT},
>       {"do", DO},
>       {"domain", DOMAIN_P},
> ***************
> *** 123,128 ****
> --- 124,130 ----
>       {"drop", DROP},
>       {"each", EACH},
>       {"else", ELSE},
> +     {"enable", ENABLE},
>       {"encoding", ENCODING},
>       {"encrypted", ENCRYPTED},
>       {"end", END_P},
> diff -cr pgsql.orig/src/include/commands/trigger.h pgsql/src/include/commands/trigger.h
> *** pgsql.orig/src/include/commands/trigger.h    2005-05-30 16:20:58.000000000 +0900
> --- pgsql/src/include/commands/trigger.h    2005-07-01 17:14:37.000000000 +0900
> ***************
> *** 164,169 ****
> --- 164,172 ----
>
>   extern void AfterTriggerSetState(ConstraintsSetStmt *stmt);
>
> + extern void EnableDisableTrigger(Relation rel,
> +                                  const char *tgname,
> +                                  bool enable);
>
>   /*
>    * in utils/adt/ri_triggers.c
> diff -cr pgsql.orig/src/include/nodes/parsenodes.h pgsql/src/include/nodes/parsenodes.h
> *** pgsql.orig/src/include/nodes/parsenodes.h    2005-06-29 04:51:24.000000000 +0900
> --- pgsql/src/include/nodes/parsenodes.h    2005-07-01 14:20:14.000000000 +0900
> ***************
> *** 822,827 ****
> --- 822,829 ----
>       AT_ClusterOn,                /* CLUSTER ON */
>       AT_DropCluster,                /* SET WITHOUT CLUSTER */
>       AT_DropOids,                /* SET WITHOUT OIDS */
> +     AT_EnableTrig,                /* ENABLE TRIGGER */
> +     AT_DisableTrig,                /* DISABLE TRIGGER */
>       AT_SetTableSpace            /* SET TABLESPACE */
>   } AlterTableType;
>
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 3: if posting/reading through Usenet, please send an appropriate
>        subscribe-nomail command to majordomo@postgresql.org so that your
>        message can get through to the mailing list cleanly

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Re: enable/disable trigger (Re: Fwd: [HACKERS] Open items)

From
Satoshi Nagayasu
Date:
Here is new patch with pg_dump modification.

Bruce Momjian wrote:
> I am waiting for pg_dump support for this patch.
>
> ---------------------------------------------------------------------------
>
> Satoshi Nagayasu wrote:
>
>>Bruce Momjian wrote:
>>
>>>I am not sure what to do with this patch.  It is missing dump
>>>capability, there is no clause to disable all triggers on a table, and
>>>it uses a table owner check when a super user check is required (because
>>>of referential integrity).
>>>
>>>Satoshi, are you still working on it?  If not does someone else want to
>>>complete it?  I realized you just started on it but the deadline is
>>>soon.
>>
>>I've already implemented 'ENABLE/DISABLE TRIGGER ALL',
>>and the superuser check has been added.  Please check it.
>>
>>And, I'm going to have a business trip to Sydney this weekend,
>>so I'll complete pg_dump stuffs while my flight.
>>
>>Thank you.
>>--
>>NAGAYASU Satoshi <nagayasus@nttdata.co.jp>
>
>
>>diff -cr pgsql.orig/src/backend/commands/tablecmds.c pgsql/src/backend/commands/tablecmds.c
>>*** pgsql.orig/src/backend/commands/tablecmds.c    2005-06-28 14:08:54.000000000 +0900
>>--- pgsql/src/backend/commands/tablecmds.c    2005-07-01 15:50:27.000000000 +0900
>>***************
>>*** 236,241 ****
>>--- 236,243 ----
>>                                                Oid newOwnerId);
>>  static void ATExecClusterOn(Relation rel, const char *indexName);
>>  static void ATExecDropCluster(Relation rel);
>>+ static void ATExecEnableDisableTrigger(Relation rel, char *trigname,
>>+                                        bool enable);
>>  static void ATPrepSetTableSpace(AlteredTableInfo *tab, Relation rel,
>>                      char *tablespacename);
>>  static void ATExecSetTableSpace(Oid tableOid, Oid newTableSpace);
>>***************
>>*** 1993,1998 ****
>>--- 1995,2005 ----
>>              }
>>              pass = AT_PASS_DROP;
>>              break;
>>+         case AT_EnableTrig:        /* ENABLE TRIGGER */
>>+         case AT_DisableTrig:    /* DISABLE TRIGGER */
>>+             ATSimplePermissions(rel, false);
>>+             pass = AT_PASS_MISC;
>>+             break;
>>          case AT_SetTableSpace:    /* SET TABLESPACE */
>>              /* This command never recurses */
>>              ATPrepSetTableSpace(tab, rel, cmd->name);
>>***************
>>*** 2155,2160 ****
>>--- 2162,2173 ----
>>               * Nothing to do here; Phase 3 does the work
>>               */
>>              break;
>>+         case AT_EnableTrig:        /* ENABLE TRIGGER */
>>+             ATExecEnableDisableTrigger(rel, cmd->name, true);
>>+             break;
>>+         case AT_DisableTrig:    /* DISABLE TRIGGER */
>>+             ATExecEnableDisableTrigger(rel, cmd->name, false);
>>+             break;
>>          default:                /* oops */
>>              elog(ERROR, "unrecognized alter table type: %d",
>>                   (int) cmd->subtype);
>>***************
>>*** 5465,5470 ****
>>--- 5478,5492 ----
>>  }
>>
>>  /*
>>+  * ALTER TABLE ENABLE/DISABLE TRIGGER
>>+  */
>>+ static void
>>+ ATExecEnableDisableTrigger(Relation rel, char *trigname, bool enable)
>>+ {
>>+     EnableDisableTrigger(rel, trigname, enable);
>>+ }
>>+
>>+ /*
>>   * ALTER TABLE SET TABLESPACE
>>   */
>>  static void
>>diff -cr pgsql.orig/src/backend/commands/trigger.c pgsql/src/backend/commands/trigger.c
>>*** pgsql.orig/src/backend/commands/trigger.c    2005-05-30 16:20:58.000000000 +0900
>>--- pgsql/src/backend/commands/trigger.c    2005-07-04 10:40:27.000000000 +0900
>>***************
>>*** 3063,3065 ****
>>--- 3063,3158 ----
>>          afterTriggerAddEvent(new_event);
>>      }
>>  }
>>+
>>+ /* ----------
>>+  * EnableDisableTrigger()
>>+  *
>>+  *    Called by ALTER TABLE ENABLE/DISABLE TRIGGER
>>+  *  to change 'tgenabled' flag in the pg_trigger.
>>+  * ----------
>>+  */
>>+ void
>>+ EnableDisableTrigger(Relation rel, const char *tgname, bool enable)
>>+ {
>>+     Relation tgrel;
>>+     SysScanDesc tgscan;
>>+     ScanKeyData keys[2];
>>+     HeapTuple tuple;
>>+     int nkeys;
>>+     int changed;
>>+
>>+     /* Permissions checks */
>>+     if (!pg_class_ownercheck(RelationGetRelid(rel), GetUserId()))
>>+         aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_CLASS,
>>+                        RelationGetRelationName(rel));
>>+
>>+     if (!allowSystemTableMods && IsSystemRelation(rel))
>>+         ereport(ERROR,
>>+                 (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
>>+                  errmsg("permission denied: \"%s\" is a system catalog",
>>+                         RelationGetRelationName(rel))));
>>+
>>+     tgrel = heap_open(TriggerRelationId, RowExclusiveLock);
>>+
>>+     nkeys = 2;
>>+     changed = 0;
>>+     if ( strcmp(tgname, "*")==0 )
>>+     {
>>+         if ( !superuser() )
>>+             elog(ERROR, "ENABLE/DISABLE TRIGGER ALL is allowed superuser only.");
>>+
>>+         nkeys = 1;
>>+     }
>>+
>>+     ScanKeyInit(&keys[0],
>>+                 Anum_pg_trigger_tgrelid,
>>+                 BTEqualStrategyNumber, F_OIDEQ,
>>+                 ObjectIdGetDatum(RelationGetRelid(rel)));
>>+     ScanKeyInit(&keys[1],
>>+                 Anum_pg_trigger_tgname,
>>+                 BTEqualStrategyNumber, F_NAMEEQ,
>>+                 CStringGetDatum(tgname));
>>+
>>+     tgscan = systable_beginscan(tgrel, TriggerRelidNameIndexId, true,
>>+                                 SnapshotNow, nkeys, keys);
>>+
>>+     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 != enable )
>>+         {
>>+             if ( !superuser() && pg_trigger->tgisconstraint )
>>+             {
>>+                 elog(NOTICE, "Constraint trigger can be enabled/disabled "
>>+                      "only by superuser.");
>>+                 continue;
>>+             }
>>+
>>+             pg_trigger->tgenabled = enable;
>>+
>>+             simple_heap_update(tgrel, &newtup->t_self, newtup);
>>+
>>+             /* Keep catalog indexes current */
>>+             CatalogUpdateIndexes(tgrel, newtup);
>>+
>>+             changed++;
>>+         }
>>+
>>+         heap_freetuple(newtup);
>>+     }
>>+     systable_endscan(tgscan);
>>+
>>+     heap_close(tgrel, RowExclusiveLock);
>>+
>>+     CommandCounterIncrement();
>>+
>>+     FreeTriggerDesc(rel->trigdesc);
>>+     RelationBuildTriggers(rel);
>>+
>>+     elog(NOTICE, "%d trigger(s) on %s %s.",
>>+          changed,
>>+          NameStr(rel->rd_rel->relname),
>>+          enable ? "enabled" : "disabled");
>>+ }
>>diff -cr pgsql.orig/src/backend/parser/gram.y pgsql/src/backend/parser/gram.y
>>*** pgsql.orig/src/backend/parser/gram.y    2005-06-30 05:34:13.000000000 +0900
>>--- pgsql/src/backend/parser/gram.y    2005-07-04 10:06:08.000000000 +0900
>>***************
>>*** 348,356 ****
>>
>>      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
>>
>>!     EACH ELSE ENCODING ENCRYPTED END_P ESCAPE EXCEPT EXCLUDING
>>      EXCLUSIVE EXECUTE EXISTS EXPLAIN EXTERNAL EXTRACT
>>
>>      FALSE_P FETCH FIRST_P FLOAT_P FOR FORCE FOREIGN FORWARD
>>--- 348,356 ----
>>
>>      DATABASE DAY_P DEALLOCATE DEC DECIMAL_P DECLARE DEFAULT DEFAULTS
>>      DEFERRABLE DEFERRED DEFINER DELETE_P DELIMITER DELIMITERS
>>!     DESC DISABLE DISTINCT DO DOMAIN_P DOUBLE_P DROP
>>
>>!     EACH ELSE ENABLE ENCODING ENCRYPTED END_P ESCAPE EXCEPT EXCLUDING
>>      EXCLUSIVE EXECUTE EXISTS EXPLAIN EXTERNAL EXTRACT
>>
>>      FALSE_P FETCH FIRST_P FLOAT_P FOR FORCE FOREIGN FORWARD
>>***************
>>*** 1389,1394 ****
>>--- 1389,1426 ----
>>                      n->name = NULL;
>>                      $$ = (Node *)n;
>>                  }
>>+             /* ALTER TABLE <name> ENABLE TRIGGER <trig> */
>>+             | ENABLE TRIGGER name
>>+                 {
>>+                     AlterTableCmd *n = makeNode(AlterTableCmd);
>>+                     n->subtype = AT_EnableTrig;
>>+                     n->name = $3;
>>+                     $$ = (Node *)n;
>>+                 }
>>+             /* ALTER TABLE <name> ENABLE TRIGGER ALL */
>>+             | ENABLE TRIGGER ALL
>>+                 {
>>+                     AlterTableCmd *n = makeNode(AlterTableCmd);
>>+                     n->subtype = AT_EnableTrig;
>>+                     n->name = pstrdup("*");
>>+                     $$ = (Node *)n;
>>+                 }
>>+             /* ALTER TABLE <name> DISABLE TRIGGER <trig> */
>>+             | DISABLE TRIGGER name
>>+                 {
>>+                     AlterTableCmd *n = makeNode(AlterTableCmd);
>>+                     n->subtype = AT_DisableTrig;
>>+                     n->name = $3;
>>+                     $$ = (Node *)n;
>>+                 }
>>+             /* ALTER TABLE <name> DISABLE TRIGGER ALL */
>>+             | DISABLE TRIGGER ALL
>>+                 {
>>+                     AlterTableCmd *n = makeNode(AlterTableCmd);
>>+                     n->subtype = AT_DisableTrig;
>>+                     n->name = pstrdup("*");
>>+                     $$ = (Node *)n;
>>+                 }
>>              | alter_rel_cmd
>>                  {
>>                      $$ = $1;
>>***************
>>*** 7960,7969 ****
>>--- 7992,8003 ----
>>              | DELETE_P
>>              | DELIMITER
>>              | DELIMITERS
>>+             | DISABLE
>>              | DOMAIN_P
>>              | DOUBLE_P
>>              | DROP
>>              | EACH
>>+             | ENABLE
>>              | ENCODING
>>              | ENCRYPTED
>>              | ESCAPE
>>diff -cr pgsql.orig/src/backend/parser/keywords.c pgsql/src/backend/parser/keywords.c
>>*** pgsql.orig/src/backend/parser/keywords.c    2005-06-30 05:34:14.000000000 +0900
>>--- pgsql/src/backend/parser/keywords.c    2005-07-01 14:38:13.000000000 +0900
>>***************
>>*** 116,121 ****
>>--- 116,122 ----
>>      {"delimiter", DELIMITER},
>>      {"delimiters", DELIMITERS},
>>      {"desc", DESC},
>>+     {"disable", DISABLE},
>>      {"distinct", DISTINCT},
>>      {"do", DO},
>>      {"domain", DOMAIN_P},
>>***************
>>*** 123,128 ****
>>--- 124,130 ----
>>      {"drop", DROP},
>>      {"each", EACH},
>>      {"else", ELSE},
>>+     {"enable", ENABLE},
>>      {"encoding", ENCODING},
>>      {"encrypted", ENCRYPTED},
>>      {"end", END_P},
>>diff -cr pgsql.orig/src/include/commands/trigger.h pgsql/src/include/commands/trigger.h
>>*** pgsql.orig/src/include/commands/trigger.h    2005-05-30 16:20:58.000000000 +0900
>>--- pgsql/src/include/commands/trigger.h    2005-07-01 17:14:37.000000000 +0900
>>***************
>>*** 164,169 ****
>>--- 164,172 ----
>>
>>  extern void AfterTriggerSetState(ConstraintsSetStmt *stmt);
>>
>>+ extern void EnableDisableTrigger(Relation rel,
>>+                                  const char *tgname,
>>+                                  bool enable);
>>
>>  /*
>>   * in utils/adt/ri_triggers.c
>>diff -cr pgsql.orig/src/include/nodes/parsenodes.h pgsql/src/include/nodes/parsenodes.h
>>*** pgsql.orig/src/include/nodes/parsenodes.h    2005-06-29 04:51:24.000000000 +0900
>>--- pgsql/src/include/nodes/parsenodes.h    2005-07-01 14:20:14.000000000 +0900
>>***************
>>*** 822,827 ****
>>--- 822,829 ----
>>      AT_ClusterOn,                /* CLUSTER ON */
>>      AT_DropCluster,                /* SET WITHOUT CLUSTER */
>>      AT_DropOids,                /* SET WITHOUT OIDS */
>>+     AT_EnableTrig,                /* ENABLE TRIGGER */
>>+     AT_DisableTrig,                /* DISABLE TRIGGER */
>>      AT_SetTableSpace            /* SET TABLESPACE */
>>  } AlterTableType;
>>
>>
>>
>>---------------------------(end of broadcast)---------------------------
>>TIP 3: if posting/reading through Usenet, please send an appropriate
>>       subscribe-nomail command to majordomo@postgresql.org so that your
>>       message can get through to the mailing list cleanly
>
>


--
NAGAYASU Satoshi <nagayasus@nttdata.co.jp>
diff -cr pgsql.orig/src/backend/commands/tablecmds.c pgsql/src/backend/commands/tablecmds.c
*** pgsql.orig/src/backend/commands/tablecmds.c    2005-06-28 14:08:54.000000000 +0900
--- pgsql/src/backend/commands/tablecmds.c    2005-07-06 16:36:02.000000000 +0900
***************
*** 236,241 ****
--- 236,243 ----
                                                Oid newOwnerId);
  static void ATExecClusterOn(Relation rel, const char *indexName);
  static void ATExecDropCluster(Relation rel);
+ static void ATExecEnableDisableTrigger(Relation rel, char *trigname,
+                                        bool enable);
  static void ATPrepSetTableSpace(AlteredTableInfo *tab, Relation rel,
                      char *tablespacename);
  static void ATExecSetTableSpace(Oid tableOid, Oid newTableSpace);
***************
*** 1993,1998 ****
--- 1995,2005 ----
              }
              pass = AT_PASS_DROP;
              break;
+         case AT_EnableTrig:        /* ENABLE TRIGGER */
+         case AT_DisableTrig:    /* DISABLE TRIGGER */
+             ATSimplePermissions(rel, false);
+             pass = AT_PASS_MISC;
+             break;
          case AT_SetTableSpace:    /* SET TABLESPACE */
              /* This command never recurses */
              ATPrepSetTableSpace(tab, rel, cmd->name);
***************
*** 2155,2160 ****
--- 2162,2173 ----
               * Nothing to do here; Phase 3 does the work
               */
              break;
+         case AT_EnableTrig:        /* ENABLE TRIGGER */
+             ATExecEnableDisableTrigger(rel, cmd->name, true);
+             break;
+         case AT_DisableTrig:    /* DISABLE TRIGGER */
+             ATExecEnableDisableTrigger(rel, cmd->name, false);
+             break;
          default:                /* oops */
              elog(ERROR, "unrecognized alter table type: %d",
                   (int) cmd->subtype);
***************
*** 5465,5470 ****
--- 5478,5492 ----
  }

  /*
+  * ALTER TABLE ENABLE/DISABLE TRIGGER
+  */
+ static void
+ ATExecEnableDisableTrigger(Relation rel, char *trigname, bool enable)
+ {
+     EnableDisableTrigger(rel, trigname, enable);
+ }
+
+ /*
   * ALTER TABLE SET TABLESPACE
   */
  static void
diff -cr pgsql.orig/src/backend/commands/trigger.c pgsql/src/backend/commands/trigger.c
*** pgsql.orig/src/backend/commands/trigger.c    2005-05-30 16:20:58.000000000 +0900
--- pgsql/src/backend/commands/trigger.c    2005-07-06 16:36:02.000000000 +0900
***************
*** 3063,3065 ****
--- 3063,3158 ----
          afterTriggerAddEvent(new_event);
      }
  }
+
+ /* ----------
+  * EnableDisableTrigger()
+  *
+  *    Called by ALTER TABLE ENABLE/DISABLE TRIGGER
+  *  to change 'tgenabled' flag in the pg_trigger.
+  * ----------
+  */
+ void
+ EnableDisableTrigger(Relation rel, const char *tgname, bool enable)
+ {
+     Relation tgrel;
+     SysScanDesc tgscan;
+     ScanKeyData keys[2];
+     HeapTuple tuple;
+     int nkeys;
+     int changed;
+
+     /* Permissions checks */
+     if (!pg_class_ownercheck(RelationGetRelid(rel), GetUserId()))
+         aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_CLASS,
+                        RelationGetRelationName(rel));
+
+     if (!allowSystemTableMods && IsSystemRelation(rel))
+         ereport(ERROR,
+                 (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+                  errmsg("permission denied: \"%s\" is a system catalog",
+                         RelationGetRelationName(rel))));
+
+     tgrel = heap_open(TriggerRelationId, RowExclusiveLock);
+
+     nkeys = 2;
+     changed = 0;
+     if ( strcmp(tgname, "*")==0 )
+     {
+         if ( !superuser() )
+             elog(ERROR, "ENABLE/DISABLE TRIGGER ALL is allowed superuser only.");
+
+         nkeys = 1;
+     }
+
+     ScanKeyInit(&keys[0],
+                 Anum_pg_trigger_tgrelid,
+                 BTEqualStrategyNumber, F_OIDEQ,
+                 ObjectIdGetDatum(RelationGetRelid(rel)));
+     ScanKeyInit(&keys[1],
+                 Anum_pg_trigger_tgname,
+                 BTEqualStrategyNumber, F_NAMEEQ,
+                 CStringGetDatum(tgname));
+
+     tgscan = systable_beginscan(tgrel, TriggerRelidNameIndexId, true,
+                                 SnapshotNow, nkeys, keys);
+
+     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 != enable )
+         {
+             if ( !superuser() && pg_trigger->tgisconstraint )
+             {
+                 elog(NOTICE, "Constraint trigger can be enabled/disabled "
+                      "only by superuser.");
+                 continue;
+             }
+
+             pg_trigger->tgenabled = enable;
+
+             simple_heap_update(tgrel, &newtup->t_self, newtup);
+
+             /* Keep catalog indexes current */
+             CatalogUpdateIndexes(tgrel, newtup);
+
+             changed++;
+         }
+
+         heap_freetuple(newtup);
+     }
+     systable_endscan(tgscan);
+
+     heap_close(tgrel, RowExclusiveLock);
+
+     CommandCounterIncrement();
+
+     FreeTriggerDesc(rel->trigdesc);
+     RelationBuildTriggers(rel);
+
+     elog(NOTICE, "%d trigger(s) on %s %s.",
+          changed,
+          NameStr(rel->rd_rel->relname),
+          enable ? "enabled" : "disabled");
+ }
diff -cr pgsql.orig/src/backend/parser/gram.y pgsql/src/backend/parser/gram.y
*** pgsql.orig/src/backend/parser/gram.y    2005-06-30 05:34:13.000000000 +0900
--- pgsql/src/backend/parser/gram.y    2005-07-06 16:36:02.000000000 +0900
***************
*** 348,356 ****

      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

!     EACH ELSE ENCODING ENCRYPTED END_P ESCAPE EXCEPT EXCLUDING
      EXCLUSIVE EXECUTE EXISTS EXPLAIN EXTERNAL EXTRACT

      FALSE_P FETCH FIRST_P FLOAT_P FOR FORCE FOREIGN FORWARD
--- 348,356 ----

      DATABASE DAY_P DEALLOCATE DEC DECIMAL_P DECLARE DEFAULT DEFAULTS
      DEFERRABLE DEFERRED DEFINER DELETE_P DELIMITER DELIMITERS
!     DESC DISABLE DISTINCT DO DOMAIN_P DOUBLE_P DROP

!     EACH ELSE ENABLE ENCODING ENCRYPTED END_P ESCAPE EXCEPT EXCLUDING
      EXCLUSIVE EXECUTE EXISTS EXPLAIN EXTERNAL EXTRACT

      FALSE_P FETCH FIRST_P FLOAT_P FOR FORCE FOREIGN FORWARD
***************
*** 1389,1394 ****
--- 1389,1426 ----
                      n->name = NULL;
                      $$ = (Node *)n;
                  }
+             /* ALTER TABLE <name> ENABLE TRIGGER <trig> */
+             | ENABLE TRIGGER name
+                 {
+                     AlterTableCmd *n = makeNode(AlterTableCmd);
+                     n->subtype = AT_EnableTrig;
+                     n->name = $3;
+                     $$ = (Node *)n;
+                 }
+             /* ALTER TABLE <name> ENABLE TRIGGER ALL */
+             | ENABLE TRIGGER ALL
+                 {
+                     AlterTableCmd *n = makeNode(AlterTableCmd);
+                     n->subtype = AT_EnableTrig;
+                     n->name = pstrdup("*");
+                     $$ = (Node *)n;
+                 }
+             /* ALTER TABLE <name> DISABLE TRIGGER <trig> */
+             | DISABLE TRIGGER name
+                 {
+                     AlterTableCmd *n = makeNode(AlterTableCmd);
+                     n->subtype = AT_DisableTrig;
+                     n->name = $3;
+                     $$ = (Node *)n;
+                 }
+             /* ALTER TABLE <name> DISABLE TRIGGER ALL */
+             | DISABLE TRIGGER ALL
+                 {
+                     AlterTableCmd *n = makeNode(AlterTableCmd);
+                     n->subtype = AT_DisableTrig;
+                     n->name = pstrdup("*");
+                     $$ = (Node *)n;
+                 }
              | alter_rel_cmd
                  {
                      $$ = $1;
***************
*** 7960,7969 ****
--- 7992,8003 ----
              | DELETE_P
              | DELIMITER
              | DELIMITERS
+             | DISABLE
              | DOMAIN_P
              | DOUBLE_P
              | DROP
              | EACH
+             | ENABLE
              | ENCODING
              | ENCRYPTED
              | ESCAPE
diff -cr pgsql.orig/src/backend/parser/keywords.c pgsql/src/backend/parser/keywords.c
*** pgsql.orig/src/backend/parser/keywords.c    2005-06-30 05:34:14.000000000 +0900
--- pgsql/src/backend/parser/keywords.c    2005-07-06 16:36:02.000000000 +0900
***************
*** 116,121 ****
--- 116,122 ----
      {"delimiter", DELIMITER},
      {"delimiters", DELIMITERS},
      {"desc", DESC},
+     {"disable", DISABLE},
      {"distinct", DISTINCT},
      {"do", DO},
      {"domain", DOMAIN_P},
***************
*** 123,128 ****
--- 124,130 ----
      {"drop", DROP},
      {"each", EACH},
      {"else", ELSE},
+     {"enable", ENABLE},
      {"encoding", ENCODING},
      {"encrypted", ENCRYPTED},
      {"end", END_P},
diff -cr pgsql.orig/src/bin/pg_dump/pg_dump.c pgsql/src/bin/pg_dump/pg_dump.c
*** pgsql.orig/src/bin/pg_dump/pg_dump.c    2005-06-30 12:02:56.000000000 +0900
--- pgsql/src/bin/pg_dump/pg_dump.c    2005-07-06 16:43:58.000000000 +0900
***************
*** 3273,3278 ****
--- 3273,3279 ----
                  i_tgtype,
                  i_tgnargs,
                  i_tgargs,
+                 i_tgenabled,
                  i_tgisconstraint,
                  i_tgconstrname,
                  i_tgconstrrelid,
***************
*** 3308,3314 ****
              appendPQExpBuffer(query,
                                "SELECT tgname, "
                                "tgfoid::pg_catalog.regproc as tgfname, "
!                               "tgtype, tgnargs, tgargs, "
                             "tgisconstraint, tgconstrname, tgdeferrable, "
                           "tgconstrrelid, tginitdeferred, tableoid, oid, "
                   "tgconstrrelid::pg_catalog.regclass as tgconstrrelname "
--- 3309,3315 ----
              appendPQExpBuffer(query,
                                "SELECT tgname, "
                                "tgfoid::pg_catalog.regproc as tgfname, "
!                               "tgtype, tgnargs, tgargs, tgenabled, "
                             "tgisconstraint, tgconstrname, tgdeferrable, "
                           "tgconstrrelid, tginitdeferred, tableoid, oid, "
                   "tgconstrrelid::pg_catalog.regclass as tgconstrrelname "
***************
*** 3372,3377 ****
--- 3373,3379 ----
          i_tgtype = PQfnumber(res, "tgtype");
          i_tgnargs = PQfnumber(res, "tgnargs");
          i_tgargs = PQfnumber(res, "tgargs");
+         i_tgenabled = PQfnumber(res, "tgenabled");
          i_tgisconstraint = PQfnumber(res, "tgisconstraint");
          i_tgconstrname = PQfnumber(res, "tgconstrname");
          i_tgconstrrelid = PQfnumber(res, "tgconstrrelid");
***************
*** 3394,3399 ****
--- 3396,3402 ----
              tginfo[j].tgtype = atoi(PQgetvalue(res, j, i_tgtype));
              tginfo[j].tgnargs = atoi(PQgetvalue(res, j, i_tgnargs));
              tginfo[j].tgargs = strdup(PQgetvalue(res, j, i_tgargs));
+             tginfo[j].tgenabled = *(PQgetvalue(res, j, i_tgenabled)) == 't';
              tginfo[j].tgisconstraint = *(PQgetvalue(res, j, i_tgisconstraint)) == 't';
              tginfo[j].tgdeferrable = *(PQgetvalue(res, j, i_tgdeferrable)) == 't';
              tginfo[j].tginitdeferred = *(PQgetvalue(res, j, i_tginitdeferred)) == 't';
***************
*** 7804,7809 ****
--- 7807,7820 ----
      }
      appendPQExpBuffer(query, ");\n");

+     if ( !tginfo->tgenabled )
+     {
+         appendPQExpBuffer(query, "\n");
+         appendPQExpBuffer(query, "ALTER TABLE %s DIABLE TRIGGER %s;\n",
+                           tbinfo->dobj.name,
+                           tginfo->dobj.name);
+     }
+
      ArchiveEntry(fout, tginfo->dobj.catId, tginfo->dobj.dumpId,
                   tginfo->dobj.name,
                   tbinfo->dobj.namespace->dobj.name,
diff -cr pgsql.orig/src/bin/pg_dump/pg_dump.h pgsql/src/bin/pg_dump/pg_dump.h
*** pgsql.orig/src/bin/pg_dump/pg_dump.h    2005-06-30 12:03:02.000000000 +0900
--- pgsql/src/bin/pg_dump/pg_dump.h    2005-07-06 16:36:02.000000000 +0900
***************
*** 263,268 ****
--- 263,269 ----
      TableInfo  *tgtable;        /* link to table the trigger is for */
      char       *tgfname;
      int            tgtype;
+     bool        tgenabled;
      int            tgnargs;
      char       *tgargs;
      bool        tgisconstraint;
diff -cr pgsql.orig/src/include/commands/trigger.h pgsql/src/include/commands/trigger.h
*** pgsql.orig/src/include/commands/trigger.h    2005-05-30 16:20:58.000000000 +0900
--- pgsql/src/include/commands/trigger.h    2005-07-06 16:36:02.000000000 +0900
***************
*** 164,169 ****
--- 164,172 ----

  extern void AfterTriggerSetState(ConstraintsSetStmt *stmt);

+ extern void EnableDisableTrigger(Relation rel,
+                                  const char *tgname,
+                                  bool enable);

  /*
   * in utils/adt/ri_triggers.c
diff -cr pgsql.orig/src/include/nodes/parsenodes.h pgsql/src/include/nodes/parsenodes.h
*** pgsql.orig/src/include/nodes/parsenodes.h    2005-06-29 04:51:24.000000000 +0900
--- pgsql/src/include/nodes/parsenodes.h    2005-07-06 16:36:02.000000000 +0900
***************
*** 822,827 ****
--- 822,829 ----
      AT_ClusterOn,                /* CLUSTER ON */
      AT_DropCluster,                /* SET WITHOUT CLUSTER */
      AT_DropOids,                /* SET WITHOUT OIDS */
+     AT_EnableTrig,                /* ENABLE TRIGGER */
+     AT_DisableTrig,                /* DISABLE TRIGGER */
      AT_SetTableSpace            /* SET TABLESPACE */
  } AlterTableType;


Re: enable/disable trigger (Re: Fwd: [HACKERS] Open items)

From
Alvaro Herrera
Date:
On Mon, Aug 08, 2005 at 08:07:05AM +0900, Satoshi Nagayasu wrote:
> Here is new patch with pg_dump modification.

There are a few elog() calls that should really be ereport().  Also
this message

> +     elog(NOTICE, "%d trigger(s) on %s %s.",
> +          changed,
> +          NameStr(rel->rd_rel->relname),
> +          enable ? "enabled" : "disabled");

should really be two messages (Maybe even four: disabled-plural,
disabled-singular, enabled-plural, enabled-singular)


There's a SQL typo here:

> +             appendPQExpBuffer(query, "ALTER TABLE %s DIABLE TRIGGER %s;\n",

--
Alvaro Herrera (<alvherre[a]alvh.no-ip.org>)
"La conclusión que podemos sacar de esos estudios es que
no podemos sacar ninguna conclusión de ellos" (Tanenbaum)

Re: enable/disable trigger (Re: Fwd: [HACKERS] Open items)

From
Satoshi Nagayasu
Date:
Alvaro Herrera wrote:
> There are a few elog() calls that should really be ereport().  Also
> this message

I've fixed to call ereport() on permission error.

>>+     elog(NOTICE, "%d trigger(s) on %s %s.",
>>+          changed,
>>+          NameStr(rel->rd_rel->relname),
>>+          enable ? "enabled" : "disabled");
>
>
> should really be two messages (Maybe even four: disabled-plural,
> disabled-singular, enabled-plural, enabled-singular)

What does "really be two messages" mean?

> There's a SQL typo here:
>
>>+             appendPQExpBuffer(query, "ALTER TABLE %s DIABLE TRIGGER %s;\n",

Fixed.

--
NAGAYASU Satoshi <nagayasus@nttdata.co.jp>

Re: enable/disable trigger (Re: Fwd: [HACKERS] Open items)

From
Alvaro Herrera
Date:
On Mon, Aug 08, 2005 at 02:13:28PM +0900, Satoshi Nagayasu wrote:
> Alvaro Herrera wrote:

> >>+     elog(NOTICE, "%d trigger(s) on %s %s.",
> >>+          changed,
> >>+          NameStr(rel->rd_rel->relname),
> >>+          enable ? "enabled" : "disabled");
> >
> >
> > should really be two messages (Maybe even four: disabled-plural,
> > disabled-singular, enabled-plural, enabled-singular)
>
> What does "really be two messages" mean?

I mean you should do this:

if (enabled)
{
    if (changed == 1)
        ereport("One trigger on %s enabled")
    else
        ereport("%d triggers on %d enabled")
}

etc.

--
Alvaro Herrera (<alvherre[a]alvh.no-ip.org>)
"I personally became interested in Linux while I was dating an English major
who wouldn't know an operating system if it walked up and bit him."
(Val Henson)

Re: enable/disable trigger (Re: Fwd: [HACKERS] Open items)

From
Tom Lane
Date:
Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
>> What does "really be two messages" mean?

> I mean you should do this:

> if (enabled)
> {
>     if (changed == 1)
>         ereport("One trigger on %s enabled")
>     else
>         ereport("%d triggers on %d enabled")
> }

This isn't really a gain in localizability because it assumes that there
are only singular and plural forms.  I do agree that plugging words like
"enabled" or "disabled" into a string is not good style.  Please read
the message style guidelines at
http://developer.postgresql.org/docs/postgres/error-style-guide.html
particularly the section about writing localization-friendly messages
http://developer.postgresql.org/docs/postgres/nls-programmer.html#NLS-GUIDELINES

            regards, tom lane

Re: enable/disable trigger (Re: Fwd: [HACKERS] Open items)

From
Satoshi Nagayasu
Date:
> This isn't really a gain in localizability because it assumes that there
> are only singular and plural forms.  I do agree that plugging words like
> "enabled" or "disabled" into a string is not good style.  Please read
> the message style guidelines at
> http://developer.postgresql.org/docs/postgres/error-style-guide.html
> particularly the section about writing localization-friendly messages
> http://developer.postgresql.org/docs/postgres/nls-programmer.html#NLS-GUIDELINES

I did not know about this. I'll check my code for this style.

Thanks.
--
NAGAYASU Satoshi <nagayasus@nttdata.co.jp>

Re: enable/disable trigger (Re: Fwd: [HACKERS] Open items)

From
Satoshi Nagayasu
Date:
The message format for elog() report is cleaned up.

--
NAGAYASU Satoshi <nagayasus@nttdata.co.jp>
diff -cr pgsql.orig/src/backend/commands/tablecmds.c pgsql/src/backend/commands/tablecmds.c
*** pgsql.orig/src/backend/commands/tablecmds.c    2005-06-28 14:08:54.000000000 +0900
--- pgsql/src/backend/commands/tablecmds.c    2005-08-08 13:46:44.000000000 +0900
***************
*** 236,241 ****
--- 236,243 ----
                                                Oid newOwnerId);
  static void ATExecClusterOn(Relation rel, const char *indexName);
  static void ATExecDropCluster(Relation rel);
+ static void ATExecEnableDisableTrigger(Relation rel, char *trigname,
+                                        bool enable);
  static void ATPrepSetTableSpace(AlteredTableInfo *tab, Relation rel,
                      char *tablespacename);
  static void ATExecSetTableSpace(Oid tableOid, Oid newTableSpace);
***************
*** 1993,1998 ****
--- 1995,2005 ----
              }
              pass = AT_PASS_DROP;
              break;
+         case AT_EnableTrig:        /* ENABLE TRIGGER */
+         case AT_DisableTrig:    /* DISABLE TRIGGER */
+             ATSimplePermissions(rel, false);
+             pass = AT_PASS_MISC;
+             break;
          case AT_SetTableSpace:    /* SET TABLESPACE */
              /* This command never recurses */
              ATPrepSetTableSpace(tab, rel, cmd->name);
***************
*** 2155,2160 ****
--- 2162,2173 ----
               * Nothing to do here; Phase 3 does the work
               */
              break;
+         case AT_EnableTrig:        /* ENABLE TRIGGER */
+             ATExecEnableDisableTrigger(rel, cmd->name, true);
+             break;
+         case AT_DisableTrig:    /* DISABLE TRIGGER */
+             ATExecEnableDisableTrigger(rel, cmd->name, false);
+             break;
          default:                /* oops */
              elog(ERROR, "unrecognized alter table type: %d",
                   (int) cmd->subtype);
***************
*** 5465,5470 ****
--- 5478,5492 ----
  }

  /*
+  * ALTER TABLE ENABLE/DISABLE TRIGGER
+  */
+ static void
+ ATExecEnableDisableTrigger(Relation rel, char *trigname, bool enable)
+ {
+     EnableDisableTrigger(rel, trigname, enable);
+ }
+
+ /*
   * ALTER TABLE SET TABLESPACE
   */
  static void
diff -cr pgsql.orig/src/backend/commands/trigger.c pgsql/src/backend/commands/trigger.c
*** pgsql.orig/src/backend/commands/trigger.c    2005-05-30 16:20:58.000000000 +0900
--- pgsql/src/backend/commands/trigger.c    2005-08-16 11:23:47.000000000 +0900
***************
*** 3063,3065 ****
--- 3063,3174 ----
          afterTriggerAddEvent(new_event);
      }
  }
+
+ /* ----------
+  * EnableDisableTrigger()
+  *
+  *    Called by ALTER TABLE ENABLE/DISABLE TRIGGER
+  *  to change 'tgenabled' flag in the pg_trigger.
+  * ----------
+  */
+ void
+ EnableDisableTrigger(Relation rel, const char *tgname, bool enable)
+ {
+     Relation tgrel;
+     SysScanDesc tgscan;
+     ScanKeyData keys[2];
+     HeapTuple tuple;
+     int nkeys;
+     int changed;
+
+     /* Permissions checks */
+     if (!pg_class_ownercheck(RelationGetRelid(rel), GetUserId()))
+         aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_CLASS,
+                        RelationGetRelationName(rel));
+
+     if (!allowSystemTableMods && IsSystemRelation(rel))
+         ereport(ERROR,
+                 (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+                  errmsg("permission denied: \"%s\" is a system catalog",
+                         RelationGetRelationName(rel))));
+
+     tgrel = heap_open(TriggerRelationId, RowExclusiveLock);
+
+     nkeys = 2;
+     changed = 0;
+     if ( strcmp(tgname, "*")==0 )
+     {
+         if ( !superuser() )
+             ereport(ERROR,
+                     (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+                      errmsg("ENABLE/DISABLE TRIGGER ALL is allowed superuser only.")));
+
+         nkeys = 1;
+     }
+
+     ScanKeyInit(&keys[0],
+                 Anum_pg_trigger_tgrelid,
+                 BTEqualStrategyNumber, F_OIDEQ,
+                 ObjectIdGetDatum(RelationGetRelid(rel)));
+     ScanKeyInit(&keys[1],
+                 Anum_pg_trigger_tgname,
+                 BTEqualStrategyNumber, F_NAMEEQ,
+                 CStringGetDatum(tgname));
+
+     tgscan = systable_beginscan(tgrel, TriggerRelidNameIndexId, true,
+                                 SnapshotNow, nkeys, keys);
+
+     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 != enable )
+         {
+             if ( !superuser() && pg_trigger->tgisconstraint )
+             {
+                 elog(NOTICE, "Constraint trigger can be enabled/disabled "
+                      "only by superuser.");
+                 continue;
+             }
+
+             pg_trigger->tgenabled = enable;
+
+             simple_heap_update(tgrel, &newtup->t_self, newtup);
+
+             /* Keep catalog indexes current */
+             CatalogUpdateIndexes(tgrel, newtup);
+
+             changed++;
+         }
+
+         heap_freetuple(newtup);
+     }
+     systable_endscan(tgscan);
+
+     heap_close(tgrel, RowExclusiveLock);
+
+     CommandCounterIncrement();
+
+     FreeTriggerDesc(rel->trigdesc);
+     RelationBuildTriggers(rel);
+
+     if ( enable )
+     {
+         if ( changed==1 )
+                 elog(NOTICE, "One trigger on %s is enabled.",
+                      NameStr(rel->rd_rel->relname));
+         else if ( changed>1 )
+                 elog(NOTICE, "%d triggers on %s are enabled.",
+                      changed, NameStr(rel->rd_rel->relname));
+     }
+     else
+     {
+         if ( changed==1 )
+                 elog(NOTICE, "One trigger on %s is disabled.",
+                      NameStr(rel->rd_rel->relname));
+         else if ( changed>1 )
+                 elog(NOTICE, "%d triggers on %s are disabled.",
+                      changed, NameStr(rel->rd_rel->relname));
+     }
+ }
diff -cr pgsql.orig/src/backend/parser/gram.y pgsql/src/backend/parser/gram.y
*** pgsql.orig/src/backend/parser/gram.y    2005-06-30 05:34:13.000000000 +0900
--- pgsql/src/backend/parser/gram.y    2005-08-08 13:46:44.000000000 +0900
***************
*** 348,356 ****

      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

!     EACH ELSE ENCODING ENCRYPTED END_P ESCAPE EXCEPT EXCLUDING
      EXCLUSIVE EXECUTE EXISTS EXPLAIN EXTERNAL EXTRACT

      FALSE_P FETCH FIRST_P FLOAT_P FOR FORCE FOREIGN FORWARD
--- 348,356 ----

      DATABASE DAY_P DEALLOCATE DEC DECIMAL_P DECLARE DEFAULT DEFAULTS
      DEFERRABLE DEFERRED DEFINER DELETE_P DELIMITER DELIMITERS
!     DESC DISABLE DISTINCT DO DOMAIN_P DOUBLE_P DROP

!     EACH ELSE ENABLE ENCODING ENCRYPTED END_P ESCAPE EXCEPT EXCLUDING
      EXCLUSIVE EXECUTE EXISTS EXPLAIN EXTERNAL EXTRACT

      FALSE_P FETCH FIRST_P FLOAT_P FOR FORCE FOREIGN FORWARD
***************
*** 1389,1394 ****
--- 1389,1426 ----
                      n->name = NULL;
                      $$ = (Node *)n;
                  }
+             /* ALTER TABLE <name> ENABLE TRIGGER <trig> */
+             | ENABLE TRIGGER name
+                 {
+                     AlterTableCmd *n = makeNode(AlterTableCmd);
+                     n->subtype = AT_EnableTrig;
+                     n->name = $3;
+                     $$ = (Node *)n;
+                 }
+             /* ALTER TABLE <name> ENABLE TRIGGER ALL */
+             | ENABLE TRIGGER ALL
+                 {
+                     AlterTableCmd *n = makeNode(AlterTableCmd);
+                     n->subtype = AT_EnableTrig;
+                     n->name = pstrdup("*");
+                     $$ = (Node *)n;
+                 }
+             /* ALTER TABLE <name> DISABLE TRIGGER <trig> */
+             | DISABLE TRIGGER name
+                 {
+                     AlterTableCmd *n = makeNode(AlterTableCmd);
+                     n->subtype = AT_DisableTrig;
+                     n->name = $3;
+                     $$ = (Node *)n;
+                 }
+             /* ALTER TABLE <name> DISABLE TRIGGER ALL */
+             | DISABLE TRIGGER ALL
+                 {
+                     AlterTableCmd *n = makeNode(AlterTableCmd);
+                     n->subtype = AT_DisableTrig;
+                     n->name = pstrdup("*");
+                     $$ = (Node *)n;
+                 }
              | alter_rel_cmd
                  {
                      $$ = $1;
***************
*** 7960,7969 ****
--- 7992,8003 ----
              | DELETE_P
              | DELIMITER
              | DELIMITERS
+             | DISABLE
              | DOMAIN_P
              | DOUBLE_P
              | DROP
              | EACH
+             | ENABLE
              | ENCODING
              | ENCRYPTED
              | ESCAPE
diff -cr pgsql.orig/src/backend/parser/keywords.c pgsql/src/backend/parser/keywords.c
*** pgsql.orig/src/backend/parser/keywords.c    2005-06-30 05:34:14.000000000 +0900
--- pgsql/src/backend/parser/keywords.c    2005-08-08 13:46:44.000000000 +0900
***************
*** 116,121 ****
--- 116,122 ----
      {"delimiter", DELIMITER},
      {"delimiters", DELIMITERS},
      {"desc", DESC},
+     {"disable", DISABLE},
      {"distinct", DISTINCT},
      {"do", DO},
      {"domain", DOMAIN_P},
***************
*** 123,128 ****
--- 124,130 ----
      {"drop", DROP},
      {"each", EACH},
      {"else", ELSE},
+     {"enable", ENABLE},
      {"encoding", ENCODING},
      {"encrypted", ENCRYPTED},
      {"end", END_P},
diff -cr pgsql.orig/src/bin/pg_dump/pg_dump.c pgsql/src/bin/pg_dump/pg_dump.c
*** pgsql.orig/src/bin/pg_dump/pg_dump.c    2005-06-30 12:02:56.000000000 +0900
--- pgsql/src/bin/pg_dump/pg_dump.c    2005-08-08 13:46:44.000000000 +0900
***************
*** 3273,3278 ****
--- 3273,3279 ----
                  i_tgtype,
                  i_tgnargs,
                  i_tgargs,
+                 i_tgenabled,
                  i_tgisconstraint,
                  i_tgconstrname,
                  i_tgconstrrelid,
***************
*** 3308,3314 ****
              appendPQExpBuffer(query,
                                "SELECT tgname, "
                                "tgfoid::pg_catalog.regproc as tgfname, "
!                               "tgtype, tgnargs, tgargs, "
                             "tgisconstraint, tgconstrname, tgdeferrable, "
                           "tgconstrrelid, tginitdeferred, tableoid, oid, "
                   "tgconstrrelid::pg_catalog.regclass as tgconstrrelname "
--- 3309,3315 ----
              appendPQExpBuffer(query,
                                "SELECT tgname, "
                                "tgfoid::pg_catalog.regproc as tgfname, "
!                               "tgtype, tgnargs, tgargs, tgenabled, "
                             "tgisconstraint, tgconstrname, tgdeferrable, "
                           "tgconstrrelid, tginitdeferred, tableoid, oid, "
                   "tgconstrrelid::pg_catalog.regclass as tgconstrrelname "
***************
*** 3372,3377 ****
--- 3373,3379 ----
          i_tgtype = PQfnumber(res, "tgtype");
          i_tgnargs = PQfnumber(res, "tgnargs");
          i_tgargs = PQfnumber(res, "tgargs");
+         i_tgenabled = PQfnumber(res, "tgenabled");
          i_tgisconstraint = PQfnumber(res, "tgisconstraint");
          i_tgconstrname = PQfnumber(res, "tgconstrname");
          i_tgconstrrelid = PQfnumber(res, "tgconstrrelid");
***************
*** 3394,3399 ****
--- 3396,3402 ----
              tginfo[j].tgtype = atoi(PQgetvalue(res, j, i_tgtype));
              tginfo[j].tgnargs = atoi(PQgetvalue(res, j, i_tgnargs));
              tginfo[j].tgargs = strdup(PQgetvalue(res, j, i_tgargs));
+             tginfo[j].tgenabled = *(PQgetvalue(res, j, i_tgenabled)) == 't';
              tginfo[j].tgisconstraint = *(PQgetvalue(res, j, i_tgisconstraint)) == 't';
              tginfo[j].tgdeferrable = *(PQgetvalue(res, j, i_tgdeferrable)) == 't';
              tginfo[j].tginitdeferred = *(PQgetvalue(res, j, i_tginitdeferred)) == 't';
***************
*** 7804,7809 ****
--- 7807,7820 ----
      }
      appendPQExpBuffer(query, ");\n");

+     if ( !tginfo->tgenabled )
+     {
+         appendPQExpBuffer(query, "\n");
+         appendPQExpBuffer(query, "ALTER TABLE %s DISABLE TRIGGER %s;\n",
+                           tbinfo->dobj.name,
+                           tginfo->dobj.name);
+     }
+
      ArchiveEntry(fout, tginfo->dobj.catId, tginfo->dobj.dumpId,
                   tginfo->dobj.name,
                   tbinfo->dobj.namespace->dobj.name,
diff -cr pgsql.orig/src/bin/pg_dump/pg_dump.h pgsql/src/bin/pg_dump/pg_dump.h
*** pgsql.orig/src/bin/pg_dump/pg_dump.h    2005-06-30 12:03:02.000000000 +0900
--- pgsql/src/bin/pg_dump/pg_dump.h    2005-08-08 13:46:44.000000000 +0900
***************
*** 263,268 ****
--- 263,269 ----
      TableInfo  *tgtable;        /* link to table the trigger is for */
      char       *tgfname;
      int            tgtype;
+     bool        tgenabled;
      int            tgnargs;
      char       *tgargs;
      bool        tgisconstraint;
diff -cr pgsql.orig/src/include/commands/trigger.h pgsql/src/include/commands/trigger.h
*** pgsql.orig/src/include/commands/trigger.h    2005-05-30 16:20:58.000000000 +0900
--- pgsql/src/include/commands/trigger.h    2005-08-16 11:24:07.000000000 +0900
***************
*** 164,169 ****
--- 164,172 ----

  extern void AfterTriggerSetState(ConstraintsSetStmt *stmt);

+ extern void EnableDisableTrigger(Relation rel,
+                                  const char *tgname,
+                                  bool enable);

  /*
   * in utils/adt/ri_triggers.c
diff -cr pgsql.orig/src/include/nodes/parsenodes.h pgsql/src/include/nodes/parsenodes.h
*** pgsql.orig/src/include/nodes/parsenodes.h    2005-06-29 04:51:24.000000000 +0900
--- pgsql/src/include/nodes/parsenodes.h    2005-08-08 13:46:44.000000000 +0900
***************
*** 822,827 ****
--- 822,829 ----
      AT_ClusterOn,                /* CLUSTER ON */
      AT_DropCluster,                /* SET WITHOUT CLUSTER */
      AT_DropOids,                /* SET WITHOUT OIDS */
+     AT_EnableTrig,                /* ENABLE TRIGGER */
+     AT_DisableTrig,                /* DISABLE TRIGGER */
      AT_SetTableSpace            /* SET TABLESPACE */
  } AlterTableType;


Re: enable/disable trigger (Re: Fwd: [HACKERS] Open items)

From
Bruce Momjian
Date:
Thanks, modified patch applied by Tom, with the addition of a USER
triggers only mode.

---------------------------------------------------------------------------

Satoshi Nagayasu wrote:
> The message format for elog() report is cleaned up.
>
> --
> NAGAYASU Satoshi <nagayasus@nttdata.co.jp>

> diff -cr pgsql.orig/src/backend/commands/tablecmds.c pgsql/src/backend/commands/tablecmds.c
> *** pgsql.orig/src/backend/commands/tablecmds.c    2005-06-28 14:08:54.000000000 +0900
> --- pgsql/src/backend/commands/tablecmds.c    2005-08-08 13:46:44.000000000 +0900
> ***************
> *** 236,241 ****
> --- 236,243 ----
--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Re: enable/disable trigger (Re: Fwd: [HACKERS] Open items)

From
Gavin Sherry
Date:
Attached is a patch adding regression tests for this code.

Thanks,

Gavin

On Tue, 23 Aug 2005, Bruce Momjian wrote:

>
> Thanks, modified patch applied by Tom, with the addition of a USER
> triggers only mode.
>
> ---------------------------------------------------------------------------
>
> Satoshi Nagayasu wrote:
> > The message format for elog() report is cleaned up.
> >
> > --
> > NAGAYASU Satoshi <nagayasus@nttdata.co.jp>
>
> > diff -cr pgsql.orig/src/backend/commands/tablecmds.c pgsql/src/backend/commands/tablecmds.c
> > *** pgsql.orig/src/backend/commands/tablecmds.c    2005-06-28 14:08:54.000000000 +0900
> > --- pgsql/src/backend/commands/tablecmds.c    2005-08-08 13:46:44.000000000 +0900
> > ***************
> > *** 236,241 ****
> > --- 236,243 ----
> --
>   Bruce Momjian                        |  http://candle.pha.pa.us
>   pgman@candle.pha.pa.us               |  (610) 359-1001
>   +  If your life is a hard drive,     |  13 Roberts Road
>   +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073
>
> ---------------------------(end of broadcast)---------------------------
> TIP 4: Have you searched our list archives?
>
>                http://archives.postgresql.org
>

Attachment

Re: enable/disable trigger (Re: Fwd: [HACKERS] Open items)

From
Bruce Momjian
Date:
Your patch has been added to the PostgreSQL unapplied patches list at:

    http://momjian.postgresql.org/cgi-bin/pgpatches

It will be applied as soon as one of the PostgreSQL committers reviews
and approves it.

---------------------------------------------------------------------------


Gavin Sherry wrote:
> Attached is a patch adding regression tests for this code.
>
> Thanks,
>
> Gavin
>
> On Tue, 23 Aug 2005, Bruce Momjian wrote:
>
> >
> > Thanks, modified patch applied by Tom, with the addition of a USER
> > triggers only mode.
> >
> > ---------------------------------------------------------------------------
> >
> > Satoshi Nagayasu wrote:
> > > The message format for elog() report is cleaned up.
> > >
> > > --
> > > NAGAYASU Satoshi <nagayasus@nttdata.co.jp>
> >
> > > diff -cr pgsql.orig/src/backend/commands/tablecmds.c pgsql/src/backend/commands/tablecmds.c
> > > *** pgsql.orig/src/backend/commands/tablecmds.c    2005-06-28 14:08:54.000000000 +0900
> > > --- pgsql/src/backend/commands/tablecmds.c    2005-08-08 13:46:44.000000000 +0900
> > > ***************
> > > *** 236,241 ****
> > > --- 236,243 ----
> > --
> >   Bruce Momjian                        |  http://candle.pha.pa.us
> >   pgman@candle.pha.pa.us               |  (610) 359-1001
> >   +  If your life is a hard drive,     |  13 Roberts Road
> >   +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073
> >
> > ---------------------------(end of broadcast)---------------------------
> > TIP 4: Have you searched our list archives?
> >
> >                http://archives.postgresql.org
> >

Content-Description:

[ Attachment, skipping... ]

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Re: enable/disable trigger (Re: Fwd: [HACKERS] Open items)

From
Bruce Momjian
Date:
Patch applied.  Thanks.

---------------------------------------------------------------------------


Gavin Sherry wrote:
> Attached is a patch adding regression tests for this code.
>
> Thanks,
>
> Gavin
>
> On Tue, 23 Aug 2005, Bruce Momjian wrote:
>
> >
> > Thanks, modified patch applied by Tom, with the addition of a USER
> > triggers only mode.
> >
> > ---------------------------------------------------------------------------
> >
> > Satoshi Nagayasu wrote:
> > > The message format for elog() report is cleaned up.
> > >
> > > --
> > > NAGAYASU Satoshi <nagayasus@nttdata.co.jp>
> >
> > > diff -cr pgsql.orig/src/backend/commands/tablecmds.c pgsql/src/backend/commands/tablecmds.c
> > > *** pgsql.orig/src/backend/commands/tablecmds.c    2005-06-28 14:08:54.000000000 +0900
> > > --- pgsql/src/backend/commands/tablecmds.c    2005-08-08 13:46:44.000000000 +0900
> > > ***************
> > > *** 236,241 ****
> > > --- 236,243 ----
> > --
> >   Bruce Momjian                        |  http://candle.pha.pa.us
> >   pgman@candle.pha.pa.us               |  (610) 359-1001
> >   +  If your life is a hard drive,     |  13 Roberts Road
> >   +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073
> >
> > ---------------------------(end of broadcast)---------------------------
> > TIP 4: Have you searched our list archives?
> >
> >                http://archives.postgresql.org
> >

Content-Description:

[ Attachment, skipping... ]

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073