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

---------------------------(end of broadcast)---------------------------
TIP 5: Have you checked our extensive FAQ?

http://www.postgresql.org/docs/faq

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


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