Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit167075b

Browse files
committed
Add strict_multi_assignment and too_many_rows plpgsql checks
Until now shadowed_variables was the only plpgsql check supported byplpgsql.extra_warnings and plpgsql.extra_errors. This patch introducestwo new checks - strict_multi_assignment and too_many_rows. Unlikeshadowed_variables, these new checks are enforced at run-time.strict_multi_assignment checks that commands allowing multi-assignment(for example SELECT INTO) have the same number of sources and targets.too_many_rows checks that queries with an INTO clause return one rowexactly.These checks are aimed at cases that are technically valid and allowed,but are often a sign of a bug. Therefore those checks are expected tobe enabled primarily in development and testing environments.Author: Pavel StehuleReviewed-by: Stephen Frost, Tomas VondraDiscussion:https://www.postgresql.org/message-id/flat/CAFj8pRA2kKRDKpUNwLY0GeG1OqOp+tLS2yQA1V41gzuSz-hCng@mail.gmail.com
1 parent2d30675 commit167075b

File tree

6 files changed

+400
-33
lines changed

6 files changed

+400
-33
lines changed

‎doc/src/sgml/plpgsql.sgml

Lines changed: 93 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -5034,7 +5034,7 @@ a_output := a_output || $$ if v_$$ || referrer_keys.kind || $$ like '$$
50345034

50355035
</sect2>
50365036
<sect2 id="plpgsql-extra-checks">
5037-
<title>Additional Compile-time Checks</title>
5037+
<title>Additional Compile-timeand Run-timeChecks</title>
50385038

50395039
<para>
50405040
To aid the user in finding instances of simple but common problems before
@@ -5046,26 +5046,64 @@ a_output := a_output || $$ if v_$$ || referrer_keys.kind || $$ like '$$
50465046
so you are advised to test in a separate development environment.
50475047
</para>
50485048

5049-
<para>
5050-
These additional checks are enabled through the configuration variables
5051-
<varname>plpgsql.extra_warnings</varname> for warnings and
5052-
<varname>plpgsql.extra_errors</varname> for errors. Both can be set either to
5053-
a comma-separated list of checks, <literal>"none"</literal> or <literal>"all"</literal>.
5054-
The default is <literal>"none"</literal>. Currently the list of available checks
5055-
includes only one:
5056-
<variablelist>
5057-
<varlistentry>
5058-
<term><varname>shadowed_variables</varname></term>
5059-
<listitem>
5060-
<para>
5061-
Checks if a declaration shadows a previously defined variable.
5062-
</para>
5063-
</listitem>
5064-
</varlistentry>
5065-
</variablelist>
5049+
<para>
5050+
Setting <varname>plpgsql.extra_warnings</varname>, or
5051+
<varname>plpgsql.extra_errors</varname>, as appropriate, to <literal>"all"</literal>
5052+
is encouraged in development and/or testing environments.
5053+
</para>
5054+
5055+
<para>
5056+
These additional checks are enabled through the configuration variables
5057+
<varname>plpgsql.extra_warnings</varname> for warnings and
5058+
<varname>plpgsql.extra_errors</varname> for errors. Both can be set either to
5059+
a comma-separated list of checks, <literal>"none"</literal> or
5060+
<literal>"all"</literal>. The default is <literal>"none"</literal>. Currently
5061+
the list of available checks includes:
5062+
<variablelist>
5063+
<varlistentry>
5064+
<term><varname>shadowed_variables</varname></term>
5065+
<listitem>
5066+
<para>
5067+
Checks if a declaration shadows a previously defined variable.
5068+
</para>
5069+
</listitem>
5070+
</varlistentry>
50665071

