Re: enable/disable trigger (Re: Fwd: [HACKERS] Open items) - Mailing list pgsql-patches

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