Thread: enable/disable trigger (Re: Fwd: [HACKERS] Open items)
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;
>>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
> 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>
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
> 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>
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
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;
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;
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
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
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>
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
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
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
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
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
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
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;
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
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
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;
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)
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>
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)
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
> 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>
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;
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
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
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
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