5067-
The following example shows the effect of <varname>plpgsql.extra_warnings</varname>
5068-
set to <varname>shadowed_variables</varname>:
5072+
<varlistentry>
5073+
<term><varname>strict_multi_assignment</varname></term>
5074+
<listitem>
5075+
<para>
5076+
Some <application>PL/PgSQL</application> commands allow assigning
5077+
values to more than one variable at a time, such as
5078+
<command>SELECT INTO</command>. Typically, the number of target
5079+
variables and the number of source variables should match, though
5080+
<application>PL/PgSQL</application> will use <literal>NULL</literal>
5081+
for missing values and extra variables are ignored. Enabling this
5082+
check will cause <application>PL/PgSQL</application> to throw a
5083+
<literal>WARNING</literal> or <literal>ERROR</literal> whenever the
5084+
number of target variables and the number of source variables are
5085+
different.
5086+
</para>
5087+
</listitem>
5088+
</varlistentry>
5089+
5090+
<varlistentry>
5091+
<term><varname>too_many_rows</varname></term>
5092+
<listitem>
5093+
<para>
5094+
Enabling this check will cause <application>PL/PgSQL</application> to
5095+
check if a given query returns more than one row when an
5096+
<literal>INTO</literal> clause is used. As an <literal>INTO</literal>
5097+
statement will only ever use one row, having a query return multiple
5098+
rows is generally either inefficient and/or nondeterministic and
5099+
therefore is likely an error.
5100+
</para>
5101+
</listitem>
5102+
</varlistentry>
5103+
</variablelist>
5104+
5105+
The following example shows the effect of <varname>plpgsql.extra_warnings</varname>
5106+
set to <varname>shadowed_variables</varname>:
50695107
<programlisting>
50705108
SET plpgsql.extra_warnings TO 'shadowed_variables';
50715109

@@ -5081,8 +5119,41 @@ LINE 3: f1 int;
50815119
^
50825120
CREATE FUNCTION
50835121
</programlisting>
5084-
</para>
5085-
</sect2>
5122+
The below example shows the effects of setting
5123+
<varname>plpgsql.extra_warnings</varname> to
5124+
<varname>strict_multi_assignment</varname>:
5125+
<programlisting>
5126+
SET plpgsql.extra_warnings TO 'strict_multi_assignment';
5127+
5128+
CREATE OR REPLACE FUNCTION public.foo()
5129+
RETURNS void
5130+
LANGUAGE plpgsql
5131+
AS $$
5132+
DECLARE
5133+
x int;
5134+
y int;
5135+
BEGIN
5136+
SELECT 1 INTO x, y;
5137+
SELECT 1, 2 INTO x, y;
5138+
SELECT 1, 2, 3 INTO x, y;
5139+
END;
5140+
$$;
5141+
5142+
SELECT foo();
5143+
WARNING: number of source and target fields in assignment do not match
5144+
DETAIL: strict_multi_assignment check of extra_warnings is active.
5145+
HINT: Make sure the query returns the exact list of columns.
5146+
WARNING: number of source and target fields in assignment do not match
5147+
DETAIL: strict_multi_assignment check of extra_warnings is active.
5148+
HINT: Make sure the query returns the exact list of columns.
5149+
5150+
foo
5151+
-----
5152+
5153+
(1 row)
5154+
</programlisting>
5155+
</para>
5156+
</sect2>
50865157
</sect1>
50875158

50885159
<!-- **** Porting from Oracle PL/SQL **** -->

‎src/pl/plpgsql/src/pl_exec.c

