Thread: Disabling triggers / constraints
After trying to do some custom dumping/restoring, and having to resort
to the awful trick of changing the trigger counts on the catalog (the
stuff pg_dump adds), decided to add a couple little variables to control
disabling constraints and triggers. Added them to guc.c so that they
show up in the SET/SHOW list, and added checks accordingly (always at
top level, to avoid unnecessary function calls and loops). Variables are
'disable_constraints' and 'disable_triggers' and they are "false" by
default. I find it quite useful for loading large sets of data (I make
heavy use of CHECK constraints).
I am unsure wether it is right to send such an uncalled-for patch, but
seeing as this is such a simple thing that can be quite useful (large
volumes of data), I thought it could be useful.
(apply with patch -p0 from the root distro directory)
Cheers
- Jorge Pereira
*** src/backend/utils/misc/guc.c.orig 2004-05-19 18:52:08.718580424 +0100
--- src/backend/utils/misc/guc.c 2004-05-19 18:50:46.162130912 +0100
***************
*** 131,136 ****
--- 131,140 ----
int log_min_duration_statement = -1;
+ /* Control disabling of triggers (BS,AS,BR and AR) and constraints (useful on mass insert from dumps)*/
+ bool disable_constraints;
+ bool disable_triggers;
+
/*
* These variables are all dummies that don't do anything, except in some
*** src/backend/executor/execMain.c.orig 2004-05-19 18:53:14.251617888 +0100
--- src/backend/executor/execMain.c 2004-05-19 18:53:19.501819736 +0100
***************
*** 90,95 ****
--- 90,99 ----
evalPlanQual *priorepq);
static void EvalPlanQualStop(evalPlanQual *epq);
+
+ extern bool disable_constraints;
+ extern bool disable_triggers;
+
/* end of local decls */
***************
*** 1063,1068 ****
--- 1067,1073 ----
/*
* Process BEFORE EACH STATEMENT triggers
*/
+ if ( !disable_triggers )
switch (operation)
{
case CMD_UPDATE:
***************
*** 1281,1286 ****
--- 1286,1292 ----
/*
* Process AFTER EACH STATEMENT triggers
*/
+ if ( !disable_triggers )
switch (operation)
{
case CMD_UPDATE:
***************
*** 1379,1385 ****
resultRelationDesc = resultRelInfo->ri_RelationDesc;
/* BEFORE ROW INSERT Triggers */
! if (resultRelInfo->ri_TrigDesc &&
resultRelInfo->ri_TrigDesc->n_before_row[TRIGGER_EVENT_INSERT] > 0)
{
HeapTuple newtuple;
--- 1385,1391 ----
resultRelationDesc = resultRelInfo->ri_RelationDesc;
/* BEFORE ROW INSERT Triggers */
! if ( !disable_triggers && resultRelInfo->ri_TrigDesc &&
resultRelInfo->ri_TrigDesc->n_before_row[TRIGGER_EVENT_INSERT] > 0)
{
HeapTuple newtuple;
***************
*** 1405,1411 ****
/*
* Check the constraints of the tuple
*/
! if (resultRelationDesc->rd_att->constr)
ExecConstraints(resultRelInfo, slot, estate);
/*
--- 1411,1417 ----
/*
* Check the constraints of the tuple
*/
! if ( !disable_constraints && resultRelationDesc->rd_att->constr )
ExecConstraints(resultRelInfo, slot, estate);
/*
***************
*** 1431,1437 ****
ExecInsertIndexTuples(slot, &(tuple->t_self), estate, false);
/* AFTER ROW INSERT Triggers */
! ExecARInsertTriggers(estate, resultRelInfo, tuple);
}
/* ----------------------------------------------------------------
--- 1437,1444 ----
ExecInsertIndexTuples(slot, &(tuple->t_self), estate, false);
/* AFTER ROW INSERT Triggers */
! if ( !disable_triggers )
! ExecARInsertTriggers(estate, resultRelInfo, tuple);
}
/* ----------------------------------------------------------------
***************
*** 1458,1464 ****
resultRelationDesc = resultRelInfo->ri_RelationDesc;
/* BEFORE ROW DELETE Triggers */
! if (resultRelInfo->ri_TrigDesc &&
resultRelInfo->ri_TrigDesc->n_before_row[TRIGGER_EVENT_DELETE] > 0)
{
bool dodelete;
--- 1465,1471 ----
resultRelationDesc = resultRelInfo->ri_RelationDesc;
/* BEFORE ROW DELETE Triggers */
! if ( !disable_triggers && resultRelInfo->ri_TrigDesc &&
resultRelInfo->ri_TrigDesc->n_before_row[TRIGGER_EVENT_DELETE] > 0)
{
bool dodelete;
***************
*** 1525,1531 ****
*/
/* AFTER ROW DELETE Triggers */
! ExecARDeleteTriggers(estate, resultRelInfo, tupleid);
}
/* ----------------------------------------------------------------
--- 1532,1539 ----
*/
/* AFTER ROW DELETE Triggers */
! if ( !disable_triggers )
! ExecARDeleteTriggers(estate, resultRelInfo, tupleid);
}
/* ----------------------------------------------------------------
***************
*** 1569,1575 ****
resultRelationDesc = resultRelInfo->ri_RelationDesc;
/* BEFORE ROW UPDATE Triggers */
! if (resultRelInfo->ri_TrigDesc &&
resultRelInfo->ri_TrigDesc->n_before_row[TRIGGER_EVENT_UPDATE] > 0)
{
HeapTuple newtuple;
--- 1577,1583 ----
resultRelationDesc = resultRelInfo->ri_RelationDesc;
/* BEFORE ROW UPDATE Triggers */
! if ( !disable_triggers && resultRelInfo->ri_TrigDesc &&
resultRelInfo->ri_TrigDesc->n_before_row[TRIGGER_EVENT_UPDATE] > 0)
{
HeapTuple newtuple;
***************
*** 1604,1610 ****
* there's no need to do them again.)
*/
lreplace:;
! if (resultRelationDesc->rd_att->constr)
ExecConstraints(resultRelInfo, slot, estate);
/*
--- 1612,1618 ----
* there's no need to do them again.)
*/
lreplace:;
! if ( !disable_constraints && resultRelationDesc->rd_att->constr )
ExecConstraints(resultRelInfo, slot, estate);
/*
***************
*** 1678,1684 ****
ExecInsertIndexTuples(slot, &(tuple->t_self), estate, false);
/* AFTER ROW UPDATE Triggers */
! ExecARUpdateTriggers(estate, resultRelInfo, tupleid, tuple);
}
static const char *
--- 1686,1693 ----
ExecInsertIndexTuples(slot, &(tuple->t_self), estate, false);
/* AFTER ROW UPDATE Triggers */
! if ( !disable_triggers )
! ExecARUpdateTriggers(estate, resultRelInfo, tupleid, tuple);
}
static const char *
Jorge Pereira <jrp@ideiaprima.com> writes:
> ... decided to add a couple little variables to control
> disabling constraints and triggers.
I'm not of the opinion that we actually want any such thing, as it's a
blatant violation of the fundamental concept of data integrity. But in
any case not with such poor control over which triggers get suppressed.
A per-table setting with appropriate permissions checks might possibly
be acceptable.
regards, tom lane
Tom Lane wrote: >Jorge Pereira <jrp@ideiaprima.com> writes: > > >>... decided to add a couple little variables to control disabling constraints and triggers. >> >> >I'm not of the opinion that we actually want any such thing, as it's a >blatant violation of the fundamental concept of data integrity. > > I can understand your concerns. But for the sake of context for context, here's an example of the code generated by pg_dump --disable-triggers: | -- Disable triggers | UPDATE pg_catalog.pg_class SET reltriggers = 0 WHERE oid = 'table_1'::pg_catalog.regclass; | -- INSERT / UPDATE statements; | -- Enable triggers | UPDATE pg_catalog.pg_class SET reltriggers = (SELECT pg_catalog.count(*) FROM pg_catalog.pg_trigger where pg_class.oid = tgrelid) WHERE oid = 'table_1'::pg_catalog.regclass; What I propose is | SET disable_triggers=1; | -- INSERT / UPDATE statements; | SET disable_triggers=0; This is not an option for daily use, just something that can be set to allow large volumes of data that is known to be conforming to be put into the database. It is critical for datawarehousing operations, where large volumes of data (on the TB scale) already processed and validated need to be put into the database. It is quite useful also for situations where checks depend on the existence of data in the database. I can put forward a few examples if it's deemed appropriate. :) In comparison, most DBs I've experienced with (Oracle, MySQL and argh MSSQL) have some way of disablling integrity checks and triggers (mainly for loading large sets of data known to be good). I don't see a need to do it on a per-table basis, seeing as this is mostly a per-datablock need - I couldn't think of a situation where enabling it only on one table would be benefitial, as that would imply that some of tha data you are inputing might not be conforming - which in turn means you shouldn't even be using this. On the other hand, you're absolutely right in that this is clearly something that should be done only by the database owner. a) would something similar be considered if such permission check was added (for owner only)? b) would it be considered only if changeable on a per-table basis? I'm new here. :) I hope I don't come across as someone trying to force his view of things, really just trying to pass on the experience I've had before, and which led me to the despair of having to go and tweak code. ;) Good thing of OS that I could. Cheers - Jorge Pereira
Yes, agreed. I think we decided that super-user-only could disable trigger on a global basis. I prevent folks from mucking with the system tables to do it. --------------------------------------------------------------------------- Jorge Pereira wrote: > Tom Lane wrote: > > >Jorge Pereira <jrp@ideiaprima.com> writes: > > > > > >>... decided to add a couple little variables to control disabling constraints and triggers. > >> > >> > >I'm not of the opinion that we actually want any such thing, as it's a > >blatant violation of the fundamental concept of data integrity. > > > > > I can understand your concerns. But for the sake of context for context, > here's an example of the code generated by pg_dump --disable-triggers: > > | -- Disable triggers > | UPDATE pg_catalog.pg_class SET reltriggers = 0 WHERE oid = > 'table_1'::pg_catalog.regclass; > | -- INSERT / UPDATE statements; > | -- Enable triggers > | UPDATE pg_catalog.pg_class SET reltriggers = (SELECT > pg_catalog.count(*) FROM pg_catalog.pg_trigger where pg_class.oid = > tgrelid) WHERE oid = 'table_1'::pg_catalog.regclass; > > > What I propose is > > | SET disable_triggers=1; > | -- INSERT / UPDATE statements; > | SET disable_triggers=0; > > This is not an option for daily use, just something that can be set to > allow large volumes of data that is known to be conforming to be put > into the database. It is critical for datawarehousing operations, where > large volumes of data (on the TB scale) already processed and validated > need to be put into the database. It is quite useful also for situations > where checks depend on the existence of data in the database. I can put > forward a few examples if it's deemed appropriate. :) > In comparison, most DBs I've experienced with (Oracle, MySQL and argh > MSSQL) have some way of disablling integrity checks and triggers (mainly > for loading large sets of data known to be good). > > I don't see a need to do it on a per-table basis, seeing as this is > mostly a per-datablock need - I couldn't think of a situation where > enabling it only on one table would be benefitial, as that would imply > that some of tha data you are inputing might not be conforming - which > in turn means you shouldn't even be using this. > On the other hand, you're absolutely right in that this is clearly > something that should be done only by the database owner. a) would > something similar be considered if such permission check was added (for > owner only)? b) would it be considered only if changeable on a per-table > basis? > > I'm new here. :) I hope I don't come across as someone trying to force > his view of things, really just trying to pass on the experience I've > had before, and which led me to the despair of having to go and tweak > code. ;) Good thing of OS that I could. > > Cheers > - Jorge Pereira > > ---------------------------(end of broadcast)--------------------------- > TIP 2: you can get off all lists at once with the unregister command > (send "unregister YourEmailAddressHere" to majordomo@postgresql.org) > -- 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
> Yes, agreed. I think we decided that super-user-only could disable > trigger on a global basis. I prevent folks from mucking with the system > tables to do it. It should be a per-table thing: ALTER TABLE blah [ DISABLE | ENABLE ] [ALL | FOREIGN KEY ] TRIGGERS; Chris
Christopher Kings-Lynne wrote: >> Yes, agreed. I think we decided that super-user-only could disable >> trigger on a global basis. I prevent folks from mucking with the system >> tables to do it. > > > It should be a per-table thing: > > ALTER TABLE blah [ DISABLE | ENABLE ] [ALL | FOREIGN KEY ] TRIGGERS; Doing it with "alter table" seems to imply that the change is permanent (eg, the table loses checking), whereas that is most certainly not what is wanted. With a SET variable it lasts only for the session, and doesn't have to be reset manually. Assuming one wants the setting to last, as far as I can think of, an alter table would also mean either a) doing the aforementioned juggling with setting number of triggers to 0 and recounting when enabling or b) adding a new field to tables on the catalog. Solution a) I think is a nasty hack, and doesn't reflect the fact that the table *does* have triggers (which just happen to be disabled), whereas solution a) probably implies adding a field to tables in the catalog - and it's probably too much trouble? On the other hand, "alter table" has the advantage of being far more intuitive. Any other comments on this?
> Doing it with "alter table" seems to imply that the change is permanent > (eg, the table loses checking), whereas that is most certainly not what > is wanted. With a SET variable it lasts only for the session, and > doesn't have to be reset manually. Ah, then in that case, how about adding to the existing SET CONSTRAINTS command? Chris
Christopher Kings-Lynne <chriskl@familyhealth.com.au> writes:
> It should be a per-table thing:
Exactly. I don't care whether you're superuser, you should not be able
to disable all triggers, and certainly any facility provided for this
purpose should not encourage you to do that instead of disabling just
the triggers you need. Think about the triggers on pg_shadow ...
regards, tom lane
Jorge Pereira <jrp@ideiaprima.com> writes:
> I can understand your concerns. But for the sake of context for context,
> here's an example of the code generated by pg_dump --disable-triggers:
I'm well aware of what pg_dump does. Please observe that what it does
(a) affects only one table at a time, and (b) requires superuser
privileges.
> What I propose is
> | SET disable_triggers=1;
> | -- INSERT / UPDATE statements;
> | SET disable_triggers=0;
This is totally unsafe, the more so the more triggers you have.
> I don't see a need to do it on a per-table basis,
Security: a global disable may break things you didn't even know
existed. In particular I do not wish to see such a feature able to
disable triggers on tables you don't own.
regards, tom lane