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$$;
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 $$
return ;
end;$$ language plpgsql;
ERROR: syntax error at end of input at character 8
CONTEXT: SQL statement in PL/PgSQL function "missing_return_expr" near line 2

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.


Attachment: plpgsql_cleanup-34.patch.gz
Description: application/gzip

---------------------------(end of broadcast)---------------------------
TIP 5: Have you checked our extensive FAQ?


Reply via email to