Lines changed: 101 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4020,6 +4020,12 @@ exec_stmt_execsql(PLpgSQL_execstate *estate,
40204020
longtcount;
40214021
intrc;
40224022
PLpgSQL_expr*expr=stmt->sqlstmt;
4023+
inttoo_many_rows_level=0;
4024+
4025+
if (plpgsql_extra_errors&PLPGSQL_XCHECK_TOOMANYROWS)
4026+
too_many_rows_level=ERROR;
4027+
elseif (plpgsql_extra_warnings&PLPGSQL_XCHECK_TOOMANYROWS)
4028+
too_many_rows_level=WARNING;
40234029

40244030
/*
40254031
* On the first call for this statement generate the plan, and detect
@@ -4059,9 +4065,10 @@ exec_stmt_execsql(PLpgSQL_execstate *estate,
40594065

40604066
/*
40614067
* If we have INTO, then we only need one row back ... but if we have INTO
4062-
* STRICT, ask for two rows, so that we can verify the statement returns
4063-
* only one. INSERT/UPDATE/DELETE are always treated strictly. Without
4064-
* INTO, just run the statement to completion (tcount = 0).
4068+
* STRICT or extra check too_many_rows, ask for two rows, so that we can
4069+
* verify the statement returns only one. INSERT/UPDATE/DELETE are always
4070+
* treated strictly. Without INTO, just run the statement to completion
4071+
* (tcount = 0).
40654072
*
40664073
* We could just ask for two rows always when using INTO, but there are
40674074
* some cases where demanding the extra row costs significant time, eg by
@@ -4070,7 +4077,7 @@ exec_stmt_execsql(PLpgSQL_execstate *estate,
40704077
*/
40714078
if (stmt->into)
40724079
{
4073-
if (stmt->strict||stmt->mod_stmt)
4080+
if (stmt->strict||stmt->mod_stmt||too_many_rows_level)
40744081
tcount=2;
40754082
else
40764083
tcount=1;
@@ -4187,19 +4194,23 @@ exec_stmt_execsql(PLpgSQL_execstate *estate,
41874194
}
41884195
else
41894196
{
4190-
if (n>1&& (stmt->strict||stmt->mod_stmt))
4197+
if (n>1&& (stmt->strict||stmt->mod_stmt||too_many_rows_level))
41914198
{
41924199
char*errdetail;
4200+
interrlevel;
41934201

41944202
if (estate->func->print_strict_params)
41954203
errdetail=format_expr_params(estate,expr);
41964204
else
41974205
errdetail=NULL;
41984206

4199-
ereport(ERROR,
4207+
errlevel= (stmt->strict||stmt->mod_stmt) ?ERROR :too_many_rows_level;
4208+
4209+
ereport(errlevel,
42004210
(errcode(ERRCODE_TOO_MANY_ROWS),
42014211
errmsg("query returned more than one row"),
4202-
errdetail ?errdetail_internal("parameters: %s",errdetail) :0));
4212+
errdetail ?errdetail_internal("parameters: %s",errdetail) :0,
4213+
errhint("Make sure the query returns a single row, or use LIMIT 1")));
42034214
}
42044215
/* Put the first result row into the target */
42054216
exec_move_row(estate,target,tuptab->vals[0],tuptab->tupdesc);
@@ -6835,6 +6846,19 @@ exec_move_row_from_fields(PLpgSQL_execstate *estate,
68356846
inttd_natts=tupdesc ?tupdesc->natts :0;
68366847
intfnum;
68376848
intanum;
6849+
intstrict_multiassignment_level=0;
6850+
6851+
/*
6852+
* The extra check strict strict_multi_assignment can be active,
6853+
* only when input tupdesc is specified.
6854+
*/
6855+
if (tupdesc!=NULL)
6856+
{
6857+
if (plpgsql_extra_errors&PLPGSQL_XCHECK_STRICTMULTIASSIGNMENT)
6858+
strict_multiassignment_level=ERROR;
6859+
elseif (plpgsql_extra_warnings&PLPGSQL_XCHECK_STRICTMULTIASSIGNMENT)
6860+
strict_multiassignment_level=WARNING;
6861+
}
68386862

68396863
/* Handle RECORD-target case */
68406864
if (target->dtype==PLPGSQL_DTYPE_REC)
@@ -6913,10 +6937,23 @@ exec_move_row_from_fields(PLpgSQL_execstate *estate,
69136937
}
69146938
else
69156939
{
6940+
/* no source for destination column */
69166941
value= (Datum)0;
69176942
isnull= true;
69186943
valtype=UNKNOWNOID;
69196944
valtypmod=-1;
6945+
6946+
/* When source value is missing */
6947+
if (strict_multiassignment_level)
6948+
ereport(strict_multiassignment_level,
6949+
(errcode(ERRCODE_DATATYPE_MISMATCH),
6950+
errmsg("number of source and target fields in assignment do not match"),
6951+
/* translator: %s represents a name of an extra check */
6952+
errdetail("%s check of %s is active.",
6953+
"strict_multi_assignment",
6954+
strict_multiassignment_level==ERROR ?"extra_errors" :
6955+
"extra_warnings"),
6956+
errhint("Make sure the query returns the exact list of columns.")));
69206957
}
69216958

69226959
/* Cast the new value to the right type, if needed. */
@@ -6930,6 +6967,29 @@ exec_move_row_from_fields(PLpgSQL_execstate *estate,
69306967
newnulls[fnum]=isnull;
69316968
}
69326969

6970+
/*
6971+
* When strict_multiassignment extra check is active, then ensure
6972+
* there are no unassigned source attributes.
6973+
*/
6974+
if (strict_multiassignment_level&&anum<td_natts)
6975+
{
6976+
/* skip dropped columns in the source descriptor */
6977+
while (anum<td_natts&&
6978+
TupleDescAttr(tupdesc,anum)->attisdropped)
6979+
anum++;
6980+
6981+
if (anum<td_natts)
6982+
ereport(strict_multiassignment_level,
6983+
(errcode(ERRCODE_DATATYPE_MISMATCH),
6984+
errmsg("number of source and target fields in assignment do not match"),
6985+
/* translator: %s represents a name of an extra check */
6986+
errdetail("%s check of %s is active.",
6987+
"strict_multi_assignment",
6988+
strict_multiassignment_level==ERROR ?"extra_errors" :
6989+
"extra_warnings"),
6990+
errhint("Make sure the query returns the exact list of columns.")));
6991+
}
6992+
69336993
values=newvalues;
69346994
nulls=newnulls;
69356995
}
@@ -6986,16 +7046,50 @@ exec_move_row_from_fields(PLpgSQL_execstate *estate,
69867046
}
69877047
else
69887048
{
7049+
/* no source for destination column */
69897050
value= (Datum)0;
69907051
isnull= true;
69917052
valtype=UNKNOWNOID;
69927053
valtypmod=-1;
7054+
7055+
if (strict_multiassignment_level)
7056+
ereport(strict_multiassignment_level,
7057+
(errcode(ERRCODE_DATATYPE_MISMATCH),
7058+
errmsg("number of source and target fields in assignment do not match"),
7059+
/* translator: %s represents a name of an extra check */
7060+
errdetail("%s check of %s is active.",
7061+
"strict_multi_assignment",
7062+
strict_multiassignment_level==ERROR ?"extra_errors" :
7063+
"extra_warnings"),
7064+
errhint("Make sure the query returns the exact list of columns.")));
69937065
}
69947066

69957067
exec_assign_value(estate, (PLpgSQL_datum*)var,
69967068
value,isnull,valtype,valtypmod);
69977069
}
69987070

7071+
/*
7072+
* When strict_multiassignment extra check is active, ensure there
7073+
* are no unassigned source attributes.
7074+
*/
7075+
if (strict_multiassignment_level&&anum<td_natts)
7076+
{
7077+
while (anum<td_natts&&
7078+
TupleDescAttr(tupdesc,anum)->attisdropped)
7079+
anum++;/* skip dropped column in tuple */
7080+
7081+
if (anum<td_natts)
7082+
ereport(strict_multiassignment_level,
7083+
(errcode(ERRCODE_DATATYPE_MISMATCH),
7084+
errmsg("number of source and target fields in assignment do not match"),
7085+
/* translator: %s represents a name of an extra check */
7086+
errdetail("%s check of %s is active.",
7087+
"strict_multi_assignment",
7088+
strict_multiassignment_level==ERROR ?"extra_errors" :
7089+
"extra_warnings"),
7090+
errhint("Make sure the query returns the exact list of columns.")));
7091+
}
7092+
69997093
return;
70007094
}
70017095

