From: | Neil Conway <neilc(at)samurai(dot)com> |
---|---|
To: | Satoshi Nagayasu <nagayasus(at)nttdata(dot)co(dot)jp> |
Cc: | pgsql-patches(at)postgresql(dot)org |
Subject: | Re: enable/disable trigger (Re: Fwd: [HACKERS] Open items) |
Date: | 2005-07-01 08:10:57 |
Message-ID: | 42C4FA91.3090606@samurai.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers pgsql-patches |
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
From | Date | Subject | |
---|---|---|---|
Next Message | Satoshi Nagayasu | 2005-07-01 08:32:56 | Re: enable/disable trigger (Re: Fwd: [HACKERS] Open items) |
Previous Message | Fabien COELHO | 2005-07-01 08:05:22 | Re: [PATCHES] Users/Groups -> Roles |
From | Date | Subject | |
---|---|---|---|
Next Message | Satoshi Nagayasu | 2005-07-01 08:32:56 | Re: enable/disable trigger (Re: Fwd: [HACKERS] Open items) |
Previous Message | Fabien COELHO | 2005-07-01 08:05:22 | Re: [PATCHES] Users/Groups -> Roles |