Hi
I am starting new thread for this patch (related to merging some ideas from
plpgsql2 project thread).
Here is simple patch with two new extra_warnings, extra_errors checks
1. strict_multi_assignment - checks the number of target variables and
source values.
2. too_many_rows - checks if returned rows is more than one
The extra checks are designed to help identify some possible errors or
issues. It is not way how to change a design, behave and features of
plpgsql language.
Now, the extra checks are three fields only - it is easy maintainable now,
and I don't see a necessity to implement some another management for extra
checks.
Regards
Pavel
diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml
index d3272e1..f81dea0 100644
--- a/doc/src/sgml/plpgsql.sgml
+++ b/doc/src/sgml/plpgsql.sgml
@@ -4847,7 +4847,7 @@ a_output := a_output || $$ if v_$$ || referrer_keys.kind || $$ like '$$
</sect2>
<sect2 id="plpgsql-extra-checks">
- <title>Additional Compile-time Checks</title>
+ <title>Additional Compile-time and Run-time Checks</title>
<para>
To aid the user in finding instances of simple but common problems before
@@ -4859,6 +4859,11 @@ a_output := a_output || $$ if v_$$ || referrer_keys.kind || $$ like '$$
so you are advised to test in a separate development environment.
</para>
+ <para>
+ The setting <varname>plpgsql.extra_warnings</> to <literal>all</> is a
+ good idea in developer or test environments.
+ </para>
+
<para>
These additional checks are enabled through the configuration variables
<varname>plpgsql.extra_warnings</> for warnings and
@@ -4875,6 +4880,30 @@ a_output := a_output || $$ if v_$$ || referrer_keys.kind || $$ like '$$
</para>
</listitem>
</varlistentry>
+
+ <varlistentry>
+ <term><varname>strict_multi_assignment</varname></term>
+ <listitem>
+ <para>
+ Some <application>PL/PgSQL</> commands allows to assign a values to
+ more than one variable. The number of target variables should not be
+ equal to number of source values. Missing values are replaced by NULL
+ value, spare values are ignored. More times this situation signalize
+ some error.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
+ <term><varname>too_many_rows</varname></term>
+ <listitem>
+ <para>
+ When result is assigned to a variable by <literal>INTO</literal> clause,
+ checks if query returns more than one row. In this case the assignment
+ is not deterministic usually - and it can be signal some issues in design.
+ </para>
+ </listitem>
+ </varlistentry>
</variablelist>
The following example shows the effect of <varname>plpgsql.extra_warnings</>
@@ -4894,6 +4923,34 @@ LINE 3: f1 int;
^
CREATE FUNCTION
</programlisting>
+
+ The another example shows the effect of <varname>plpgsql.extra_warnings</>
+ set to <varname>strict_multi_assignment</>:
+<programlisting>
+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 attributies (1) does not match expected attributies (2)
+WARNING: Number of evaluated attributies (3) does not match expected attributies (2)
+ foo
+-----
+
+(1 row)
+</programlisting>
</para>
</sect2>
</sect1>
diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index 192cbcf..880c1b8 100644
--- a/src/pl/plpgsql/src/pl_exec.c
+++ b/src/pl/plpgsql/src/pl_exec.c
@@ -3616,6 +3616,24 @@ exec_stmt_execsql(PLpgSQL_execstate *estate,
long tcount;
int rc;
PLpgSQL_expr *expr = stmt->sqlstmt;
+ bool too_many_rows_check;
+ int too_many_rows_level;
+
+ if (plpgsql_extra_errors & PLPGSQL_XCHECK_TOOMANYROWS)
+ {
+ too_many_rows_check = true;
+ too_many_rows_level = ERROR;
+ }
+ else if (plpgsql_extra_warnings & PLPGSQL_XCHECK_TOOMANYROWS)
+ {
+ too_many_rows_check = true;
+ too_many_rows_level = WARNING;
+ }
+ else
+ {
+ too_many_rows_check = false;
+ too_many_rows_level = NOTICE;
+ }
/*
* On the first call for this statement generate the plan, and detect
@@ -3666,7 +3684,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_check)
tcount = 2;
else
tcount = 1;
@@ -3786,7 +3804,7 @@ 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_check))
{
char *errdetail;
@@ -3795,7 +3813,7 @@ exec_stmt_execsql(PLpgSQL_execstate *estate,
else
errdetail = NULL;
- ereport(ERROR,
+ ereport(too_many_rows_level == WARNING && !stmt->strict ? WARNING : ERROR,
(errcode(ERRCODE_TOO_MANY_ROWS),
errmsg("query returned more than one row"),
errdetail ? errdetail_internal("parameters: %s", errdetail) : 0));
@@ -6009,12 +6027,48 @@ exec_move_row(PLpgSQL_execstate *estate,
int t_natts;
int fnum;
int anum;
+ bool strict_multiassignment_check;
+ int strict_multiassignment_level;
+
+ if (plpgsql_extra_errors & PLPGSQL_XCHECK_STRICTMULTIASSIGNMENT)
+ {
+ strict_multiassignment_check = true;
+ strict_multiassignment_level = ERROR;
+ }
+ else if (plpgsql_extra_warnings & PLPGSQL_XCHECK_STRICTMULTIASSIGNMENT)
+ {
+ strict_multiassignment_check = true;
+ strict_multiassignment_level = WARNING;
+ }
+ else
+ {
+ strict_multiassignment_check = false;
+ strict_multiassignment_level = NOTICE;
+ }
if (HeapTupleIsValid(tup))
t_natts = HeapTupleHeaderGetNatts(tup->t_data);
else
t_natts = 0;
+ if (strict_multiassignment_check)
+ {
+ int i;
+
+ anum = 0;
+ for (i = 0; i < td_natts; i++)
+ if (!tupdesc->attrs[i]->attisdropped)
+ anum++;
+
+ if (anum != row->nfields)
+ {
+ ereport(strict_multiassignment_level,
+ (errcode(ERRCODE_DATATYPE_MISMATCH),
+ errmsg("Number of evaluated attributies (%d) does not match expected attributies (%d)",
+ anum, row->nfields)));
+ }
+ }
+
anum = 0;
for (fnum = 0; fnum < row->nfields; fnum++)
{
diff --git a/src/pl/plpgsql/src/pl_handler.c b/src/pl/plpgsql/src/pl_handler.c
index ca8c9cb..85acf96 100644
--- a/src/pl/plpgsql/src/pl_handler.c
+++ b/src/pl/plpgsql/src/pl_handler.c
@@ -89,6 +89,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 3421eed..623af81 100644
--- a/src/pl/plpgsql/src/plpgsql.h
+++ b/src/pl/plpgsql/src/plpgsql.h
@@ -1025,7 +1025,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 79513e4..b09e83a 100644
--- a/src/test/regress/expected/plpgsql.out
+++ b/src/test/regress/expected/plpgsql.out
@@ -3422,6 +3422,54 @@ 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
+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
+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 attributies (1) does not match expected attributies (2)
+WARNING: Number of evaluated attributies (3) does not match expected attributies (2)
+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 attributies (1) does not match expected attributies (2)
+CONTEXT: PL/pgSQL function inline_code_block line 6 at SQL statement
+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 877d3ad..aa47e93 100644
--- a/src/test/regress/sql/plpgsql.sql
+++ b/src/test/regress/sql/plpgsql.sql
@@ -2840,6 +2840,57 @@ 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
+$$;
+
+reset plpgsql.extra_errors;
+reset plpgsql.extra_warnings;
+
-- 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
