Обсуждение: Changes to functions and triggers

Поиск
Список
Период
Сортировка

Changes to functions and triggers

От
darcy@druid.net (D'Arcy J.M. Cain)
Дата:
Something changed in 7.02 from 6.3.  I used to do this:

CREATE FUNCTION make_date()     RETURNS opaque   AS '/usr/pgsql/modules/make_date.so'    LANGUAGE 'c';
CREATE TRIGGER make_edate   BEFORE INSERT OR UPDATE ON bgroup   FOR EACH ROW   EXECUTE PROCEDURE make_date(edate, aniv,
emon,eyear);   
 

This no longer works.  I looked and the docs and it seems that this
should work instead.

CREATE FUNCTION make_date(date, int, int, int)     RETURNS opaque   AS '/usr/pgsql/modules/make_date.so'    LANGUAGE
'c';
CREATE TRIGGER make_edate   BEFORE INSERT OR UPDATE ON bgroup   FOR EACH ROW   EXECUTE PROCEDURE make_date('edate',
'aniv','emon', 'eyear');   
 

But this gives me;

ERROR:  CreateTrigger: function make_date() does not exist

Is this broken now or am I not understanding the documentation?  Why
is it looking for a make_date that takes no args?

-- 
D'Arcy J.M. Cain <darcy@{druid|vex}.net>   |  Democracy is three wolves
http://www.druid.net/darcy/                |  and a sheep voting on
+1 416 425 1212     (DoD#0082)    (eNTP)   |  what's for dinner.


Re: Changes to functions and triggers

От
Tom Lane
Дата:
darcy@druid.net (D'Arcy J.M. Cain) writes:
> Something changed in 7.02 from 6.3.  I used to do this:
> CREATE FUNCTION make_date()  
>     RETURNS opaque
>     AS '/usr/pgsql/modules/make_date.so' 
>     LANGUAGE 'c';
> CREATE TRIGGER make_edate
>     BEFORE INSERT OR UPDATE ON bgroup
>     FOR EACH ROW
>     EXECUTE PROCEDURE make_date(edate, aniv, emon, eyear);   

> This no longer works.

Details?

> I looked and the docs and it seems that this should work instead.

> CREATE FUNCTION make_date(date, int, int, int)  
>     RETURNS opaque
>     AS '/usr/pgsql/modules/make_date.so' 
>     LANGUAGE 'c';
> CREATE TRIGGER make_edate
>     BEFORE INSERT OR UPDATE ON bgroup
>     FOR EACH ROW
>     EXECUTE PROCEDURE make_date('edate', 'aniv', 'emon', 'eyear');   

No.  Trigger procedures never take explicit arguments --- whatever
you may have stated in the CREATE TRIGGER command gets passed in
in the trigger data structure.  (A pretty bizarre and ugly choice
if you ask me, but not worth breaking existing code to change...)

There's surely been a lot of changes in 7.0 that could have broken
user-written triggers, but you'll need to look to your C code to
find the problem.  What you've shown us looks fine.
        regards, tom lane


Re: Changes to functions and triggers

От
Thomas Lockhart
Дата:
> This no longer works.  I looked and the docs and it seems that this
> should work instead.
> 
> CREATE FUNCTION make_date(date, int, int, int)
>     RETURNS opaque
>     AS '/usr/pgsql/modules/make_date.so'
>     LANGUAGE 'c';
> CREATE TRIGGER make_edate
>     BEFORE INSERT OR UPDATE ON bgroup
>     FOR EACH ROW
>     EXECUTE PROCEDURE make_date('edate', 'aniv', 'emon', 'eyear');

What if you leave out the quotes on the last line above, so the column
names are actually visible? It looks like the example in the docs might
be a poor choice since the function is intended to manipulate columns,
so the text representation of the column name is being passed in.

Don't know if that is the source of your trouble though...
                 - Thomas


Re: Changes to functions and triggers

От
darcy@druid.net (D'Arcy J.M. Cain)
Дата:
Thus spake Thomas Lockhart
> > This no longer works.  I looked and the docs and it seems that this
> > should work instead.
> > 
> > CREATE FUNCTION make_date(date, int, int, int)
> >     RETURNS opaque
> >     AS '/usr/pgsql/modules/make_date.so'
> >     LANGUAGE 'c';
> > CREATE TRIGGER make_edate
> >     BEFORE INSERT OR UPDATE ON bgroup
> >     FOR EACH ROW
> >     EXECUTE PROCEDURE make_date('edate', 'aniv', 'emon', 'eyear');
> 
> What if you leave out the quotes on the last line above, so the column
> names are actually visible? It looks like the example in the docs might
> be a poor choice since the function is intended to manipulate columns,
> so the text representation of the column name is being passed in.
> 
> Don't know if that is the source of your trouble though...

Nope.  I added that after reading the web page but without them it still
has the problem.

-- 
D'Arcy J.M. Cain <darcy@{druid|vex}.net>   |  Democracy is three wolves
http://www.druid.net/darcy/                |  and a sheep voting on
+1 416 425 1212     (DoD#0082)    (eNTP)   |  what's for dinner.


Re: Changes to functions and triggers

От
darcy@druid.net (D'Arcy J.M. Cain)
Дата:
Thus spake Tom Lane
> darcy@druid.net (D'Arcy J.M. Cain) writes:
> > Something changed in 7.02 from 6.3.  I used to do this:
> > CREATE FUNCTION make_date()  
> >     RETURNS opaque
> >     AS '/usr/pgsql/modules/make_date.so' 
> >     LANGUAGE 'c';
> > CREATE TRIGGER make_edate
> >     BEFORE INSERT OR UPDATE ON bgroup
> >     FOR EACH ROW
> >     EXECUTE PROCEDURE make_date(edate, aniv, emon, eyear);   
> 
> > This no longer works.
> 
> Details?

Same error as I gave for the new version I wrote.

> > I looked and the docs and it seems that this should work instead.
> 
> > CREATE FUNCTION make_date(date, int, int, int)  
> >     RETURNS opaque
> >     AS '/usr/pgsql/modules/make_date.so' 
> >     LANGUAGE 'c';
> > CREATE TRIGGER make_edate
> >     BEFORE INSERT OR UPDATE ON bgroup
> >     FOR EACH ROW
> >     EXECUTE PROCEDURE make_date('edate', 'aniv', 'emon', 'eyear');   
> 
> No.  Trigger procedures never take explicit arguments --- whatever
> you may have stated in the CREATE TRIGGER command gets passed in
> in the trigger data structure.  (A pretty bizarre and ugly choice
> if you ask me, but not worth breaking existing code to change...)

Hmm.  Are you saying that the above is wrong?  I took it right from
the web page documentation.

> There's surely been a lot of changes in 7.0 that could have broken
> user-written triggers, but you'll need to look to your C code to
> find the problem.  What you've shown us looks fine.

Really?  That code always worked before.  Besides, it doesn't look to me
like my C code ever gets called.  The failure seems to be at the SQL
level saying that there is no function to call.

-- 
D'Arcy J.M. Cain <darcy@{druid|vex}.net>   |  Democracy is three wolves
http://www.druid.net/darcy/                |  and a sheep voting on
+1 416 425 1212     (DoD#0082)    (eNTP)   |  what's for dinner.


Re: Changes to functions and triggers

От
Tom Lane
Дата:
darcy@druid.net (D'Arcy J.M. Cain) writes:
>>>> I looked and the docs and it seems that this should work instead.
>> 
>>>> CREATE FUNCTION make_date(date, int, int, int)  
>>>> RETURNS opaque
>>>> AS '/usr/pgsql/modules/make_date.so' 
>>>> LANGUAGE 'c';
>>>> CREATE TRIGGER make_edate
>>>> BEFORE INSERT OR UPDATE ON bgroup
>>>> FOR EACH ROW
>>>> EXECUTE PROCEDURE make_date('edate', 'aniv', 'emon', 'eyear');   
>> 
>> No.  Trigger procedures never take explicit arguments --- whatever
>> you may have stated in the CREATE TRIGGER command gets passed in
>> in the trigger data structure.  (A pretty bizarre and ugly choice
>> if you ask me, but not worth breaking existing code to change...)

> Hmm.  Are you saying that the above is wrong?

Yes.

> I took it right from the web page documentation.

What web page?  http://www.postgresql.org/docs/postgres/triggers.htm
still says what it always has (complete with bad grammar ;-)):
The trigger function must be created before the trigger iscreated as a function taking no arguments and returns
opaque.
        regards, tom lane


Re: Changes to functions and triggers

От
darcy@druid.net (D'Arcy J.M. Cain)
Дата:
Thus spake Tom Lane
> darcy@druid.net (D'Arcy J.M. Cain) writes:
> >>>> I looked and the docs and it seems that this should work instead.
> >> 
> >>>> CREATE FUNCTION make_date(date, int, int, int)  
> >>>> RETURNS opaque
> >>>> AS '/usr/pgsql/modules/make_date.so' 
> >>>> LANGUAGE 'c';
> >>>> CREATE TRIGGER make_edate
> >>>> BEFORE INSERT OR UPDATE ON bgroup
> >>>> FOR EACH ROW
> >>>> EXECUTE PROCEDURE make_date('edate', 'aniv', 'emon', 'eyear');   
> >> 
> >> No.  Trigger procedures never take explicit arguments --- whatever
> >> you may have stated in the CREATE TRIGGER command gets passed in
> >> in the trigger data structure.  (A pretty bizarre and ugly choice
> >> if you ask me, but not worth breaking existing code to change...)
> 
> > Hmm.  Are you saying that the above is wrong?
> 
> Yes.
> 
> > I took it right from the web page documentation.
> 
> What web page?  http://www.postgresql.org/docs/postgres/triggers.htm
> still says what it always has (complete with bad grammar ;-)):
> 
>     The trigger function must be created before the trigger is
>     created as a function taking no arguments and returns opaque.

OK, so I went back to basically what I had before.

CREATE FUNCTION make_date()                      RETURNS opaque   AS '/usr/pgsql/modules/make_date.so'       LANGUAGE
'c';

CREATE TRIGGER make_dates   BEFORE INSERT OR UPDATE ON bgroup   FOR EACH ROW   EXECUTE PROCEDURE make_date (edate,
aniv,emon, eyear);
 

INSERT INTO bgroup (bname, client_id, actypid, aniv, emon, eyear, pmon, pyear)VALUES ('guest', 1000, 1, 1, 1, 2000, 1,
2000);

And here is what I get.

ERROR:  fmgr_info: function 24224: cache lookup failed

-- 
D'Arcy J.M. Cain <darcy@{druid|vex}.net>   |  Democracy is three wolves
http://www.druid.net/darcy/                |  and a sheep voting on
+1 416 425 1212     (DoD#0082)    (eNTP)   |  what's for dinner.


Re: Changes to functions and triggers

От
Tom Lane
Дата:
darcy@druid.net (D'Arcy J.M. Cain) writes:
> OK, so I went back to basically what I had before.

> CREATE FUNCTION make_date()                   
>     RETURNS opaque
>     AS '/usr/pgsql/modules/make_date.so'    
>     LANGUAGE 'c';

> CREATE TRIGGER make_dates
>     BEFORE INSERT OR UPDATE ON bgroup
>     FOR EACH ROW
>     EXECUTE PROCEDURE make_date (edate, aniv, emon, eyear);

> INSERT INTO bgroup (bname, client_id, actypid, aniv, emon, eyear, pmon, pyear)
>     VALUES ('guest', 1000, 1, 1, 1, 2000, 1, 2000);

Looks OK to me.

> And here is what I get.
> ERROR:  fmgr_info: function 24224: cache lookup failed

You sure you didn't fall into the same old trap of you-must-create-
the-trigger-after-the-function?  If you drop and recreate the function,
it has a new OID, so you have to drop and recreate the trigger because
it links to the function by OID.

(Someday we ought to make that work better.)
        regards, tom lane


Re: Changes to functions and triggers

От
darcy@druid.net (D'Arcy J.M. Cain)
Дата:
Thus spake Tom Lane
> darcy@druid.net (D'Arcy J.M. Cain) writes:
> > OK, so I went back to basically what I had before.
> 
> > CREATE FUNCTION make_date()                   
> >     RETURNS opaque
> >     AS '/usr/pgsql/modules/make_date.so'    
> >     LANGUAGE 'c';
> 
> > CREATE TRIGGER make_dates
> >     BEFORE INSERT OR UPDATE ON bgroup
> >     FOR EACH ROW
> >     EXECUTE PROCEDURE make_date (edate, aniv, emon, eyear);
> 
> > INSERT INTO bgroup (bname, client_id, actypid, aniv, emon, eyear, pmon, pyear)
> >     VALUES ('guest', 1000, 1, 1, 1, 2000, 1, 2000);
> 
> Looks OK to me.
> 
> > And here is what I get.
> > ERROR:  fmgr_info: function 24224: cache lookup failed
> 
> You sure you didn't fall into the same old trap of you-must-create-
> the-trigger-after-the-function?  If you drop and recreate the function,
> it has a new OID, so you have to drop and recreate the trigger because
> it links to the function by OID.

Positive.  I dropped both then did the above in the order shown.

-- 
D'Arcy J.M. Cain <darcy@{druid|vex}.net>   |  Democracy is three wolves
http://www.druid.net/darcy/                |  and a sheep voting on
+1 416 425 1212     (DoD#0082)    (eNTP)   |  what's for dinner.


Re: Changes to functions and triggers

От
Tom Lane
Дата:
darcy@druid.net (D'Arcy J.M. Cain) writes:
>> You sure you didn't fall into the same old trap of you-must-create-
>> the-trigger-after-the-function?  If you drop and recreate the function,
>> it has a new OID, so you have to drop and recreate the trigger because
>> it links to the function by OID.

> Positive.  I dropped both then did the above in the order shown.

Hm.  Is the OID shown in the error message the correct OID for your
trigger function, or not?  (Try "select oid,* from pg_proc where
proname = 'make_date'", maybe also "select * from pg_proc where
oid = 24224")

There are trigger examples in the regress tests that are hardly
different from your example, so the trigger feature is surely not
broken completely.  Maybe a platform-specific problem?  Which
version were you running again, exactly, and what configuration
options?  BTW, do the regress tests pass for you?
        regards, tom lane


Re: Changes to functions and triggers

От
darcy@druid.net (D'Arcy J.M. Cain)
Дата:
Thus spake D'Arcy J.M. Cain
> OK, so I went back to basically what I had before.
> 
> CREATE FUNCTION make_date()                   
>     RETURNS opaque
>     AS '/usr/pgsql/modules/make_date.so'    
>     LANGUAGE 'c';
> 
> CREATE TRIGGER make_dates
>     BEFORE INSERT OR UPDATE ON bgroup
>     FOR EACH ROW
>     EXECUTE PROCEDURE make_date (edate, aniv, emon, eyear);
> 
> INSERT INTO bgroup (bname, client_id, actypid, aniv, emon, eyear, pmon, pyear)
>     VALUES ('guest', 1000, 1, 1, 1, 2000, 1, 2000);
> 
> And here is what I get.
> 
> ERROR:  fmgr_info: function 24224: cache lookup failed

I must have done this wrong.  The actual error I get when I start from
scratch is this:

ERROR:  make_date (bgroup): 0 args

That message comes from my program at the start of the function.
   if (!CurrentTriggerData)       elog(ERROR, "make_date: triggers are not initialized");   if
(TRIGGER_FIRED_FOR_STATEMENT(CurrentTriggerData->tg_event))      elog(ERROR, "make_date: can't process STATEMENT
events");  if (TRIGGER_FIRED_AFTER(CurrentTriggerData->tg_event))       elog(ERROR, "make_date: must be fired before
event");
   if (TRIGGER_FIRED_BY_INSERT(CurrentTriggerData->tg_event))       rettuple = CurrentTriggerData->tg_trigtuple;   else
if(TRIGGER_FIRED_BY_UPDATE(CurrentTriggerData->tg_event))       rettuple = CurrentTriggerData->tg_newtuple;   else
elog(ERROR, "make_date: can't process DELETE events");
 
   rel = CurrentTriggerData->tg_relation;   relname = SPI_getrelname(rel);
   trigger = CurrentTriggerData->tg_trigger;
   nargs = trigger->tgnargs;   if (nargs != 4)       elog(ERROR, "make_date (%s): %d args", relname, nargs);

All I have before that is the declarations.

-- 
D'Arcy J.M. Cain <darcy@{druid|vex}.net>   |  Democracy is three wolves
http://www.druid.net/darcy/                |  and a sheep voting on
+1 416 425 1212     (DoD#0082)    (eNTP)   |  what's for dinner.


Re: Changes to functions and triggers

От
Tom Lane
Дата:
darcy@druid.net (D'Arcy J.M. Cain) writes:
> I must have done this wrong.  The actual error I get when I start from
> scratch is this:
> ERROR:  make_date (bgroup): 0 args

> That message comes from my program at the start of the function.

>      [ blah blah blah ]

>     trigger = CurrentTriggerData->tg_trigger;

>     nargs = trigger->tgnargs;
>     if (nargs != 4)
>         elog(ERROR, "make_date (%s): %d args", relname, nargs);

Hmm.  Not sure if this is the root of the problem or not, but it's
*real* dangerous to assume that the global CurrentTriggerData stays
set (the same way!) throughout your function.  You should copy
CurrentTriggerData into a local TriggerData * variable immediately
upon being called and then just use that variable.  I wonder if you
could be losing because some other trigger is getting invoked before
your routine returns...

CurrentTriggerData doesn't even exist anymore in current sources,
so doing it that way will also ease the pain of updating to 7.1 ;-)

Another thing to keep in mind is that the data structures pointed to
by CurrentTriggerData had better be treated as read-only.
        regards, tom lane


Re: Changes to functions and triggers

От
darcy@druid.net (D'Arcy J.M. Cain)
Дата:
Thus spake Zeugswetter Andreas
> > darcy@druid.net (D'Arcy J.M. Cain) writes:
> > >     nargs = trigger->tgnargs;
> > >     if (nargs != 4)
> > >         elog(ERROR, "make_date (%s): %d args", relname, nargs);
> 
> The simple answer is, that your procedure does not take four arguments.
> The old and new tuple are passed implicitly to your procedure,
> they don't show up in the argument list.

Right.  That's why the function takes void as its parameter list.  (I hadn't
shown that in my message.)  The code above finds the args from the global
environment.

In fact, my problem was, I think, that I needed to use the SPI_connect() and
SPI_finish() functions which I don't think were available when I first wrote
the function.  I added those, recompiled and all now works.  It just took
me a while to realize that the problem was in the C code and not the SQL
statements to use it.

> This is also the reason your code works if you add four "dummy" string 
> arguments (you can test that by supplying random values not the column names
> in your create trigger statement).

Nope.  Without the fix above it didn't work no matter what I tried.

Thanks for everyone's help.  Now I can move on to the operator defining
problem but that's a subject for another message.

It's funny but the two areas inthe new version that I am having trouble
with are the two that I originally helped document.  :-)

-- 
D'Arcy J.M. Cain <darcy@{druid|vex}.net>   |  Democracy is three wolves
http://www.druid.net/darcy/                |  and a sheep voting on
+1 416 425 1212     (DoD#0082)    (eNTP)   |  what's for dinner.