Hi
2017-11-30 3:44 GMT+01:00 Michael Paquier <michael.paqu...@gmail.com>:
> On Wed, Sep 13, 2017 at 12:51 PM, Pavel Stehule <pavel.steh...@gmail.com>
> wrote:
> >
> >
> > 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
> >> 2cd70845240087da205695baedab6412342d1dbe
> >> 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
>
> + <para>
> + The setting <varname>plpgsql.extra_warnings</> to <literal>all</> is
> a
> + good idea in developer or test environments.
> + </para>
> At least documentation needs patching, or this is going to generate
> warnings on HEAD at compilation. I am moving this to next CF for lack
> of reviews, and the status is waiting on author as this needs at least
> a couple of doc fixes.
>
fixed doc attached
Regards
Pavel
> --
> Michael
>
diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml
index 7d23ed437e..efa48bc13c 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 '$$
</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
@@ -4963,6 +4963,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</varname> to <literal>all</literal> is a
+ good idea in developer or test environments.
+ </para>
+
<para>
These additional checks are enabled through the configuration variables
<varname>plpgsql.extra_warnings</varname> for warnings and
@@ -4979,6 +4984,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</application> 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</varname>
@@ -4997,6 +5026,34 @@ WARNING: variable "f1" shadows a previously defined variable
LINE 3: f1 int;
^
CREATE FUNCTION
+</programlisting>
+
+ The another example shows the effect of <varname>plpgsql.extra_warnings</varname>
+ set to <varname>strict_multi_assignment</varname>:
+<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>
diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index ec480cb0ba..0879e84cd2 100644
--- a/src/pl/plpgsql/src/pl_exec.c
+++ b/src/pl/plpgsql/src/pl_exec.c
@@ -3629,6 +3629,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
@@ -3678,7 +3696,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;
@@ -3798,7 +3816,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;
@@ -3807,7 +3825,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));
@@ -6033,12 +6051,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 d6e5bc3353..72776c2e2f 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 1c355132b7..189750316c 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 $$