‎src/pl/plpgsql/src/pl_handler.c

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,10 @@ plpgsql_extra_checks_check_hook(char **newvalue, void **extra, GucSource source)
9292

9393
if (pg_strcasecmp(tok,"shadowed_variables")==0)
9494
extrachecks |=PLPGSQL_XCHECK_SHADOWVAR;
95+
elseif (pg_strcasecmp(tok,"too_many_rows")==0)
96+
extrachecks |=PLPGSQL_XCHECK_TOOMANYROWS;
97+
elseif (pg_strcasecmp(tok,"strict_multi_assignment")==0)
98+
extrachecks |=PLPGSQL_XCHECK_STRICTMULTIASSIGNMENT;
9599
elseif (pg_strcasecmp(tok,"all")==0||pg_strcasecmp(tok,"none")==0)
96100
{
97101
GUC_check_errdetail("Key word \"%s\" cannot be combined with other key words.",tok);

‎src/pl/plpgsql/src/plpgsql.h

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1135,10 +1135,12 @@ extern bool plpgsql_print_strict_params;
11351135

11361136
externboolplpgsql_check_asserts;
11371137

1138-
/* extra compile-time checks */
1139-
#definePLPGSQL_XCHECK_NONE0
1140-
#definePLPGSQL_XCHECK_SHADOWVAR1
1141-
#definePLPGSQL_XCHECK_ALL((int) ~0)
1138+
/* extra compile-time and run-time checks */
1139+
#definePLPGSQL_XCHECK_NONE0
1140+
#definePLPGSQL_XCHECK_SHADOWVAR(1 << 1)
1141+
#definePLPGSQL_XCHECK_TOOMANYROWS(1 << 2)
1142+
#definePLPGSQL_XCHECK_STRICTMULTIASSIGNMENT(1 << 3)
1143+
#definePLPGSQL_XCHECK_ALL((int) ~0)
11421144

11431145
externintplpgsql_extra_warnings;
11441146
externintplpgsql_extra_errors;

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp