Greetings fellow elephants,
I would humbly like to submit for your consideration my proposal for
alleviating pain caused by one of the most annoying footguns in
PL/PgSQL: the behaviour of SELECT .. INTO when the query returns more
than one row. Some of you might know that no exception is raised in
this case (as opposed to INSERT/UPDATE/DELETE .. INTO, all of them
yielding TOO_MANY_ROWS), which can hide subtle bugs in queries if during
testing the query always returns only one row or the "correct" one
happens to be picked up every time. Additionally, the row_count() after
execution is always going to be either 0 or 1, so even if you want to
explicitly guard against potentially broken queries, you can't do so!
So I added the following compile-time option:
set plpgsql.consistent_into to true;
create or replace function footest() returns void as $$
declare
x int;
begin
-- too many rows
select 1 from foo into x;
end$$ language plpgsql;
select footest();
ERROR: query returned more than one row
It defaults to false to preserve full backwards compatibility. Also
turning it on makes the executor try and find two rows, so it might have
an effect on performance as well. The patch, as currently written, also
changes the behaviour of EXECUTE .. INTO, but I don't feel strongly
about whether that should be affected as well or not.
Regards,
Marko Tiikkaja
*** a/doc/src/sgml/plpgsql.sgml
--- b/doc/src/sgml/plpgsql.sgml
***************
*** 1043,1050 **** DELETE ... RETURNING <replaceable>expressions</replaceable>
INTO <optional>STRIC
clause, then <replaceable>target</replaceable> will be set to the first
row returned by the query, or to nulls if the query returned no rows.
(Note that <quote>the first row</> is not
! well-defined unless you've used <literal>ORDER BY</>.) Any result rows
! after the first row are discarded.
You can check the special <literal>FOUND</literal> variable (see
<xref linkend="plpgsql-statements-diagnostics">) to
determine whether a row was returned:
--- 1043,1053 ----
clause, then <replaceable>target</replaceable> will be set to the first
row returned by the query, or to nulls if the query returned no rows.
(Note that <quote>the first row</> is not
! well-defined unless you've used <literal>ORDER BY</>.) By default,
! any result rows after the first row are discarded. However, if the
! <literal>consistent_into</> compile option has been enabled and the
! query returns more than one row, the run-time error
! <literal>TOO_MANY_ROWS</> will be reported.
You can check the special <literal>FOUND</literal> variable (see
<xref linkend="plpgsql-statements-diagnostics">) to
determine whether a row was returned:
***************
*** 1172,1178 **** EXECUTE <replaceable
class="command">command-string</replaceable> <optional> INT
record variable is used, it will configure itself to match the
result structure automatically). If multiple rows are returned,
only the first will be assigned to the <literal>INTO</literal>
! variable. If no rows are returned, NULL is assigned to the
<literal>INTO</literal> variable(s). If no <literal>INTO</literal>
clause is specified, the query results are discarded.
</para>
--- 1175,1183 ----
record variable is used, it will configure itself to match the
result structure automatically). If multiple rows are returned,
only the first will be assigned to the <literal>INTO</literal>
! variable, unless <literal>consistent_into</literal> has been
! enabled, in which case <literal>TOO_MANY_ROWS</literal> is raised.
! If no rows are returned, NULL is assigned to the
<literal>INTO</literal> variable(s). If no <literal>INTO</literal>
clause is specified, the query results are discarded.
</para>
*** a/src/pl/plpgsql/src/pl_comp.c
--- b/src/pl/plpgsql/src/pl_comp.c
***************
*** 352,357 **** do_compile(FunctionCallInfo fcinfo,
--- 352,358 ----
function->out_param_varno = -1; /* set up for no OUT param */
function->resolve_option = plpgsql_variable_conflict;
function->print_strict_params = plpgsql_print_strict_params;
+ function->consistent_into = plpgsql_consistent_into;
if (is_dml_trigger)
function->fn_is_trigger = PLPGSQL_DML_TRIGGER;
***************
*** 849,854 **** plpgsql_compile_inline(char *proc_source)
--- 850,856 ----
function->out_param_varno = -1; /* set up for no OUT param */
function->resolve_option = plpgsql_variable_conflict;
function->print_strict_params = plpgsql_print_strict_params;
+ function->consistent_into = plpgsql_consistent_into;
plpgsql_ns_init();
plpgsql_ns_push(func_name);
*** a/src/pl/plpgsql/src/pl_exec.c
--- b/src/pl/plpgsql/src/pl_exec.c
***************
*** 3308,3316 **** exec_stmt_execsql(PLpgSQL_execstate *estate,
/*
* If we have INTO, then we only need one row back ... but if we have
INTO
! * STRICT, ask for two rows, so that we can verify the statement returns
! * only one. INSERT/UPDATE/DELETE are always treated strictly. Without
! * INTO, just run the statement to completion (tcount = 0).
*
* We could just ask for two rows always when using INTO, but there are
* some cases where demanding the extra row costs significant time, eg
by
--- 3308,3317 ----
/*
* If we have INTO, then we only need one row back ... but if we have
INTO
! * STRICT or consistent_into is enabled, ask for two rows, so that we
can
! * verify the statement returns only one. INSERT/UPDATE/DELETE are
always
! * treated strictly. Without INTO, just run the statement to completion
! * (tcount = 0).
*
* We could just ask for two rows always when using INTO, but there are
* some cases where demanding the extra row costs significant time, eg
by
***************
*** 3319,3325 **** exec_stmt_execsql(PLpgSQL_execstate *estate,
*/
if (stmt->into)
{
! if (stmt->strict || stmt->mod_stmt)
tcount = 2;
else
tcount = 1;
--- 3320,3326 ----
*/
if (stmt->into)
{
! if (stmt->strict || stmt->mod_stmt ||
estate->func->consistent_into)
tcount = 2;
else
tcount = 1;
***************
*** 3439,3445 **** exec_stmt_execsql(PLpgSQL_execstate *estate,
}
else
{
! if (n > 1 && (stmt->strict || stmt->mod_stmt))
{
char *errdetail;
--- 3440,3446 ----
}
else
{
! if (n > 1 && (stmt->strict || stmt->mod_stmt ||
estate->func->consistent_into))
{
char *errdetail;
***************
*** 3606,3614 **** exec_stmt_dynexecute(PLpgSQL_execstate *estate,
elog(ERROR, "unsupported target");
/*
! * If SELECT ... INTO specified STRICT, and the query didn't
find
! * exactly one row, throw an error. If STRICT was not
specified, then
! * allow the query to find any number of rows.
*/
if (n == 0)
{
--- 3607,3615 ----
elog(ERROR, "unsupported target");
/*
! * If SELECT ... INTO specified STRICT or consistent_info is
enabled,
! * and the query didn't find exactly one row, throw an error.
! * Otherwise we allow the query to find any number of rows.
*/
if (n == 0)
{
***************
*** 3631,3637 **** exec_stmt_dynexecute(PLpgSQL_execstate *estate,
}
else
{
! if (n > 1 && stmt->strict)
{
char *errdetail;
--- 3632,3638 ----
}
else
{
! if (n > 1 && (stmt->strict ||
estate->func->consistent_into))
{
char *errdetail;
*** a/src/pl/plpgsql/src/pl_gram.y
--- b/src/pl/plpgsql/src/pl_gram.y
***************
*** 185,191 **** static List *read_raise_options(void);
%type <forvariable> for_variable
%type <stmt> for_control
! %type <str> any_identifier opt_block_label opt_label option_value
%type <list> proc_sect proc_stmts stmt_elsifs stmt_else
%type <loop_body> loop_body
--- 185,192 ----
%type <forvariable> for_variable
%type <stmt> for_control
! %type <str> any_identifier opt_block_label opt_label
! %type <boolean> option_value
%type <list> proc_sect proc_stmts stmt_elsifs stmt_else
%type <loop_body> loop_body
***************
*** 253,258 **** static List *read_raise_options(void);
--- 254,260 ----
%token <keyword> K_COLLATE
%token <keyword> K_COLUMN
%token <keyword> K_COLUMN_NAME
+ %token <keyword> K_CONSISTENT_INTO
%token <keyword> K_CONSTANT
%token <keyword> K_CONSTRAINT
%token <keyword> K_CONSTRAINT_NAME
***************
*** 355,368 **** comp_option : '#' K_OPTION K_DUMP
{
plpgsql_DumpExecTree = true;
}
| '#' K_PRINT_STRICT_PARAMS option_value
{
! if (strcmp($3, "on") == 0)
!
plpgsql_curr_compile->print_strict_params = true;
! else if (strcmp($3, "off") == 0)
!
plpgsql_curr_compile->print_strict_params = false;
! else
! elog(ERROR,
"unrecognized print_strict_params option %s", $3);
}
| '#' K_VARIABLE_CONFLICT K_ERROR
{
--- 357,369 ----
{
plpgsql_DumpExecTree = true;
}
+ | '#' K_CONSISTENT_INTO option_value
+ {
+
plpgsql_curr_compile->consistent_into = $3;
+ }
| '#' K_PRINT_STRICT_PARAMS option_value
{
!
plpgsql_curr_compile->print_strict_params = $3;
}
| '#' K_VARIABLE_CONFLICT K_ERROR
{
***************
*** 380,390 **** comp_option : '#' K_OPTION K_DUMP
option_value : T_WORD
{
! $$ = $1.ident;
}
| unreserved_keyword
{
! $$ = pstrdup($1);
}
opt_semi :
--- 381,401 ----
option_value : T_WORD
{
! if (strcmp($1.ident, "on") == 0)
! $$ = true;
! else if (strcmp($1.ident, "off") == 0)
! $$ = false;
! else
! elog(ERROR, "unrecognized
option_value %s", $1.ident);
}
| unreserved_keyword
{
! if (strcmp($1, "on") == 0)
! $$ = true;
! else if (strcmp($1, "off") == 0)
! $$ = false;
! else
! elog(ERROR, "unrecognized
option_value %s", $1);
}
opt_semi :
*** a/src/pl/plpgsql/src/pl_handler.c
--- b/src/pl/plpgsql/src/pl_handler.c
***************
*** 38,43 **** static const struct config_enum_entry
variable_conflict_options[] = {
--- 38,44 ----
int plpgsql_variable_conflict = PLPGSQL_RESOLVE_ERROR;
bool plpgsql_print_strict_params = false;
+ bool plpgsql_consistent_into = false;
/* Hook for plugins */
PLpgSQL_plugin **plugin_ptr = NULL;
***************
*** 75,80 **** _PG_init(void)
--- 76,89 ----
false,
PGC_USERSET, 0,
NULL, NULL, NULL);
+ DefineCustomBoolVariable("plpgsql.consistent_into",
+ gettext_noop("Always
raise an exception if a statement with INTO returns more than one row."),
+ NULL,
+
&plpgsql_consistent_into,
+ false,
+ PGC_USERSET, 0,
+ NULL, NULL, NULL);
+
EmitWarningsOnPlaceholders("plpgsql");
*** a/src/pl/plpgsql/src/pl_scanner.c
--- b/src/pl/plpgsql/src/pl_scanner.c
***************
*** 111,116 **** static const ScanKeyword unreserved_keywords[] = {
--- 111,117 ----
PG_KEYWORD("backward", K_BACKWARD, UNRESERVED_KEYWORD)
PG_KEYWORD("column", K_COLUMN, UNRESERVED_KEYWORD)
PG_KEYWORD("column_name", K_COLUMN_NAME, UNRESERVED_KEYWORD)
+ PG_KEYWORD("consistent_into", K_CONSISTENT_INTO, UNRESERVED_KEYWORD)
PG_KEYWORD("constant", K_CONSTANT, UNRESERVED_KEYWORD)
PG_KEYWORD("constraint", K_CONSTRAINT, UNRESERVED_KEYWORD)
PG_KEYWORD("constraint_name", K_CONSTRAINT_NAME, UNRESERVED_KEYWORD)
*** a/src/pl/plpgsql/src/plpgsql.h
--- b/src/pl/plpgsql/src/plpgsql.h
***************
*** 738,743 **** typedef struct PLpgSQL_function
--- 738,744 ----
PLpgSQL_resolve_option resolve_option;
bool print_strict_params;
+ bool consistent_into;
int ndatums;
PLpgSQL_datum **datums;
***************
*** 880,885 **** extern IdentifierLookup plpgsql_IdentifierLookup;
--- 881,887 ----
extern int plpgsql_variable_conflict;
extern bool plpgsql_print_strict_params;
+ extern bool plpgsql_consistent_into;
extern bool plpgsql_check_syntax;
extern bool plpgsql_DumpExecTree;
*** a/src/test/regress/expected/plpgsql.out
--- b/src/test/regress/expected/plpgsql.out
***************
*** 3208,3213 **** select footest();
--- 3208,3236 ----
ERROR: query returned more than one row
DETAIL: parameters: p1 = '2', p3 = 'foo'
CONTEXT: PL/pgSQL function footest() line 10 at SQL statement
+ -- test consistent INTO
+ set plpgsql.consistent_into to true;
+ create or replace function footest() returns void as $$
+ declare
+ x int;
+ begin
+ -- too many rows
+ select 1 from foo into x;
+ end$$ language plpgsql;
+ select footest();
+ ERROR: query returned more than one row
+ CONTEXT: PL/pgSQL function footest() line 6 at SQL statement
+ create or replace function footest() returns void as $$
+ declare
+ x int;
+ begin
+ -- too many rows
+ execute $q$select 1 from foo$q$ into x;
+ end$$ language plpgsql;
+ select footest();
+ ERROR: query returned more than one row
+ CONTEXT: PL/pgSQL function footest() line 6 at EXECUTE statement
+ reset plpgsql.consistent_into;
-- test scrollable cursor support
create function sc_test() returns setof integer as $$
declare
*** a/src/test/regress/sql/plpgsql.sql
--- b/src/test/regress/sql/plpgsql.sql
***************
*** 2689,2694 **** end$$ language plpgsql;
--- 2689,2720 ----
select footest();
+ -- test consistent INTO
+
+ set plpgsql.consistent_into to true;
+
+ create or replace function footest() returns void as $$
+ declare
+ x int;
+ begin
+ -- too many rows
+ select 1 from foo into x;
+ end$$ language plpgsql;
+
+ select footest();
+
+ create or replace function footest() returns void as $$
+ declare
+ x int;
+ begin
+ -- too many rows
+ execute $q$select 1 from foo$q$ into x;
+ end$$ language plpgsql;
+
+ select footest();
+
+ reset plpgsql.consistent_into;
+
-- test scrollable cursor support
create function sc_test() returns setof integer as $$
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
