Tom Lane wrote:
> Still a few bricks shy of a load I fear [...]
My apologies; thanks for the code review.
> regression=# create or replace function foo() returns int language plpgsql as $$
> regression$# begin
> regression$# return ;
> regression$# end$$;
> CREATE FUNCTION
> regression=# select foo();
> server closed the connection unexpectedly
>
> If you're going to stick a NULL into the return's expression then you'd
> better check for same when it's used.
Right. Better I think is to only allow a missing RETURN expression for
functions declared to return void anyway; that catches the mistake at
compile-time. I've implemented this behavior in the revised patch. The
error message in this scenario is still a little poor:
create function missing_return_expr() returns int as $$
begin
return ;
end;$$ language plpgsql;
ERROR: syntax error at end of input at character 8
QUERY: SELECT
CONTEXT: SQL statement in PL/PgSQL function "missing_return_expr" near
line 2
LINE 1: SELECT
^
Is it worth putting in a special-case to look for an unexpected ';' in
either the RETURN production or read_sql_construct() ?
> What I was actually intending to check when I ran into that is why some
> of the error paths in your FOR-loop rewrite use
> plpgsql_error_lineno = $1;
> while others have
> plpgsql_error_lineno = plpgsql_scanner_lineno();
> I suspect the former is more appropriate, at least for errors that
> reference the FOR variable and not some other part of the statement.
> Also why the inconsistent use of yyerror() vs ereport()?
Sorry, both of these result from sloppiness on my part: I think the code
was originally correct, but I tried to refactor some of the more complex
parts of the FOR statement production into a separate function, and
eventually wasn't able to (because of the `forvariable' union member).
When I gave up and reverted the code, I obviously wasn't careful enough.
Attached is a revised patch.
-Neil
