2017-09-13 1:42 GMT+02:00 Daniel Gustafsson <dan...@yesql.se>:
> > On 08 Apr 2017, at 15:46, David Steele <da...@pgmasters.net> wrote:
> >
> >> On 1/13/17 6:55 AM, Marko Tiikkaja wrote:
> >>> On Fri, Jan 13, 2017 at 2:46 AM, Jim Nasby <jim.na...@bluetreble.com
> >>> <mailto:jim.na...@bluetreble.com>> wrote:
> >>>
> >>> On 1/11/17 5:54 AM, Pavel Stehule wrote:
> >>>
> >>> + <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.
> >>>
> >>>
> >>> Shouldn't this also apply to
> >>>
> >>> var := blah FROM some_table WHERE ...;
> >>>
> >>> ?
> >>>
> >>> AIUI that's one of the beefs the plpgsql2 project has.
> >>>
> >>>
> >>> No, not at all. That syntax is undocumented and only works because
> >>> PL/PgSQL is a hack internally. We don't use it, and frankly I don't
> >>> think anyone should.
> >
> > This submission has been moved to CF 2017-07.
>
> This patch was automatically marked as “Waiting for author” since it needs
> to
> be updated with the macro changes in 2cd70845240087da205695baedab64
> 12342d1dbe
> to compile. Changing to using TupleDescAttr(); makes it compile again.
> Can
> you submit an updated version with that fix Pavel?
>
I am sending fixed patch
Regards
Pavel
>
> Stephen, you signed up to review this patch in the previous Commitfest, do
> you
> still intend to work on this?
>
> cheers ./daniel
diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml
index 6dc438a152..7de0b8005a 100644
--- a/doc/src/sgml/plpgsql.sgml
+++ b/doc/src/sgml/plpgsql.sgml
@@ -4862,7 +4862,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
@@ -4874,6 +4874,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
@@ -4890,6 +4895,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</>
@@ -4909,6 +4938,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 9716697259..c14fdc0233 100644
--- a/src/pl/plpgsql/src/pl_exec.c
+++ b/src/pl/plpgsql/src/pl_exec.c
@@ -3623,6 +3623,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
@@ -3672,7 +3690,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;
@@ -3792,7 +3810,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;
@@ -3801,7 +3819,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));
@@ -6027,12 +6045,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 (!TupleDescAttr(tupdesc, 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 1ebb7a7b5e..dceb710703 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 2b19948562..18bff2a27e 100644
--- a/src/pl/plpgsql/src/plpgsql.h
+++ b/src/pl/plpgsql/src/plpgsql.h
@@ -1024,7 +1024,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 7d3e9225bb..17770c1cdc 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 6c9399696b..8d0b24ea93 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
