On mån, 2011-02-28 at 19:07 +0200, Peter Eisentraut wrote:
> PL/pgSQL trigger functions currently require a value to be returned,
> even though that value is not used for anything in case of a trigger
> fired AFTER. I was wondering if we could relax that. It would make
> things a bit more robust and produce clearer PL/pgSQL code. The
> specific case I'm concerned about is that a trigger function could
> accidentally be run in a BEFORE trigger even though it was not meant for
> that. It is common practice that trigger functions for AFTER triggers
> return NULL, which would have unpleasant effects if used in a BEFORE
> trigger.
>
> I think it is very uncommon to have the same function usable for BEFORE
> and AFTER triggers, so it would be valuable to have coding support
> specifically for AFTER triggers. We could just allow RETURN without
> argument, or perhaps no RETURN at all.
Here is a patch for that.
One thing that I'm concerned about with this is that it treats a plain
RETURN in a BEFORE trigger as RETURN NULL, whereas arguably it should be
an error. I haven't found a good way to handle that yet, but I'll keep
looking.
diff --git i/doc/src/sgml/plpgsql.sgml w/doc/src/sgml/plpgsql.sgml
index f33cef5..203cb29 100644
--- i/doc/src/sgml/plpgsql.sgml
+++ w/doc/src/sgml/plpgsql.sgml
@@ -1559,7 +1559,7 @@ END;
<title><command>RETURN</></title>
<synopsis>
-RETURN <replaceable>expression</replaceable>;
+RETURN <optional> <replaceable>expression</replaceable> </optional>;
</synopsis>
<para>
@@ -1592,6 +1592,13 @@ RETURN <replaceable>expression</replaceable>;
</para>
<para>
+ A trigger function used in an <literal>AFTER</literal> row trigger or in
+ a statement trigger can also use <command>RETURN</command> without
+ expression. (The result of any supplied expression would be ignored.)
+ See <xref linkend="plpgsql-trigger"> for details.
+ </para>
+
+ <para>
The return value of a function cannot be left undefined. If
control reaches the end of the top-level block of the function
without hitting a <command>RETURN</command> statement, a run-time
@@ -3511,9 +3518,9 @@ RAISE unique_violation USING MESSAGE = 'Duplicate user ID: ' || user_id;
</para>
<para>
- A trigger function must return either <symbol>NULL</symbol> or a
+ A trigger function can return either <symbol>NULL</symbol> or a
record/row value having exactly the structure of the table the
- trigger was fired for.
+ trigger was fired for, as follows.
</para>
<para>
@@ -3558,11 +3565,17 @@ RAISE unique_violation USING MESSAGE = 'Duplicate user ID: ' || user_id;
</para>
<para>
- The return value of a row-level trigger
- fired <literal>AFTER</literal> or a statement-level trigger
- fired <literal>BEFORE</> or <literal>AFTER</> is
- always ignored; it might as well be null. However, any of these types of
- triggers might still abort the entire operation by raising an error.
+ The return value of a row-level trigger fired <literal>AFTER</literal> or a
+ statement-level trigger fired <literal>BEFORE</> or <literal>AFTER</> is
+ always ignored. In these cases, the <literal>RETURN</literal> statement
+ can be used without argument or omitted altogether. (A trigger function
+ that is intended to be used in an <literal>AFTER</literal> trigger or a
+ statement-level trigger and supplies a return value that is ignored would
+ misbehave, possibly silently, if accidentally used in
+ a <literal>BEFORE</literal> or <literal>INSTEAD OF</literal> row-level
+ trigger. So it might be better not to supply a dummy return value.)
+ However, any of these types of triggers might still abort the entire
+ operation by raising an error.
</para>
<para>
@@ -3655,15 +3668,11 @@ CREATE OR REPLACE FUNCTION process_emp_audit() RETURNS TRIGGER AS $emp_audit$
--
IF (TG_OP = 'DELETE') THEN
INSERT INTO emp_audit SELECT 'D', now(), user, OLD.*;
- RETURN OLD;
ELSIF (TG_OP = 'UPDATE') THEN
INSERT INTO emp_audit SELECT 'U', now(), user, NEW.*;
- RETURN NEW;
ELSIF (TG_OP = 'INSERT') THEN
INSERT INTO emp_audit SELECT 'I', now(), user, NEW.*;
- RETURN NEW;
END IF;
- RETURN NULL; -- result is ignored since this is an AFTER trigger
END;
$emp_audit$ LANGUAGE plpgsql;
@@ -3885,9 +3894,6 @@ AS $maint_sales_summary_bytime$
-- do nothing
END;
END LOOP insert_update;
-
- RETURN NULL;
-
END;
$maint_sales_summary_bytime$ LANGUAGE plpgsql;
diff --git i/src/pl/plpgsql/src/gram.y w/src/pl/plpgsql/src/gram.y
index 8c4c2f7..a7adb5c 100644
--- i/src/pl/plpgsql/src/gram.y
+++ w/src/pl/plpgsql/src/gram.y
@@ -2868,6 +2868,8 @@ make_return_stmt(int location)
}
else if (plpgsql_curr_compile->fn_retistuple)
{
+ bool saw_semicolon = false;
+
switch (yylex())
{
case K_NULL:
@@ -2885,6 +2887,16 @@ make_return_stmt(int location)
parser_errposition(yylloc)));
break;
+ case ';':
+ if (plpgsql_curr_compile->fn_is_trigger)
+ saw_semicolon = true;
+ else
+ ereport(ERROR,
+ (errcode(ERRCODE_DATATYPE_MISMATCH),
+ errmsg("RETURN must specify a record or row variable in function returning row"),
+ parser_errposition(yylloc)));
+ break;
+
default:
ereport(ERROR,
(errcode(ERRCODE_DATATYPE_MISMATCH),
@@ -2892,7 +2904,7 @@ make_return_stmt(int location)
parser_errposition(yylloc)));
break;
}
- if (yylex() != ';')
+ if (!saw_semicolon && yylex() != ';')
yyerror("syntax error");
}
else
diff --git i/src/pl/plpgsql/src/pl_exec.c w/src/pl/plpgsql/src/pl_exec.c
index 717ad79..2236f72 100644
--- i/src/pl/plpgsql/src/pl_exec.c
+++ w/src/pl/plpgsql/src/pl_exec.c
@@ -701,10 +701,11 @@ plpgsql_exec_trigger(PLpgSQL_function *func,
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
errmsg("CONTINUE cannot be used outside a loop")));
- else
+ else if (!(TRIGGER_FIRED_FOR_STATEMENT(trigdata->tg_event) || TRIGGER_FIRED_AFTER(trigdata->tg_event)))
ereport(ERROR,
(errcode(ERRCODE_S_R_E_FUNCTION_EXECUTED_NO_RETURN_STATEMENT),
- errmsg("control reached end of trigger procedure without RETURN")));
+ errmsg("control reached end of trigger procedure without RETURN"),
+ errdetail("A trigger procedure used in a row-level trigger that is not fired AFTER must return a value.")));
}
estate.err_stmt = NULL;
diff --git i/src/test/regress/expected/plpgsql.out w/src/test/regress/expected/plpgsql.out
index 238bf5f..cda9928 100644
--- i/src/test/regress/expected/plpgsql.out
+++ w/src/test/regress/expected/plpgsql.out
@@ -4571,3 +4571,49 @@ ERROR: value for domain orderedarray violates check constraint "sorted"
CONTEXT: PL/pgSQL function "testoa" line 5 at assignment
drop function arrayassign1();
drop function testoa(x1 int, x2 int, x3 int);
+--
+-- Additional triggers tests
+--
+create table test_triggers (a int, b text);
+-- trigger function without return statement
+create function say_something() returns trigger
+language plpgsql as $$
+begin
+ raise notice '%', tg_argv[0];
+end$$;
+-- trigger function with RETURN without argument
+create function say_something2() returns trigger
+language plpgsql as $$
+begin
+ raise notice '%', tg_argv[0];
+ return;
+end$$;
+create trigger say_hello_s before insert on test_triggers for each statement execute procedure say_something('statement trigger');
+create trigger say_hello_ra after update on test_triggers for each row execute procedure say_something('after row trigger');
+create trigger say_hello_ra2 after update on test_triggers for each row execute procedure say_something2('another after row trigger');
+insert into test_triggers values (1, 'foo');
+NOTICE: statement trigger
+update test_triggers set b = 'bar' where a = 1;
+NOTICE: after row trigger
+NOTICE: another after row trigger
+create trigger say_hello_rb before update on test_triggers for each row execute procedure say_something('before row trigger'); -- will fail
+update test_triggers set b = 'baz' where a = 1;
+NOTICE: before row trigger
+ERROR: control reached end of trigger procedure without RETURN
+DETAIL: A trigger procedure used in a row-level trigger that is not fired AFTER must return a value.
+CONTEXT: PL/pgSQL function "say_something"
+drop trigger say_hello_rb on test_triggers;
+-- XXX plain RETURN behaves like RETURN NULL
+create trigger say_hello_rb2 before update on test_triggers for each row execute procedure say_something2('another before row trigger');
+update test_triggers set b = 'baz' where a = 1;
+NOTICE: another before row trigger
+select * from test_triggers order by a, b;
+ a | b
+---+-----
+ 1 | bar
+(1 row)
+
+drop trigger say_hello_rb2 on test_triggers;
+drop table test_triggers;
+drop function say_something();
+drop function say_something2();
diff --git i/src/test/regress/sql/plpgsql.sql w/src/test/regress/sql/plpgsql.sql
index b47c2de..167b1fe 100644
--- i/src/test/regress/sql/plpgsql.sql
+++ w/src/test/regress/sql/plpgsql.sql
@@ -3600,3 +3600,44 @@ select testoa(1,2,1); -- fail at update
drop function arrayassign1();
drop function testoa(x1 int, x2 int, x3 int);
+
+--
+-- Additional triggers tests
+--
+
+create table test_triggers (a int, b text);
+
+-- trigger function without return statement
+create function say_something() returns trigger
+language plpgsql as $$
+begin
+ raise notice '%', tg_argv[0];
+end$$;
+
+-- trigger function with RETURN without argument
+create function say_something2() returns trigger
+language plpgsql as $$
+begin
+ raise notice '%', tg_argv[0];
+ return;
+end$$;
+
+create trigger say_hello_s before insert on test_triggers for each statement execute procedure say_something('statement trigger');
+create trigger say_hello_ra after update on test_triggers for each row execute procedure say_something('after row trigger');
+create trigger say_hello_ra2 after update on test_triggers for each row execute procedure say_something2('another after row trigger');
+insert into test_triggers values (1, 'foo');
+update test_triggers set b = 'bar' where a = 1;
+
+create trigger say_hello_rb before update on test_triggers for each row execute procedure say_something('before row trigger'); -- will fail
+update test_triggers set b = 'baz' where a = 1;
+drop trigger say_hello_rb on test_triggers;
+
+-- XXX plain RETURN behaves like RETURN NULL
+create trigger say_hello_rb2 before update on test_triggers for each row execute procedure say_something2('another before row trigger');
+update test_triggers set b = 'baz' where a = 1;
+select * from test_triggers order by a, b;
+drop trigger say_hello_rb2 on test_triggers;
+
+drop table test_triggers;
+drop function say_something();
+drop function say_something2();
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
