Re: [PATCHES] Optional REFERENCES Feature in CREATE TRIGGER Command
[EMAIL PROTECTED] writes: Below were the communications between Tom and me before I implemented this project. I just did what he asked me to do. Part of it, maybe --- my point was that without any support in (at least) plpgsql, the feature is of only academic interest. There's not a lot of point in applying the patch when it does not do anything. Also, we tend to look with suspicion on such stuff because once you actually write code that uses the feature, you often find that you should have designed it a little differently. Nailing down the catalog representation in advance of having working code that does something useful with it is a good recipe for making mistakes. (To take one example, why does the patch only support one name? Don't you need two for the UPDATE case?) In any case the patch is missing documentation and pg_dump support, making it even less possible to use it for anything. It's project policy that all system catalog columns be documented in catalogs.sgml, and what's the use of DDL that won't survive a dump and reload? regards, tom lane ---(end of broadcast)--- TIP 9: the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [PATCHES] Optional REFERENCES Feature in CREATE TRIGGER Command
[EMAIL PROTECTED] wrote: I must be missing something, but AFAICS this patch doesn't actually *do* anything useful. It looks to me like you've implemented a write-only addition to the system catalogs. (And not even done a very good job of that --- no documentation, no pg_dump support.) What's the point? I just want to emphasis one thing. The point is this implementation is the fundamental, necessary, and critical preparation and step for implementing execution of standard SQL commands in trigger action in future, instead of using user-defined functions in trigger action. OK, but we usually don't add stuff to CVS until the feature is ready to be useful. -- 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 8: explain analyze is your friend
Re: [PATCHES] Optional REFERENCES Feature in CREATE TRIGGER Command
Thanks, Tom. 1. If you remembered, before I implemented this new feature, I did discuss with you. Please refer the following emails. Original Message Subject: Re: Referencing OLD/NEW Rows on Trigger Definition From:Tom Lane [EMAIL PROTECTED] Date:Mon, August 16, 2004 5:49 pm To: [EMAIL PROTECTED] -- So I will add two field in CreateTrigStmt struct, int old and char *identifier. Also, I will add one field (NameData tgidentifier in pg_trigger) and (char *identifier in Trigger struct). I realize that we need to add some fields to the pg_trigger catalog entry to store these names. What I am questioning is why the ExecTrigger functions would care about this data, and especially why we would duplicate that entire set of code in order to handle it. Duplicate code is a permanent maintenance problem and we try to avoid it. The OLD|NEW referencing tuple in Trigger definition will result in increasing the number of indexes in TriggerDesc struct to identify the right trigger. What? That has nothing whatever to do with the semantics of REFERENCING as defined in the SQL99 spec. AFAICS, REFERENCING just defines aliases within the trigger body for what we currently call the OLD and NEW records. I don't see that it has any impact whatever on which triggers get called. I realize that implementing the aliasing in plpgsql will be easier. But I don't know where to define REFERENCING clause in plpgsql. I think you'd just make an automatic alias for OLD/NEW during trigger setup. This is not a lot different from the handling of named parameters in 8.0 --- possibly you should look at that code first. Original Message Subject: Re: Referencing OLD/NEW Rows on Trigger Definition From:Tom Lane [EMAIL PROTECTED] Date:Tue, August 17, 2004 11:34 am To: [EMAIL PROTECTED] -- How can we identify the following triggers in TriggerDesc if we don't add more indexes? before_row before_row_old before_row_new Hm? What has that got to do with REFERENCING? AFAICS, REFERENCING is just a local property within a trigger, it has no impact on how many triggers there are or how you name them. I think you'd just make an automatic alias for OLD/NEW during trigger setup. This is not a lot different from the handling of named parameters in 8.0 --- possibly you should look at that code first. Do you mean still defining REFERENCING option in postgresql, not in plpgsql? No, you want the REFERENCING syntax in CREATE TRIGGER for SQL compatibility, but the point is that it's plpgsql where the actual useful implementation happens. Again, named parameters might be a useful precedent for you. The main backend (presently) does nothing with parameter names except parse them and store them in a system catalog. It's up to the PLs to do something interesting with them. 2. My implementation does not just do write-only process, it actually go one step further. I modified functions RelationBuildTriggers(), CopyTriggerDesc(), FreeTriggerDesc(), and equalTriggerDescs() in trigger.c to attach the alias to the corresponding trigger as a local variable for any new feature implementation or interesting usage in future, such as executing SQL commands in trigger action. To be honest, this is my first time to implement something in such a large open-source system. By the time I submitted the patch to Postgre, I have spent over 5 months on this project. Although I have run the regression tests and functional tests against the implementation and it passed all the test cases, it is quite possible to miss something of which should also be taken care. But I really have done my best with my knowledge of UNIX operation system, C programming skill, and with minimum helps and supports. Regards, Henry Yip [EMAIL PROTECTED] writes: The attached patch adds the optional REFERENCES syntax in CREATE TRIGGER statement to make an automatic alias for OLD/NEW record during trigger setup. The implementation of this new feature makes CREATE TRIGGER command more compatible to SQL standard, and allows the future implementation of executing SQL commands in trigger action. I must be missing something, but AFAICS this patch doesn't actually *do* anything useful. It looks to me like you've implemented a write-only addition to the system catalogs. (And not even done a very good job of that --- no documentation, no pg_dump support.) What's the point? regards, tom lane ---(end of broadcast)--- TIP 4: Don't 'kill -9' the postmaster
Re: [PATCHES] Optional REFERENCES Feature in CREATE TRIGGER Command
I must be missing something, but AFAICS this patch doesn't actually *do* anything useful. It looks to me like you've implemented a write-only addition to the system catalogs. (And not even done a very good job of that --- no documentation, no pg_dump support.) What's the point? I just want to emphasis one thing. The point is this implementation is the fundamental, necessary, and critical preparation and step for implementing execution of standard SQL commands in trigger action in future, instead of using user-defined functions in trigger action. Henry Yip ---(end of broadcast)--- TIP 3: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
[PATCHES] Optional REFERENCES Feature in CREATE TRIGGER Command
Hi,
The attached patch adds the optional REFERENCES syntax in CREATE TRIGGER
statement to make an automatic alias for OLD/NEW record during trigger
setup. The implementation of this new feature makes CREATE TRIGGER
command more compatible to SQL standard, and allows the future
implementation of executing SQL commands in trigger action.
After the implementation, the extended syntax of statement is as follows.
CREATE TRIGGER name BEFORE|AFTER
INSERT|DELETE|UPDATE [OR...] ON tablename
[REFERENCING OLD|NEW [AS] identifier]
[FOR [EACH] ROW|STATEMENT]
EXECUTE PROCEDURE funcname (arguments)
The patch will also update two columns, condition_reference_old_table and
condition_reference_new_table with alias names of the OLD/NEW record in
the Triggers table of the information schema.
$ diff -c src/include/nodes/parsenodes.h.orig src/include/nodes/parsenodes.h
*** src/include/nodes/parsenodes.h.orig 2004-07-15 10:15:08.0 -0400
--- src/include/nodes/parsenodes.h 2004-09-01 18:20:20.0 -0400
***
*** 994,999
--- 994,1000
RangeVar *relation; /* relation trigger is on */
List *funcname; /* qual. name of function to call */
List *args; /* list of (T_String) Values or
NIL */
+ List *tupleref; /* referenced tuple */
boolbefore; /* BEFORE/AFTER */
boolrow;/* ROW/STATEMENT */
charactions[4]; /* 1 to 3 of 'i', 'u', 'd', +
trailing \0 */
$ diff -c src/include/catalog/pg_trigger.h.orig src/include/catalog/pg_trigger.h
*** src/include/catalog/pg_trigger.h.orig 2004-07-15 10:11:46.0
-0400
--- src/include/catalog/pg_trigger.h2004-09-15 16:00:06.0 -0400
***
*** 29,52
*/
CATALOG(pg_trigger)
{
! Oid tgrelid;/* triggered relation */
NameDatatgname; /* trigger' name */
! Oid tgfoid; /* OID of function to
be called */
int2tgtype; /* BEFORE/AFTER
UPDATE/DELETE/INSERT
*
ROW/STATEMENT */
booltgenabled; /* trigger is enabled/disabled
*/
! booltgisconstraint; /* trigger is a RI constraint */
! NameDatatgconstrname; /* RI constraint name */
! Oid tgconstrrelid; /* RI table of foreign key
definition */
/* in the case of ON DELETE or ON UPDATE */
! booltgdeferrable; /* RI trigger is deferrable */
! booltginitdeferred; /* RI trigger is deferred initially */
int2tgnargs;/* # of extra arguments in
tgargs */
! int2vector tgattr; /* UPDATE of attr1, attr2 ... (NI) */
bytea tgargs; /*
first\000second\000tgnargs\000 */
int2tgncols;/* # of columns in tgcols */
bytea tgcols; /* column names */
} FormData_pg_trigger;
/*
--- 29,53
*/
CATALOG(pg_trigger)
{
! Oid tgrelid;/* triggered relation */
NameDatatgname; /* trigger' name */
! Oid tgfoid; /* OID of function to be called
*/
int2tgtype; /* BEFORE/AFTER
UPDATE/DELETE/INSERT
*
ROW/STATEMENT */
booltgenabled; /* trigger is enabled/disabled
*/
! booltgisconstraint; /* trigger is a RI constraint */
! NameDatatgconstrname; /* RI constraint name */
! Oid tgconstrrelid; /* RI table of foreign key
definition */
/* in the case of ON DELETE or ON UPDATE */
! booltgdeferrable; /* RI trigger is deferrable */
! booltginitdeferred; /* RI trigger is deferred
initially */
int2tgnargs;/* # of extra arguments in
tgargs */
! int2vector tgattr; /* UPDATE of attr1, attr2 ...
(NI) */
bytea tgargs; /*
first\000second\000tgnargs\000 */
int2tgncols;/* # of columns in tgcols */
bytea tgcols; /* column names */
+ NameDatatgidentifier; /* referenced tuple */
} FormData_pg_trigger;
/*
***
*** 60,87
*compiler constants for pg_trigger
*
*/
! #define Natts_pg_trigger 15
#define
Re: [PATCHES] Optional REFERENCES Feature in CREATE TRIGGER Command
[EMAIL PROTECTED] writes: The attached patch adds the optional REFERENCES syntax in CREATE TRIGGER statement to make an automatic alias for OLD/NEW record during trigger setup. The implementation of this new feature makes CREATE TRIGGER command more compatible to SQL standard, and allows the future implementation of executing SQL commands in trigger action. I must be missing something, but AFAICS this patch doesn't actually *do* anything useful. It looks to me like you've implemented a write-only addition to the system catalogs. (And not even done a very good job of that --- no documentation, no pg_dump support.) What's the point? regards, tom lane ---(end of broadcast)--- TIP 7: don't forget to increase your free space map settings
