Re: Optional REFERENCES Feature in CREATE TRIGGER Command - Mailing list pgsql-patches

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" <tgl@sss.pgh.pa.us>
Date:    Mon, August 16, 2004 5:49 pm
To:      hyip@site.uottawa.ca
--------------------------------------------------------------------------

> 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" <tgl@sss.pgh.pa.us>
Date:    Tue, August 17, 2004 11:34 am
To:      hyip@site.uottawa.ca
--------------------------------------------------------------------------

> 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


> hyip@site.uottawa.ca 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
>