diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml index 7d23ed437e..af1cf4ab95 100644 --- a/doc/src/sgml/plpgsql.sgml +++ b/doc/src/sgml/plpgsql.sgml @@ -4951,7 +4951,7 @@ a_output := a_output || $$ if v_$$ || referrer_keys.kind || $$ like '$$ - Additional Compile-time Checks + Additional Compile-time and Run-time Checks To aid the user in finding instances of simple but common problems before @@ -4963,26 +4963,62 @@ a_output := a_output || $$ if v_$$ || referrer_keys.kind || $$ like '$$ so you are advised to test in a separate development environment. - - These additional checks are enabled through the configuration variables - plpgsql.extra_warnings for warnings and - plpgsql.extra_errors for errors. Both can be set either to - a comma-separated list of checks, "none" or "all". - The default is "none". Currently the list of available checks - includes only one: - - - shadowed_variables - - - Checks if a declaration shadows a previously defined variable. - - - - + + Setting plpgsql.extra_warnings, or + plpgsql.extra_errors, as appropriate, to all + is encouraged in development and/or testing environments. + + + + These additional checks are enabled through the configuration variables + plpgsql.extra_warnings for warnings and + plpgsql.extra_errors for errors. Both can be set either to + a comma-separated list of checks, "none" or + "all". The default is "none". Currently + the list of available checks includes only one: + + + shadowed_variables + + + Checks if a declaration shadows a previously defined variable. + + + - The following example shows the effect of plpgsql.extra_warnings - set to shadowed_variables: + + strict_multi_assignment + + + Some PL/PgSQL commands allow assigning values + to more than one variable at a time, such as SELECT INTO. Typically, + the number of target variables and the number of source variables should + match, though PL/PgSQL will use NULL for + missing values and extra variables are ignored. Enabling this check + will cause PL/PgSQL to throw a WARNING or + ERROR whenever the number of target variables and the number of source + variables are different. + + + + + + too_many_rows + + + Enabling this check will cause PL/PgSQL to + check if a given query returns more than one row when an + INTO clause is used. As an INTO statement will only + ever use one row, having a query return multiple rows is generally + either inefficient and/or nondeterministic and therefore is likely an + error. + + + + + + The following example shows the effect of plpgsql.extra_warnings + set to shadowed_variables: SET plpgsql.extra_warnings TO 'shadowed_variables'; @@ -4998,8 +5034,38 @@ LINE 3: f1 int; ^ CREATE FUNCTION - - + The below example shows the effects of setting + plpgsql.extra_warnings to + strict_multi_assignment: + +SET plpgsql.extra_warnings TO 'strict_multi_assignment'; + +CREATE OR REPLACE FUNCTION public.foo() + RETURNS void + LANGUAGE plpgsql +AS $$ +DECLARE + x int; + y int; +BEGIN + SELECT 1 INTO x, y; + SELECT 1, 2 INTO x, y; + SELECT 1, 2, 3 INTO x, y; +END; +$$ + +SELECT foo(); +WARNING: Number of evaluated fields does not match expected. +HINT: strict_multi_assignement check of extra_warnings is active. +WARNING: Number of evaluated fields does not match expected. +HINT: strict_multi_assignement check of extra_warnings is active. + foo +----- + +(1 row) + + + diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c index d096f242cd..2ce51213f1 100644 --- a/src/pl/plpgsql/src/pl_exec.c +++ b/src/pl/plpgsql/src/pl_exec.c @@ -3559,6 +3559,12 @@ exec_stmt_execsql(PLpgSQL_execstate *estate, long tcount; int rc; PLpgSQL_expr *expr = stmt->sqlstmt; + int too_many_rows_level = 0; + + if (plpgsql_extra_errors & PLPGSQL_XCHECK_TOOMANYROWS) + too_many_rows_level = ERROR; + else if (plpgsql_extra_warnings & PLPGSQL_XCHECK_TOOMANYROWS) + too_many_rows_level = WARNING; /* * On the first call for this statement generate the plan, and detect @@ -3597,9 +3603,10 @@ 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). + * STRICT or extra check too_many_rows, 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 @@ -3608,7 +3615,7 @@ exec_stmt_execsql(PLpgSQL_execstate *estate, */ if (stmt->into) { - if (stmt->strict || stmt->mod_stmt) + if (stmt->strict || stmt->mod_stmt || too_many_rows_level >= WARNING) tcount = 2; else tcount = 1; @@ -3722,19 +3729,26 @@ exec_stmt_execsql(PLpgSQL_execstate *estate, } else { - if (n > 1 && (stmt->strict || stmt->mod_stmt)) + if (n > 1 && (stmt->strict || stmt->mod_stmt || too_many_rows_level >= WARNING)) { char *errdetail; + int errlevel; + bool use_errhint; if (estate->func->print_strict_params) errdetail = format_expr_params(estate, expr); else errdetail = NULL; - ereport(ERROR, + errlevel = stmt->strict || stmt->mod_stmt ? ERROR : too_many_rows_level; + use_errhint = !(stmt->strict || stmt->mod_stmt); + + ereport(errlevel, (errcode(ERRCODE_TOO_MANY_ROWS), errmsg("query returned more than one row"), - errdetail ? errdetail_internal("parameters: %s", errdetail) : 0)); + errdetail ? errdetail_internal("parameters: %s", errdetail) : 0, + use_errhint ? errhint("too_many_rows check of extra_%s is active.", + too_many_rows_level == ERROR ? "errors" : "warnings") : 0)); } /* Put the first result row into the target */ exec_move_row(estate, target, tuptab->vals[0], tuptab->tupdesc); @@ -5944,6 +5958,12 @@ exec_move_row(PLpgSQL_execstate *estate, int t_natts; int fnum; int anum; + int strict_multiassignment_level = 0; + + if (plpgsql_extra_errors & PLPGSQL_XCHECK_STRICTMULTIASSIGNMENT) + strict_multiassignment_level = ERROR; + else if (plpgsql_extra_warnings & PLPGSQL_XCHECK_STRICTMULTIASSIGNMENT) + strict_multiassignment_level = WARNING; if (HeapTupleIsValid(tup)) t_natts = HeapTupleHeaderGetNatts(tup->t_data); @@ -5974,6 +5994,7 @@ exec_move_row(PLpgSQL_execstate *estate, value = SPI_getbinval(tup, tupdesc, anum + 1, &isnull); else { + /* there are no data */ value = (Datum) 0; isnull = true; } @@ -5987,12 +6008,37 @@ exec_move_row(PLpgSQL_execstate *estate, isnull = true; valtype = UNKNOWNOID; valtypmod = -1; + + if (strict_multiassignment_level >= WARNING) + ereport(strict_multiassignment_level, + (errcode(ERRCODE_DATATYPE_MISMATCH), + errmsg("Number of evaluated fields does not match expected."), + errhint("strict_multi_assignement check of extra_%s is active.", + strict_multiassignment_level == ERROR ? "errors" : "warnings"))); } exec_assign_value(estate, (PLpgSQL_datum *) var, value, isnull, valtype, valtypmod); } + /* + * When stric_multiassignment extra check is active, then ensure so there + * are not more unassigned columns. + */ + if (strict_multiassignment_level >= WARNING && anum < td_natts) + { + while (anum < td_natts && + TupleDescAttr(tupdesc, anum)->attisdropped) + anum++; /* skip dropped column in tuple */ + + if (anum < td_natts) + ereport(strict_multiassignment_level, + (errcode(ERRCODE_DATATYPE_MISMATCH), + errmsg("Number of evaluated fields does not match expected."), + errhint("strict_multi_assignement check of extra_%s is active.", + strict_multiassignment_level == ERROR ? "errors" : "warnings"))); + } + return; } diff --git a/src/pl/plpgsql/src/pl_handler.c b/src/pl/plpgsql/src/pl_handler.c index 4c2ba2f734..c6064299ab 100644 --- a/src/pl/plpgsql/src/pl_handler.c +++ b/src/pl/plpgsql/src/pl_handler.c @@ -92,6 +92,10 @@ plpgsql_extra_checks_check_hook(char **newvalue, void **extra, GucSource source) if (pg_strcasecmp(tok, "shadowed_variables") == 0) extrachecks |= PLPGSQL_XCHECK_SHADOWVAR; + else if (pg_strcasecmp(tok, "too_many_rows") == 0) + extrachecks |= PLPGSQL_XCHECK_TOOMANYROWS; + else if (pg_strcasecmp(tok, "strict_multi_assignment") == 0) + extrachecks |= PLPGSQL_XCHECK_STRICTMULTIASSIGNMENT; else if (pg_strcasecmp(tok, "all") == 0 || pg_strcasecmp(tok, "none") == 0) { GUC_check_errdetail("Key word \"%s\" cannot be combined with other key words.", tok); diff --git a/src/pl/plpgsql/src/plpgsql.h b/src/pl/plpgsql/src/plpgsql.h index c571afa34b..e0ee6172db 100644 --- a/src/pl/plpgsql/src/plpgsql.h +++ b/src/pl/plpgsql/src/plpgsql.h @@ -1019,7 +1019,9 @@ extern bool plpgsql_check_asserts; /* extra compile-time checks */ #define PLPGSQL_XCHECK_NONE 0 -#define PLPGSQL_XCHECK_SHADOWVAR 1 +#define PLPGSQL_XCHECK_SHADOWVAR (1 << 1) +#define PLPGSQL_XCHECK_TOOMANYROWS (1 << 2) +#define PLPGSQL_XCHECK_STRICTMULTIASSIGNMENT (1 << 3) #define PLPGSQL_XCHECK_ALL ((int) ~0) extern int plpgsql_extra_warnings; diff --git a/src/test/regress/expected/plpgsql.out b/src/test/regress/expected/plpgsql.out index 4783807ae0..5946903974 100644 --- a/src/test/regress/expected/plpgsql.out +++ b/src/test/regress/expected/plpgsql.out @@ -3089,6 +3089,99 @@ select shadowtest(1); t (1 row) +-- runtime extra checks +set plpgsql.extra_warnings to 'too_many_rows'; +do $$ +declare x int; +begin + select v from generate_series(1,2) g(v) into x; +end; +$$; +WARNING: query returned more than one row +HINT: too_many_rows check of extra_warnings is active. +set plpgsql.extra_errors to 'too_many_rows'; +do $$ +declare x int; +begin + select v from generate_series(1,2) g(v) into x; +end; +$$; +ERROR: query returned more than one row +HINT: too_many_rows check of extra_errors is active. +CONTEXT: PL/pgSQL function inline_code_block line 4 at SQL statement +reset plpgsql.extra_errors; +reset plpgsql.extra_warnings; +set plpgsql.extra_warnings to 'strict_multi_assignment'; +do $$ +declare + x int; + y int; +begin + select 1 into x, y; + select 1,2 into x, y; + select 1,2,3 into x, y; +end +$$; +WARNING: Number of evaluated fields does not match expected. +HINT: strict_multi_assignement check of extra_warnings is active. +WARNING: Number of evaluated fields does not match expected. +HINT: strict_multi_assignement check of extra_warnings is active. +set plpgsql.extra_errors to 'strict_multi_assignment'; +do $$ +declare + x int; + y int; +begin + select 1 into x, y; + select 1,2 into x, y; + select 1,2,3 into x, y; +end +$$; +ERROR: Number of evaluated fields does not match expected. +HINT: strict_multi_assignement check of extra_errors is active. +CONTEXT: PL/pgSQL function inline_code_block line 6 at SQL statement +create table test_01(a int, b int, c int); +alter table test_01 drop column a; +do $$ +declare + x int; + y int; +begin + select * from test_01 into x, y; -- should be ok + raise notice 'ok'; + select * from test_01 into x; -- should to fail +end; +$$; +NOTICE: ok +ERROR: Number of evaluated fields does not match expected. +HINT: strict_multi_assignement check of extra_errors is active. +CONTEXT: PL/pgSQL function inline_code_block line 8 at SQL statement +do $$ +declare + t test_01; +begin + select 1, 2 into t; -- should be ok + raise notice 'ok'; + select 1, 2, 3 into t; -- should fail; +end; +$$; +NOTICE: ok +ERROR: Number of evaluated fields does not match expected. +HINT: strict_multi_assignement check of extra_errors is active. +CONTEXT: PL/pgSQL function inline_code_block line 7 at SQL statement +do $$ +declare + t test_01; +begin + select 1 into t; -- should fail; +end; +$$; +ERROR: Number of evaluated fields does not match expected. +HINT: strict_multi_assignement check of extra_errors is active. +CONTEXT: PL/pgSQL function inline_code_block line 5 at SQL statement +drop table test_01; +reset plpgsql.extra_errors; +reset plpgsql.extra_warnings; -- test scrollable cursor support create function sc_test() returns setof integer as $$ declare diff --git a/src/test/regress/sql/plpgsql.sql b/src/test/regress/sql/plpgsql.sql index 768270d467..0e255b88f1 100644 --- a/src/test/regress/sql/plpgsql.sql +++ b/src/test/regress/sql/plpgsql.sql @@ -2605,6 +2605,92 @@ declare f1 int; begin return 1; end $$ language plpgsql; select shadowtest(1); +-- runtime extra checks +set plpgsql.extra_warnings to 'too_many_rows'; + +do $$ +declare x int; +begin + select v from generate_series(1,2) g(v) into x; +end; +$$; + +set plpgsql.extra_errors to 'too_many_rows'; + +do $$ +declare x int; +begin + select v from generate_series(1,2) g(v) into x; +end; +$$; + +reset plpgsql.extra_errors; +reset plpgsql.extra_warnings; + +set plpgsql.extra_warnings to 'strict_multi_assignment'; + +do $$ +declare + x int; + y int; +begin + select 1 into x, y; + select 1,2 into x, y; + select 1,2,3 into x, y; +end +$$; + +set plpgsql.extra_errors to 'strict_multi_assignment'; + +do $$ +declare + x int; + y int; +begin + select 1 into x, y; + select 1,2 into x, y; + select 1,2,3 into x, y; +end +$$; + +create table test_01(a int, b int, c int); + +alter table test_01 drop column a; + +do $$ +declare + x int; + y int; +begin + select * from test_01 into x, y; -- should be ok + raise notice 'ok'; + select * from test_01 into x; -- should to fail +end; +$$; + +do $$ +declare + t test_01; +begin + select 1, 2 into t; -- should be ok + raise notice 'ok'; + select 1, 2, 3 into t; -- should fail; +end; +$$; + +do $$ +declare + t test_01; +begin + select 1 into t; -- should fail; +end; +$$; + +drop table test_01; + +reset plpgsql.extra_errors; +reset plpgsql.extra_warnings; + -- test scrollable cursor support create function sc_test() returns setof integer as $$