plpgsql: allow RETURN with no expression #120043
Conversation
This commit adds support in the PL/pgSQL parser for RETURN statements with no expression, like `RETURN;`. Informs cockroachdb#108298 Release note: None
|
This change is |
There was a problem hiding this comment.
Very nice, thanks!
Reviewed 4 of 4 files at r1, 7 of 7 files at r2, all commit messages.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @DrewKimball and @mgartner)
pkg/sql/opt/optbuilder/plpgsql.go line 337 at r2 (raw file):
// building the variable declarations, so that expressions that reference // those variables can be type-checked. if types.IsRecordType(b.returnType) && !b.hasOutParam() {
nit: why we skip the type checking if we have an OUT param? Consider adjusting the comment below.
This commit adds support for `RETURN` statements with no expression. This applies to routines with OUT-parameters, and those with a VOID return type. This commit also refactors handling of implicit RETURN statements for the previously mentioned cases. This ensures that implicit returns are added in all possible control flow paths, including exception handlers. Informs cockroachdb#100405 Fixes cockroachdb#114849 Fixes cockroachdb#108298 Release note (sql change): Added support for `RETURN` statements with no expression for routines with OUT-parameters and routines with a `VOID` return type.
There was a problem hiding this comment.
Reviewable status:
pkg/sql/opt/optbuilder/plpgsql.go line 337 at r2 (raw file):
Previously, yuzefovich (Yahor Yuzefovich) wrote…
nit: why we skip the type checking if we have an OUT param? Consider adjusting the comment below.
OUT params will have already determined the return type at this point. I modified the comment, and also removed the one above since it's redundant.
There was a problem hiding this comment.
Reviewed 4 of 4 files at r1, 7 of 7 files at r3, 7 of 7 files at r4, all commit messages.
Reviewable status:
pkg/sql/opt/optbuilder/plpgsql.go line 1851 at r4 (raw file):
) returnWithVoidParameterErr = pgerror.New(pgcode.DatatypeMismatch, "RETURN cannot have a parameter in function returning void",
nit: RETURN cannot have an expression here and above, if you agree
pkg/sql/plpgsql/parser/plpgsql.y line 1058 at r1 (raw file):
; stmt_return: RETURN return_expr ';'
Instead of changing the lexer, would a RETURN ';' option work?
There was a problem hiding this comment.
TFYRs!
Reviewable status:
pkg/sql/opt/optbuilder/plpgsql.go line 1851 at r4 (raw file):
Previously, mgartner (Marcus Gartner) wrote…
nit:
RETURN cannot have an expressionhere and above, if you agree
Postgres does parameter instead of expression - do you think we should change that?
pkg/sql/plpgsql/parser/plpgsql.y line 1058 at r1 (raw file):
Previously, mgartner (Marcus Gartner) wrote…
Instead of changing the lexer, would a
RETURN ';'option work?
That would make the parser mad, unfortunately. PG does the same thing in pl_gram.y, FWIW.
There was a problem hiding this comment.
Reviewable status:
pkg/sql/opt/optbuilder/plpgsql.go line 1851 at r4 (raw file):
Previously, DrewKimball (Drew Kimball) wrote…
Postgres does
parameterinstead ofexpression- do you think we should change that?
Ah ok. Fine to leave as-is then.
pkg/sql/plpgsql/parser/plpgsql.y line 1058 at r1 (raw file):
Previously, DrewKimball (Drew Kimball) wrote…
That would make the parser mad, unfortunately. PG does the same thing in
pl_gram.y, FWIW.
I figured. Oh well...
|
bors r+ |
|
Timed out. |
|
bors retry |
|
Build succeeded: |
plpgsql: add parser support for RETURN with no expression
This commit adds support in the PL/pgSQL parser for RETURN statements
with no expression, like
RETURN;.plpgsql: allow RETURN with no expression
This commit adds support for
RETURNstatements with no expression.This applies to routines with OUT-parameters, and those with a VOID
return type. This commit also refactors handling of implicit RETURN
statements for the previously mentioned cases. This ensures that implicit
returns are added in all possible control flow paths, including
exception handlers.
Informs #100405
Fixes #114849
Fixes #108298
Release note (sql change): Added support for
RETURNstatements withno expression for routines with OUT-parameters and routines with a
VOIDreturn type.