Changes to functions and triggers

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

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

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.

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.

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.

regards, tom lane

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.

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

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.

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

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.

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

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.