Hi,
assigned patch try to solve issue reported by Mor Lehr (Missing semicolon
in anonymous plpgsql block does not raise syntax error).
https://www.postgresql.org/message-id/CALyvM2bp_CXMH_Gyq87pmHJRuZDEhV40u9VP8rX=canet2w...@mail.gmail.com
by introducing a new extra error check. With this check only a_expr exprs
are allowed as plpgsql expressions. This is a small step to behaviour
described in SQL/PSM standard (although the language is different, the
expression syntax and features are almost similar. With this check the
undocumented (but supported syntax)
var := column FROM tab
is disallowed. Only ANSI syntax for embedded queries (inside assignment
statement) is allowed
var := (SELECT column FROM tab);
With this check, the reported issue (by Mor Lehr) is detected
default setting
CREATE TABLE foo3(id serial PRIMARY key, txt text);
INSERT INTO foo3 (txt) VALUES ('aaa'),('bbb');
DO $$
DECLARE
l_cnt int;
BEGIN
l_cnt := 1
DELETE FROM foo3 WHERE id=1;
END; $$
-- without reaction - just don't work
(2024-06-16 16:05:55) postgres=# set plpgsql.extra_errors to
'strict_expr_check';
SET
(2024-06-16 16:06:43) postgres=# DO $$
DECLARE
l_cnt int;
BEGIN
l_cnt := 1
DELETE FROM foo3 WHERE id=1;
END; $$;
ERROR: syntax error at or near "DELETE"
LINE 11: DELETE FROM foo3 WHERE id=1;
^
This patch has three parts
1. Introduction strict_expr_check
2. set strict_expr_check as default, and impact on regress tests
3. revert @2
I don't propose to be strict_expr_check active by default.
Comments, notes?
Regards
Pavel
From 51a8cedad8cb3ddbb8a6f5828def01b4e2bb1cac Mon Sep 17 00:00:00 2001
From: "ok...@github.com" <pavel.steh...@gmail.com>
Date: Wed, 12 Jun 2024 21:34:05 +0200
Subject: [PATCH 1/3] use strict rules for parsing PL/pgSQL expressions
Originally the rule PLpgSQL_Expr allows almost all SQL clauses. It was designed
to allow old undocumented syntax
var := col FROM tab;
The reason for support of this "strange" syntax was technical. The PLpgSQL parser
cannot use SQL parser accurately (it was really primitive), and people found
this undocumented syntax. Lattery, when it was possible to do exact parsing, from
compatibility reasons, the parsing of PL/pgSQL expressions allows described syntax.
Unfortunately, with support almost all SQL clauses, the PLpgSQL can accept
really broken code like
DO $$
DECLARE
l_cnt int;
BEGIN
l_cnt := 1
DELETE FROM foo3 WHERE id=1;
END; $$;
proposed patch introduce new extra error check strict_expr_check, that solve
this issue.
---
doc/src/sgml/plpgsql.sgml | 18 ++++
src/backend/executor/spi.c | 7 ++
src/backend/parser/gram.y | 149 +++++++++++++++++++++++++++
src/backend/parser/parser.c | 6 ++
src/include/parser/parser.h | 22 ++++
src/interfaces/ecpg/preproc/parse.pl | 10 +-
src/pl/plpgsql/src/pl_comp.c | 31 ++++++
src/pl/plpgsql/src/pl_gram.y | 48 ++++++---
src/pl/plpgsql/src/pl_handler.c | 2 +
src/pl/plpgsql/src/plpgsql.h | 19 ++++
10 files changed, 298 insertions(+), 14 deletions(-)
diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml
index 6f880b705f..e4b5febf15 100644
--- a/doc/src/sgml/plpgsql.sgml
+++ b/doc/src/sgml/plpgsql.sgml
@@ -5347,6 +5347,24 @@ a_output := a_output || $$ if v_$$ || referrer_keys.kind || $$ like '$$
</para>
</listitem>
</varlistentry>
+
+ <varlistentry id="plpgsql-extra-checks-strict-expr-check">
+ <term><varname>strict_expr_check</varname></term>
+ <listitem>
+ <para>
+ Enabling this check will cause <application>PL/pgSQL</application> to
+ check if a <application>PL/pgSQL</application> expression is just an
+ expression without any SQL clauses like <literal>FROM</literal>,
+ <literal>ORDER BY</literal>. This undocumented form of expressions
+ is allowed for compatibility reasons, but in some special cases
+ it doesn't to allow to detect broken code.
+ </para>
+
+ <para>
+ This check is allowed only <varname>plpgsql.extra_errors</varname>.
+ </para>
+ </listitem>
+ </varlistentry>
</variablelist>
The following example shows the effect of <varname>plpgsql.extra_warnings</varname>
diff --git a/src/backend/executor/spi.c b/src/backend/executor/spi.c
index e516c0a67c..94a48b837d 100644
--- a/src/backend/executor/spi.c
+++ b/src/backend/executor/spi.c
@@ -2976,11 +2976,18 @@ _SPI_error_callback(void *arg)
switch (carg->mode)
{
case RAW_PARSE_PLPGSQL_EXPR:
+ case RAW_PARSE_PLPGSQL_STRICT_EXPR:
+ case RAW_PARSE_PLPGSQL_STRICT_EXPR_LIST:
+ case RAW_PARSE_PLPGSQL_STRICT_NAMED_EXPR_LIST:
+ /* ToDo: fix to PL/pgSQL expression - in another patch */
errcontext("SQL expression \"%s\"", query);
break;
case RAW_PARSE_PLPGSQL_ASSIGN1:
case RAW_PARSE_PLPGSQL_ASSIGN2:
case RAW_PARSE_PLPGSQL_ASSIGN3:
+ case RAW_PARSE_PLPGSQL_STRICT_EXPR_ASSIGN1:
+ case RAW_PARSE_PLPGSQL_STRICT_EXPR_ASSIGN2:
+ case RAW_PARSE_PLPGSQL_STRICT_EXPR_ASSIGN3:
errcontext("PL/pgSQL assignment \"%s\"", query);
break;
default:
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 4d582950b7..bbc13a1a58 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -324,6 +324,11 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
%type <node> select_no_parens select_with_parens select_clause
simple_select values_clause
PLpgSQL_Expr PLAssignStmt
+ PLpgSQLStrictExpr PLpgSQLStrictExprs PLpgSQLStrictNamedExprs
+ PLAssignStmtStrictExpr
+
+%type <target> plpgsql_strict_expr plpgsql_strict_named_expr
+%type <list> plpgsql_strict_expr_list plpgsql_strict_named_expr_list
%type <str> opt_single_name
%type <list> opt_qualified_name
@@ -827,6 +832,12 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
%token MODE_PLPGSQL_ASSIGN1
%token MODE_PLPGSQL_ASSIGN2
%token MODE_PLPGSQL_ASSIGN3
+%token MODE_PLPGSQL_STRICT_EXPR
+%token MODE_PLPGSQL_STRICT_EXPR_LIST
+%token MODE_PLPGSQL_STRICT_NAMED_EXPR_LIST
+%token MODE_PLPGSQL_STRICT_EXPR_ASSIGN1
+%token MODE_PLPGSQL_STRICT_EXPR_ASSIGN2
+%token MODE_PLPGSQL_STRICT_EXPR_ASSIGN3
/* Precedence: lowest to highest */
@@ -958,6 +969,46 @@ parse_toplevel:
pg_yyget_extra(yyscanner)->parsetree =
list_make1(makeRawStmt((Node *) n, 0));
}
+ | MODE_PLPGSQL_STRICT_EXPR PLpgSQLStrictExpr
+ {
+ pg_yyget_extra(yyscanner)->parsetree =
+ list_make1(makeRawStmt($2, 0));
+ }
+ | MODE_PLPGSQL_STRICT_EXPR_LIST PLpgSQLStrictExprs
+ {
+ pg_yyget_extra(yyscanner)->parsetree =
+ list_make1(makeRawStmt($2, 0));
+ }
+ | MODE_PLPGSQL_STRICT_NAMED_EXPR_LIST PLpgSQLStrictNamedExprs
+ {
+ pg_yyget_extra(yyscanner)->parsetree =
+ list_make1(makeRawStmt($2, 0));
+ }
+ | MODE_PLPGSQL_STRICT_EXPR_ASSIGN1 PLAssignStmtStrictExpr
+ {
+ PLAssignStmt *n = (PLAssignStmt *) $2;
+
+ n->nnames = 1;
+ pg_yyget_extra(yyscanner)->parsetree =
+ list_make1(makeRawStmt((Node *) n, 0));
+ }
+ | MODE_PLPGSQL_STRICT_EXPR_ASSIGN2 PLAssignStmtStrictExpr
+ {
+ PLAssignStmt *n = (PLAssignStmt *) $2;
+
+ n->nnames = 2;
+ pg_yyget_extra(yyscanner)->parsetree =
+ list_make1(makeRawStmt((Node *) n, 0));
+ }
+ | MODE_PLPGSQL_STRICT_EXPR_ASSIGN3 PLAssignStmtStrictExpr
+ {
+ PLAssignStmt *n = (PLAssignStmt *) $2;
+
+ n->nnames = 3;
+ pg_yyget_extra(yyscanner)->parsetree =
+ list_make1(makeRawStmt((Node *) n, 0));
+ }
+
;
/*
@@ -17496,6 +17547,104 @@ plassign_equals: COLON_EQUALS
| '='
;
+/*
+ * In "strict" mode plpgsql expressions are just an a_expr. From compatibility
+ * reasons (with default mode) it returns SelectStmt still.
+ */
+PLpgSQLStrictExpr: a_expr
+ {
+ SelectStmt *n = makeNode(SelectStmt);
+ ResTarget *rt = makeNode(ResTarget);
+
+ rt->name = NULL;
+ rt->indirection = NIL;
+ rt->val = (Node *) $1;
+ rt->location = @1;
+
+ n->targetList = list_make1((Node *) rt);
+ $$ = (Node *) n;
+ }
+ ;
+
+PLAssignStmtStrictExpr: plassign_target opt_indirection plassign_equals PLpgSQLStrictExpr
+ {
+ PLAssignStmt *n = makeNode(PLAssignStmt);
+
+ n->name = $1;
+ n->indirection = check_indirection($2, yyscanner);
+ /* nnames will be filled by calling production */
+ n->val = (SelectStmt *) $4;
+ n->location = @1;
+ $$ = (Node *) n;
+ }
+ ;
+
+/*
+ * Used for unnamed plpgsql cursor's argument and plpgsql case in
+ * "strict" mode.
+ */
+PLpgSQLStrictExprs: plpgsql_strict_expr_list
+ {
+ SelectStmt *n = makeNode(SelectStmt);
+
+ n->targetList = $1;
+ $$ = (Node *) n;
+ }
+ ;
+
+plpgsql_strict_expr_list:
+ plpgsql_strict_expr
+ {
+ $$ = list_make1($1);
+ }
+ | plpgsql_strict_expr_list ',' plpgsql_strict_expr
+ {
+ $$ = lappend($1, $3);
+ }
+ ;
+
+plpgsql_strict_expr: a_expr
+ {
+ $$ = makeNode(ResTarget);
+ $$->name = NULL;
+ $$->indirection = NIL;
+ $$->val = (Node *) $1;
+ $$->location = @1;
+ }
+ ;
+
+/*
+ * Used for named cursor's arguments in "strict" mode
+ */
+PLpgSQLStrictNamedExprs: plpgsql_strict_named_expr_list
+ {
+ SelectStmt *n = makeNode(SelectStmt);
+
+ n->targetList = $1;
+ $$ = (Node *) n;
+ }
+ ;
+
+plpgsql_strict_named_expr_list:
+ plpgsql_strict_named_expr
+ {
+ $$ = list_make1($1);
+ }
+ | plpgsql_strict_named_expr_list ',' plpgsql_strict_named_expr
+ {
+ $$ = lappend($1, $3);
+ }
+ ;
+
+plpgsql_strict_named_expr: a_expr AS ColLabel
+ {
+ $$ = makeNode(ResTarget);
+ $$->name = $3;
+ $$->indirection = NIL;
+ $$->val = (Node *) $1;
+ $$->location = @1;
+ }
+ ;
/*
* Name classification hierarchy.
diff --git a/src/backend/parser/parser.c b/src/backend/parser/parser.c
index 118488c3f3..9d2dde6793 100644
--- a/src/backend/parser/parser.c
+++ b/src/backend/parser/parser.c
@@ -62,6 +62,12 @@ raw_parser(const char *str, RawParseMode mode)
[RAW_PARSE_PLPGSQL_ASSIGN1] = MODE_PLPGSQL_ASSIGN1,
[RAW_PARSE_PLPGSQL_ASSIGN2] = MODE_PLPGSQL_ASSIGN2,
[RAW_PARSE_PLPGSQL_ASSIGN3] = MODE_PLPGSQL_ASSIGN3,
+ [RAW_PARSE_PLPGSQL_STRICT_EXPR] = MODE_PLPGSQL_STRICT_EXPR,
+ [RAW_PARSE_PLPGSQL_STRICT_EXPR_LIST] = MODE_PLPGSQL_STRICT_EXPR_LIST,
+ [RAW_PARSE_PLPGSQL_STRICT_NAMED_EXPR_LIST] = MODE_PLPGSQL_STRICT_NAMED_EXPR_LIST,
+ [RAW_PARSE_PLPGSQL_STRICT_EXPR_ASSIGN1] = MODE_PLPGSQL_STRICT_EXPR_ASSIGN1,
+ [RAW_PARSE_PLPGSQL_STRICT_EXPR_ASSIGN2] = MODE_PLPGSQL_STRICT_EXPR_ASSIGN2,
+ [RAW_PARSE_PLPGSQL_STRICT_EXPR_ASSIGN3] = MODE_PLPGSQL_STRICT_EXPR_ASSIGN3,
};
yyextra.have_lookahead = true;
diff --git a/src/include/parser/parser.h b/src/include/parser/parser.h
index be184ec506..8e41907bd4 100644
--- a/src/include/parser/parser.h
+++ b/src/include/parser/parser.h
@@ -33,6 +33,22 @@
* RAW_PARSE_PLPGSQL_ASSIGNn: parse a PL/pgSQL assignment statement,
* and return a one-element List containing a RawStmt node. "n"
* gives the number of dotted names comprising the target ColumnRef.
+ *
+ * RAW_PARSE_PLPGSQL_STRICT_EXPR: parse a PL/pgSQL expression, and
+ * return a one-element List containing a RwaStmt node. The result is
+ * compatible with RAW_PARSE_PLPGSQL_EXPR, but parser allows only
+ * a_expr (instead almost all complete query).
+ *
+ * RAW_PARSE_PLPGSQL_STRICT_EXPR_LIST: parse a comma separated list
+ * of PL/pgSQL expressions (only a_expr are allowed). It is used by
+ * PLpGSQL CASE and OPEN commands.
+ *
+ * RAW_PARSE_PLPGSQL_STRICT_NAMED_EXPR_LIST: parse a comma separated
+ * list of a_expr node with labels. It is used for evaluation of
+ * named arguments of PLpgSQL OPEN (cursor) statement.
+ *
+ * RAW_PARSE_PLPGSQL_STRICT_EXPR_ASSIGNn: parse a PL/pgSQL assignment
+ * statement, but only a_expr are allowed).
*/
typedef enum
{
@@ -42,6 +58,12 @@ typedef enum
RAW_PARSE_PLPGSQL_ASSIGN1,
RAW_PARSE_PLPGSQL_ASSIGN2,
RAW_PARSE_PLPGSQL_ASSIGN3,
+ RAW_PARSE_PLPGSQL_STRICT_EXPR,
+ RAW_PARSE_PLPGSQL_STRICT_EXPR_LIST,
+ RAW_PARSE_PLPGSQL_STRICT_NAMED_EXPR_LIST,
+ RAW_PARSE_PLPGSQL_STRICT_EXPR_ASSIGN1,
+ RAW_PARSE_PLPGSQL_STRICT_EXPR_ASSIGN2,
+ RAW_PARSE_PLPGSQL_STRICT_EXPR_ASSIGN3,
} RawParseMode;
/* Values for the backslash_quote GUC */
diff --git a/src/interfaces/ecpg/preproc/parse.pl b/src/interfaces/ecpg/preproc/parse.pl
index fe8d3e5178..433f406778 100644
--- a/src/interfaces/ecpg/preproc/parse.pl
+++ b/src/interfaces/ecpg/preproc/parse.pl
@@ -89,7 +89,15 @@ my %replace_types = (
'PLpgSQL_Expr' => 'ignore',
'PLAssignStmt' => 'ignore',
'plassign_target' => 'ignore',
- 'plassign_equals' => 'ignore',);
+ 'plassign_equals' => 'ignore',
+ 'plpgsql_strict_expr' => 'ignore',
+ 'plpgsql_strict_named_expr' => 'ignore',
+ 'plpgsql_strict_expr_list' => 'ignore',
+ 'plpgsql_strict_named_expr_list' => 'ignore',
+ 'PLpgSQLStrictExpr' => 'ignore',
+ 'PLpgSQLStrictExprs' => 'ignore',
+ 'PLpgSQLStrictNamedExprs' => 'ignore',
+ 'PLAssignStmtStrictExpr' => 'ignore',);
# these replace_line commands excise certain keywords from the core keyword
# lists. Be sure to account for these in ColLabel and related productions.
diff --git a/src/pl/plpgsql/src/pl_comp.c b/src/pl/plpgsql/src/pl_comp.c
index f1bce708d6..8f9d3ed64d 100644
--- a/src/pl/plpgsql/src/pl_comp.c
+++ b/src/pl/plpgsql/src/pl_comp.c
@@ -84,6 +84,23 @@ static const ExceptionLabelMap exception_label_map[] = {
{NULL, 0}
};
+static const PLpgSQL_parse_modes default_parse_modes = {
+ RAW_PARSE_PLPGSQL_EXPR,
+ RAW_PARSE_PLPGSQL_EXPR,
+ RAW_PARSE_PLPGSQL_EXPR,
+ RAW_PARSE_PLPGSQL_ASSIGN1,
+ RAW_PARSE_PLPGSQL_ASSIGN2,
+ RAW_PARSE_PLPGSQL_ASSIGN3
+};
+
+static const PLpgSQL_parse_modes strict_expr_parse_modes = {
+ RAW_PARSE_PLPGSQL_STRICT_EXPR,
+ RAW_PARSE_PLPGSQL_STRICT_EXPR_LIST,
+ RAW_PARSE_PLPGSQL_STRICT_NAMED_EXPR_LIST,
+ RAW_PARSE_PLPGSQL_STRICT_EXPR_ASSIGN1,
+ RAW_PARSE_PLPGSQL_STRICT_EXPR_ASSIGN2,
+ RAW_PARSE_PLPGSQL_STRICT_EXPR_ASSIGN3
+};
/* ----------
* static prototypes
@@ -355,6 +372,11 @@ do_compile(FunctionCallInfo fcinfo,
function->extra_warnings = forValidator ? plpgsql_extra_warnings : 0;
function->extra_errors = forValidator ? plpgsql_extra_errors : 0;
+ if (function->extra_errors & PLPGSQL_XCHECK_STRICTEXPRCHECK)
+ function->pmodes = &strict_expr_parse_modes;
+ else
+ function->pmodes = &default_parse_modes;
+
if (is_dml_trigger)
function->fn_is_trigger = PLPGSQL_DML_TRIGGER;
else if (is_event_trigger)
@@ -898,6 +920,15 @@ plpgsql_compile_inline(char *proc_source)
function->extra_warnings = 0;
function->extra_errors = 0;
+ /*
+ * Although function->extra_errors is disabled, we want to
+ * do strict_expr_check inside annoymous block too.
+ */
+ if (plpgsql_extra_errors & PLPGSQL_XCHECK_STRICTEXPRCHECK)
+ function->pmodes = &strict_expr_parse_modes;
+ else
+ function->pmodes = &default_parse_modes;
+
function->nstatements = 0;
function->requires_procedure_resowner = false;
diff --git a/src/pl/plpgsql/src/pl_gram.y b/src/pl/plpgsql/src/pl_gram.y
index a29d2dfacd..d86792195d 100644
--- a/src/pl/plpgsql/src/pl_gram.y
+++ b/src/pl/plpgsql/src/pl_gram.y
@@ -178,6 +178,7 @@ static void check_raise_parameters(PLpgSQL_stmt_raise *stmt);
%type <expr> expr_until_semi
%type <expr> expr_until_then expr_until_loop opt_expr_until_when
%type <expr> opt_exitcond
+%type <expr> expressions_until_then
%type <var> cursor_variable
%type <datum> decl_cursor_arg
@@ -957,13 +958,13 @@ stmt_assign : T_DATUM
switch ($1.ident ? 1 : list_length($1.idents))
{
case 1:
- pmode = RAW_PARSE_PLPGSQL_ASSIGN1;
+ pmode = plpgsql_curr_compile->pmodes->pmode_assign1;
break;
case 2:
- pmode = RAW_PARSE_PLPGSQL_ASSIGN2;
+ pmode = plpgsql_curr_compile->pmodes->pmode_assign2;
break;
case 3:
- pmode = RAW_PARSE_PLPGSQL_ASSIGN3;
+ pmode = plpgsql_curr_compile->pmodes->pmode_assign3;
break;
default:
elog(ERROR, "unexpected number of names");
@@ -1239,7 +1240,7 @@ case_when_list : case_when_list case_when
}
;
-case_when : K_WHEN expr_until_then proc_sect
+case_when : K_WHEN expressions_until_then proc_sect
{
PLpgSQL_case_when *new = palloc(sizeof(PLpgSQL_case_when));
@@ -1269,6 +1270,14 @@ opt_case_else :
}
;
+expressions_until_then :
+ {
+ $$ = read_sql_construct(K_THEN, 0, 0, "THEN",
+ plpgsql_curr_compile->pmodes->pmode_expr_list,
+ true, true, NULL, NULL);
+ }
+ ;
+
stmt_loop : opt_loop_label K_LOOP loop_body
{
PLpgSQL_stmt_loop *new;
@@ -1488,7 +1497,8 @@ for_control : for_variable K_IN
* Relabel first expression as an expression;
* then we can check its syntax.
*/
- expr1->parseMode = RAW_PARSE_PLPGSQL_EXPR;
+ expr1->parseMode = plpgsql_curr_compile->pmodes->pmode_expr;
+
check_sql_expr(expr1->query, expr1->parseMode,
expr1loc);
@@ -1858,6 +1868,8 @@ stmt_raise : K_RAISE
*/
if (tok == SCONST)
{
+ RawParseMode pmode_expr;
+
/* old style message and parameters */
new->message = yylval.str;
/*
@@ -1870,13 +1882,15 @@ stmt_raise : K_RAISE
if (tok != ',' && tok != ';' && tok != K_USING)
yyerror("syntax error");
+ pmode_expr = plpgsql_curr_compile->pmodes->pmode_expr;
+
while (tok == ',')
{
PLpgSQL_expr *expr;
expr = read_sql_construct(',', ';', K_USING,
", or ; or USING",
- RAW_PARSE_PLPGSQL_EXPR,
+ pmode_expr,
true, true,
NULL, &tok);
new->params = lappend(new->params, expr);
@@ -2010,10 +2024,13 @@ stmt_dynexecute : K_EXECUTE
PLpgSQL_stmt_dynexecute *new;
PLpgSQL_expr *expr;
int endtoken;
+ RawParseMode pmode_expr;
+
+ pmode_expr = plpgsql_curr_compile->pmodes->pmode_expr;
expr = read_sql_construct(K_INTO, K_USING, ';',
"INTO or USING or ;",
- RAW_PARSE_PLPGSQL_EXPR,
+ pmode_expr,
true, true,
NULL, &endtoken);
@@ -2052,7 +2069,7 @@ stmt_dynexecute : K_EXECUTE
{
expr = read_sql_construct(',', ';', K_INTO,
", or ; or INTO",
- RAW_PARSE_PLPGSQL_EXPR,
+ pmode_expr,
true, true,
NULL, &endtoken);
new->params = lappend(new->params, expr);
@@ -2637,7 +2654,7 @@ static PLpgSQL_expr *
read_sql_expression(int until, const char *expected)
{
return read_sql_construct(until, 0, 0, expected,
- RAW_PARSE_PLPGSQL_EXPR,
+ plpgsql_curr_compile->pmodes->pmode_expr,
true, true, NULL, NULL);
}
@@ -2647,7 +2664,7 @@ read_sql_expression2(int until, int until2, const char *expected,
int *endtoken)
{
return read_sql_construct(until, until2, 0, expected,
- RAW_PARSE_PLPGSQL_EXPR,
+ plpgsql_curr_compile->pmodes->pmode_expr,
true, true, NULL, endtoken);
}
@@ -3840,6 +3857,7 @@ read_cursor_args(PLpgSQL_var *cursor, int until)
char **argv;
StringInfoData ds;
bool any_named = false;
+ RawParseMode pmode_expr;
tok = yylex();
if (cursor->cursor_explicit_argrow < 0)
@@ -3866,6 +3884,8 @@ read_cursor_args(PLpgSQL_var *cursor, int until)
cursor->refname),
parser_errposition(yylloc)));
+ pmode_expr = plpgsql_curr_compile->pmodes->pmode_expr;
+
/*
* Read the arguments, one by one.
*/
@@ -3936,7 +3956,7 @@ read_cursor_args(PLpgSQL_var *cursor, int until)
*/
item = read_sql_construct(',', ')', 0,
",\" or \")",
- RAW_PARSE_PLPGSQL_EXPR,
+ pmode_expr,
true, true,
NULL, &endtoken);
@@ -3977,7 +3997,9 @@ read_cursor_args(PLpgSQL_var *cursor, int until)
expr = palloc0(sizeof(PLpgSQL_expr));
expr->query = pstrdup(ds.data);
- expr->parseMode = RAW_PARSE_PLPGSQL_EXPR;
+ expr->parseMode = any_named ?
+ plpgsql_curr_compile->pmodes->pmode_named_expr_list
+ : plpgsql_curr_compile->pmodes->pmode_expr_list;
expr->plan = NULL;
expr->paramnos = NULL;
expr->target_param = -1;
@@ -4151,7 +4173,7 @@ make_case(int location, PLpgSQL_expr *t_expr,
StringInfoData ds;
/* We expect to have expressions not statements */
- Assert(expr->parseMode == RAW_PARSE_PLPGSQL_EXPR);
+ Assert(expr->parseMode == plpgsql_curr_compile->pmodes->pmode_expr_list);
/* Do the string hacking */
initStringInfo(&ds);
diff --git a/src/pl/plpgsql/src/pl_handler.c b/src/pl/plpgsql/src/pl_handler.c
index fce459ade0..f34b3befc4 100644
--- a/src/pl/plpgsql/src/pl_handler.c
+++ b/src/pl/plpgsql/src/pl_handler.c
@@ -94,6 +94,8 @@ plpgsql_extra_checks_check_hook(char **newvalue, void **extra, GucSource source)
extrachecks |= PLPGSQL_XCHECK_TOOMANYROWS;
else if (pg_strcasecmp(tok, "strict_multi_assignment") == 0)
extrachecks |= PLPGSQL_XCHECK_STRICTMULTIASSIGNMENT;
+ else if (pg_strcasecmp(tok, "strict_expr_check") == 0)
+ extrachecks |= PLPGSQL_XCHECK_STRICTEXPRCHECK;
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 50c3b28472..b2fb67e851 100644
--- a/src/pl/plpgsql/src/plpgsql.h
+++ b/src/pl/plpgsql/src/plpgsql.h
@@ -960,6 +960,21 @@ typedef enum PLpgSQL_trigtype
PLPGSQL_NOT_TRIGGER,
} PLpgSQL_trigtype;
+/*
+ * Raw parse modes, that should be used for expressions,
+ * assignment, expression's list. When extra_errors.strict_expr_check
+ * is active, then only a_expr parsing is allowed.
+ */
+typedef struct PLpgSQL_parse_modes
+{
+ RawParseMode pmode_expr;
+ RawParseMode pmode_expr_list;
+ RawParseMode pmode_named_expr_list;
+ RawParseMode pmode_assign1;
+ RawParseMode pmode_assign2;
+ RawParseMode pmode_assign3;
+} PLpgSQL_parse_modes;
+
/*
* Complete compiled function
*/
@@ -1010,6 +1025,9 @@ typedef struct PLpgSQL_function
unsigned int nstatements; /* counter for assigning stmtids */
bool requires_procedure_resowner; /* contains CALL or DO? */
+ /* Raw parse modes configuration */
+ const PLpgSQL_parse_modes *pmodes;
+
/* these fields change when the function is used */
struct PLpgSQL_execstate *cur_estate;
unsigned long use_count;
@@ -1204,6 +1222,7 @@ extern bool plpgsql_check_asserts;
#define PLPGSQL_XCHECK_SHADOWVAR (1 << 1)
#define PLPGSQL_XCHECK_TOOMANYROWS (1 << 2)
#define PLPGSQL_XCHECK_STRICTMULTIASSIGNMENT (1 << 3)
+#define PLPGSQL_XCHECK_STRICTEXPRCHECK (1 << 4)
#define PLPGSQL_XCHECK_ALL ((int) ~0)
extern int plpgsql_extra_warnings;
--
2.45.2
From 2b3546e14260021d52a58803cba0183dc808ace0 Mon Sep 17 00:00:00 2001
From: "ok...@github.com" <pavel.steh...@gmail.com>
Date: Sun, 16 Jun 2024 15:52:38 +0200
Subject: [PATCH 3/3] set plpgsql.extra_errors to "none"
Purpose of previous commit was to run tests with active strict_expr_check.
Now, reset to default and revert all changes from previous commit.
---
.../basic_archive/expected/basic_archive.out | 4 +--
contrib/basic_archive/sql/basic_archive.sql | 4 +--
src/pl/plpgsql/src/expected/plpgsql_array.out | 34 ++++++++-----------
src/pl/plpgsql/src/pl_handler.c | 4 +--
src/pl/plpgsql/src/sql/plpgsql_array.sql | 14 ++++----
.../recovery/t/026_overwrite_contrecord.pl | 4 +--
src/test/regress/expected/alter_table.out | 2 +-
src/test/regress/expected/plancache.out | 2 +-
src/test/regress/expected/plpgsql.out | 12 +++----
src/test/regress/expected/stats_ext.out | 2 +-
src/test/regress/expected/transactions.out | 4 +--
src/test/regress/sql/alter_table.sql | 2 +-
src/test/regress/sql/plancache.sql | 2 +-
src/test/regress/sql/plpgsql.sql | 12 +++----
src/test/regress/sql/stats_ext.sql | 2 +-
src/test/regress/sql/transactions.sql | 4 +--
16 files changed, 52 insertions(+), 56 deletions(-)
diff --git a/contrib/basic_archive/expected/basic_archive.out b/contrib/basic_archive/expected/basic_archive.out
index 280ff3e022..0015053e0f 100644
--- a/contrib/basic_archive/expected/basic_archive.out
+++ b/contrib/basic_archive/expected/basic_archive.out
@@ -11,8 +11,8 @@ DECLARE
loops int := 0;
BEGIN
LOOP
- archived := (SELECT count(*) > 0 FROM pg_ls_dir('.', false, false) a
- WHERE a ~ '^[0-9A-F]{24}$');
+ archived := count(*) > 0 FROM pg_ls_dir('.', false, false) a
+ WHERE a ~ '^[0-9A-F]{24}$';
IF archived OR loops > 120 * 10 THEN EXIT; END IF;
PERFORM pg_sleep(0.1);
loops := loops + 1;
diff --git a/contrib/basic_archive/sql/basic_archive.sql b/contrib/basic_archive/sql/basic_archive.sql
index 2c127a821f..14e236d57a 100644
--- a/contrib/basic_archive/sql/basic_archive.sql
+++ b/contrib/basic_archive/sql/basic_archive.sql
@@ -7,8 +7,8 @@ DECLARE
loops int := 0;
BEGIN
LOOP
- archived := (SELECT count(*) > 0 FROM pg_ls_dir('.', false, false) a
- WHERE a ~ '^[0-9A-F]{24}$');
+ archived := count(*) > 0 FROM pg_ls_dir('.', false, false) a
+ WHERE a ~ '^[0-9A-F]{24}$';
IF archived OR loops > 120 * 10 THEN EXIT; END IF;
PERFORM pg_sleep(0.1);
loops := loops + 1;
diff --git a/src/pl/plpgsql/src/expected/plpgsql_array.out b/src/pl/plpgsql/src/expected/plpgsql_array.out
index 6819c0435f..ad60e0e8be 100644
--- a/src/pl/plpgsql/src/expected/plpgsql_array.out
+++ b/src/pl/plpgsql/src/expected/plpgsql_array.out
@@ -50,38 +50,34 @@ do $$ declare a quadarray;
begin a.c1[1].i := 11; raise notice 'a = %, a.c1[1].i = %', a, a.c1[1].i; end$$;
NOTICE: a = ("{""(,11)""}",), a.c1[1].i = 11
do $$ declare a int[];
-begin a := (select array_agg(x) from (values(1),(2),(3)) v(x)); raise notice 'a = %', a; end$$;
+begin a := array_agg(x) from (values(1),(2),(3)) v(x); raise notice 'a = %', a; end$$;
NOTICE: a = {1,2,3}
create temp table onecol as select array[1,2] as f1;
do $$ declare a int[];
-begin a := (select f1 from onecol); raise notice 'a = %', a; end$$;
+begin a := f1 from onecol; raise notice 'a = %', a; end$$;
NOTICE: a = {1,2}
do $$ declare a int[];
-begin a := (select * from onecol for update); raise notice 'a = %', a; end$$;
+begin a := * from onecol for update; raise notice 'a = %', a; end$$;
NOTICE: a = {1,2}
-- error cases:
do $$ declare a int[];
-begin a := (select from onecol); raise notice 'a = %', a; end$$;
-ERROR: subquery must return only one column
-LINE 1: a := (select from onecol)
- ^
-QUERY: a := (select from onecol)
-CONTEXT: PL/pgSQL function inline_code_block line 2 at assignment
+begin a := from onecol; raise notice 'a = %', a; end$$;
+ERROR: assignment source returned 0 columns
+CONTEXT: PL/pgSQL assignment "a := from onecol"
+PL/pgSQL function inline_code_block line 2 at assignment
do $$ declare a int[];
-begin a := (select f1, f1 from onecol); raise notice 'a = %', a; end$$;
-ERROR: subquery must return only one column
-LINE 1: a := (select f1, f1 from onecol)
- ^
-QUERY: a := (select f1, f1 from onecol)
-CONTEXT: PL/pgSQL function inline_code_block line 2 at assignment
+begin a := f1, f1 from onecol; raise notice 'a = %', a; end$$;
+ERROR: assignment source returned 2 columns
+CONTEXT: PL/pgSQL assignment "a := f1, f1 from onecol"
+PL/pgSQL function inline_code_block line 2 at assignment
insert into onecol values(array[11]);
do $$ declare a int[];
-begin a := (select f1 from onecol); raise notice 'a = %', a; end$$;
-ERROR: more than one row returned by a subquery used as an expression
-CONTEXT: PL/pgSQL assignment "a := (select f1 from onecol)"
+begin a := f1 from onecol; raise notice 'a = %', a; end$$;
+ERROR: query returned more than one row
+CONTEXT: query: a := f1 from onecol
PL/pgSQL function inline_code_block line 2 at assignment
do $$ declare a int[];
-begin a := (select f1 from onecol limit 1); raise notice 'a = %', a; end$$;
+begin a := f1 from onecol limit 1; raise notice 'a = %', a; end$$;
NOTICE: a = {1,2}
do $$ declare a real;
begin a[1] := 2; raise notice 'a = %', a; end$$;
diff --git a/src/pl/plpgsql/src/pl_handler.c b/src/pl/plpgsql/src/pl_handler.c
index a17cfe01e6..f34b3befc4 100644
--- a/src/pl/plpgsql/src/pl_handler.c
+++ b/src/pl/plpgsql/src/pl_handler.c
@@ -50,7 +50,7 @@ bool plpgsql_check_asserts = true;
char *plpgsql_extra_warnings_string = NULL;
char *plpgsql_extra_errors_string = NULL;
int plpgsql_extra_warnings;
-int plpgsql_extra_errors = PLPGSQL_XCHECK_STRICTEXPRCHECK;
+int plpgsql_extra_errors;
/* Hook for plugins */
PLpgSQL_plugin **plpgsql_plugin_ptr = NULL;
@@ -193,7 +193,7 @@ _PG_init(void)
gettext_noop("List of programming constructs that should produce an error."),
NULL,
&plpgsql_extra_errors_string,
- "strict_expr_check",
+ "none",
PGC_USERSET, GUC_LIST_INPUT,
plpgsql_extra_checks_check_hook,
plpgsql_extra_errors_assign_hook,
diff --git a/src/pl/plpgsql/src/sql/plpgsql_array.sql b/src/pl/plpgsql/src/sql/plpgsql_array.sql
index 699713696d..4b9ff51594 100644
--- a/src/pl/plpgsql/src/sql/plpgsql_array.sql
+++ b/src/pl/plpgsql/src/sql/plpgsql_array.sql
@@ -46,31 +46,31 @@ do $$ declare a quadarray;
begin a.c1[1].i := 11; raise notice 'a = %, a.c1[1].i = %', a, a.c1[1].i; end$$;
do $$ declare a int[];
-begin a := (select array_agg(x) from (values(1),(2),(3)) v(x)); raise notice 'a = %', a; end$$;
+begin a := array_agg(x) from (values(1),(2),(3)) v(x); raise notice 'a = %', a; end$$;
create temp table onecol as select array[1,2] as f1;
do $$ declare a int[];
-begin a := (select f1 from onecol); raise notice 'a = %', a; end$$;
+begin a := f1 from onecol; raise notice 'a = %', a; end$$;
do $$ declare a int[];
-begin a := (select * from onecol for update); raise notice 'a = %', a; end$$;
+begin a := * from onecol for update; raise notice 'a = %', a; end$$;
-- error cases:
do $$ declare a int[];
-begin a := (select from onecol); raise notice 'a = %', a; end$$;
+begin a := from onecol; raise notice 'a = %', a; end$$;
do $$ declare a int[];
-begin a := (select f1, f1 from onecol); raise notice 'a = %', a; end$$;
+begin a := f1, f1 from onecol; raise notice 'a = %', a; end$$;
insert into onecol values(array[11]);
do $$ declare a int[];
-begin a := (select f1 from onecol); raise notice 'a = %', a; end$$;
+begin a := f1 from onecol; raise notice 'a = %', a; end$$;
do $$ declare a int[];
-begin a := (select f1 from onecol limit 1); raise notice 'a = %', a; end$$;
+begin a := f1 from onecol limit 1; raise notice 'a = %', a; end$$;
do $$ declare a real;
begin a[1] := 2; raise notice 'a = %', a; end$$;
diff --git a/src/test/recovery/t/026_overwrite_contrecord.pl b/src/test/recovery/t/026_overwrite_contrecord.pl
index da1add175a..f3e27c19af 100644
--- a/src/test/recovery/t/026_overwrite_contrecord.pl
+++ b/src/test/recovery/t/026_overwrite_contrecord.pl
@@ -33,7 +33,7 @@ $node->safe_psql(
'postgres', q{
DO $$
DECLARE
- wal_segsize int := (SELECT setting::int FROM pg_settings WHERE name = 'wal_segment_size');
+ wal_segsize int := setting::int FROM pg_settings WHERE name = 'wal_segment_size';
remain int;
iters int := 0;
BEGIN
@@ -43,7 +43,7 @@ BEGIN
from generate_series(1, 10) g;
remain := wal_segsize - (pg_current_wal_insert_lsn() - '0/0') % wal_segsize;
- IF (SELECT remain < 2 * setting::int from pg_settings where name = 'block_size') THEN
+ IF remain < 2 * setting::int from pg_settings where name = 'block_size' THEN
RAISE log 'exiting after % iterations, % bytes to end of WAL segment', iters, remain;
EXIT;
END IF;
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index 1cf06e9ebf..673361e840 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -2627,7 +2627,7 @@ LANGUAGE plpgsql AS $$
DECLARE
v_relfilenode oid;
BEGIN
- v_relfilenode := (SELECT relfilenode FROM pg_class WHERE oid = p_tablename);
+ v_relfilenode := relfilenode FROM pg_class WHERE oid = p_tablename;
EXECUTE p_ddl;
diff --git a/src/test/regress/expected/plancache.out b/src/test/regress/expected/plancache.out
index faae99515f..4e59188196 100644
--- a/src/test/regress/expected/plancache.out
+++ b/src/test/regress/expected/plancache.out
@@ -139,7 +139,7 @@ create temp view v1 as
select 2+2 as f1;
create function cache_test_2() returns int as $$
begin
- return (select f1 from v1);
+ return f1 from v1;
end$$ language plpgsql;
select cache_test_2();
cache_test_2
diff --git a/src/test/regress/expected/plpgsql.out b/src/test/regress/expected/plpgsql.out
index 3a9a1828f2..074af8f33a 100644
--- a/src/test/regress/expected/plpgsql.out
+++ b/src/test/regress/expected/plpgsql.out
@@ -121,7 +121,7 @@ create trigger tg_room_ad after delete
-- ************************************************************
create function tg_wslot_biu() returns trigger as $$
begin
- if (select count(*) = 0 from Room where roomno = new.roomno) then
+ if count(*) = 0 from Room where roomno = new.roomno then
raise exception 'Room % does not exist', new.roomno;
end if;
return new;
@@ -286,7 +286,7 @@ begin
raise exception ''no manual manipulation of HSlot'';
end if;
if tg_op = ''UPDATE'' and new.hubname != old.hubname then
- if (select count(*) > 0 from Hub where name = old.hubname) then
+ if count(*) > 0 from Hub where name = old.hubname then
raise exception ''no manual manipulation of HSlot'';
end if;
end if;
@@ -942,12 +942,12 @@ begin
return retval || pslot_backlink_view(psrec.slotlink);
end if;
if sltype = ''HS'' then
- retval := (select comment from Hub H, HSlot HS
+ retval := comment from Hub H, HSlot HS
where HS.slotname = psrec.slotlink
- and H.name = HS.hubname);
+ and H.name = HS.hubname;
retval := retval || '' slot '';
- retval := (select retval || slotno::text from HSlot
- where slotname = psrec.slotlink);
+ retval := retval || slotno::text from HSlot
+ where slotname = psrec.slotlink;
return retval;
end if;
return psrec.slotlink;
diff --git a/src/test/regress/expected/stats_ext.out b/src/test/regress/expected/stats_ext.out
index e2899a0e88..8c4da95508 100644
--- a/src/test/regress/expected/stats_ext.out
+++ b/src/test/regress/expected/stats_ext.out
@@ -327,7 +327,7 @@ CREATE STATISTICS tststats.s8 ON a, b FROM tststats.pt;
CREATE STATISTICS tststats.s9 ON a, b FROM tststats.pt1;
DO $$
DECLARE
- relname text := (SELECT reltoastrelid::regclass FROM pg_class WHERE oid = 'tststats.t'::regclass);
+ relname text := reltoastrelid::regclass FROM pg_class WHERE oid = 'tststats.t'::regclass;
BEGIN
EXECUTE 'CREATE STATISTICS tststats.s10 ON a, b FROM ' || relname;
EXCEPTION WHEN wrong_object_type THEN
diff --git a/src/test/regress/expected/transactions.out b/src/test/regress/expected/transactions.out
index 3458dad174..7f5757e89c 100644
--- a/src/test/regress/expected/transactions.out
+++ b/src/test/regress/expected/transactions.out
@@ -543,7 +543,7 @@ select * from xacttest;
rollback;
-- Now the same test with plpgsql (since it depends on SPI which is different)
create or replace function max_xacttest() returns smallint language plpgsql as
-'begin return (select max(a) from xacttest); end' stable;
+'begin return max(a) from xacttest; end' stable;
begin;
update xacttest set a = max_xacttest() + 10 where a > 0;
select * from xacttest;
@@ -558,7 +558,7 @@ select * from xacttest;
rollback;
create or replace function max_xacttest() returns smallint language plpgsql as
-'begin return (select max(a) from xacttest); end' volatile;
+'begin return max(a) from xacttest; end' volatile;
begin;
update xacttest set a = max_xacttest() + 10 where a > 0;
select * from xacttest;
diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql
index 8d89b06e38..8c8fa27a6a 100644
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -1667,7 +1667,7 @@ LANGUAGE plpgsql AS $$
DECLARE
v_relfilenode oid;
BEGIN
- v_relfilenode := (SELECT relfilenode FROM pg_class WHERE oid = p_tablename);
+ v_relfilenode := relfilenode FROM pg_class WHERE oid = p_tablename;
EXECUTE p_ddl;
diff --git a/src/test/regress/sql/plancache.sql b/src/test/regress/sql/plancache.sql
index a3dbd93468..4b2f11dcc6 100644
--- a/src/test/regress/sql/plancache.sql
+++ b/src/test/regress/sql/plancache.sql
@@ -81,7 +81,7 @@ create temp view v1 as
create function cache_test_2() returns int as $$
begin
- return (select f1 from v1);
+ return f1 from v1;
end$$ language plpgsql;
select cache_test_2();
diff --git a/src/test/regress/sql/plpgsql.sql b/src/test/regress/sql/plpgsql.sql
index fbc21b1f52..18c91572ae 100644
--- a/src/test/regress/sql/plpgsql.sql
+++ b/src/test/regress/sql/plpgsql.sql
@@ -161,7 +161,7 @@ create trigger tg_room_ad after delete
-- ************************************************************
create function tg_wslot_biu() returns trigger as $$
begin
- if (select count(*) = 0 from Room where roomno = new.roomno) then
+ if count(*) = 0 from Room where roomno = new.roomno then
raise exception 'Room % does not exist', new.roomno;
end if;
return new;
@@ -348,7 +348,7 @@ begin
raise exception ''no manual manipulation of HSlot'';
end if;
if tg_op = ''UPDATE'' and new.hubname != old.hubname then
- if (select count(*) > 0 from Hub where name = old.hubname) then
+ if count(*) > 0 from Hub where name = old.hubname then
raise exception ''no manual manipulation of HSlot'';
end if;
end if;
@@ -1071,12 +1071,12 @@ begin
return retval || pslot_backlink_view(psrec.slotlink);
end if;
if sltype = ''HS'' then
- retval := (select comment from Hub H, HSlot HS
+ retval := comment from Hub H, HSlot HS
where HS.slotname = psrec.slotlink
- and H.name = HS.hubname);
+ and H.name = HS.hubname;
retval := retval || '' slot '';
- retval := (select retval || slotno::text from HSlot
- where slotname = psrec.slotlink);
+ retval := retval || slotno::text from HSlot
+ where slotname = psrec.slotlink;
return retval;
end if;
return psrec.slotlink;
diff --git a/src/test/regress/sql/stats_ext.sql b/src/test/regress/sql/stats_ext.sql
index f2977c8ad3..0c08a6cc42 100644
--- a/src/test/regress/sql/stats_ext.sql
+++ b/src/test/regress/sql/stats_ext.sql
@@ -209,7 +209,7 @@ CREATE STATISTICS tststats.s8 ON a, b FROM tststats.pt;
CREATE STATISTICS tststats.s9 ON a, b FROM tststats.pt1;
DO $$
DECLARE
- relname text := (SELECT reltoastrelid::regclass FROM pg_class WHERE oid = 'tststats.t'::regclass);
+ relname text := reltoastrelid::regclass FROM pg_class WHERE oid = 'tststats.t'::regclass;
BEGIN
EXECUTE 'CREATE STATISTICS tststats.s10 ON a, b FROM ' || relname;
EXCEPTION WHEN wrong_object_type THEN
diff --git a/src/test/regress/sql/transactions.sql b/src/test/regress/sql/transactions.sql
index 1981955ac6..51ae1b31b3 100644
--- a/src/test/regress/sql/transactions.sql
+++ b/src/test/regress/sql/transactions.sql
@@ -320,7 +320,7 @@ rollback;
-- Now the same test with plpgsql (since it depends on SPI which is different)
create or replace function max_xacttest() returns smallint language plpgsql as
-'begin return (select max(a) from xacttest); end' stable;
+'begin return max(a) from xacttest; end' stable;
begin;
update xacttest set a = max_xacttest() + 10 where a > 0;
@@ -328,7 +328,7 @@ select * from xacttest;
rollback;
create or replace function max_xacttest() returns smallint language plpgsql as
-'begin return (select max(a) from xacttest); end' volatile;
+'begin return max(a) from xacttest; end' volatile;
begin;
update xacttest set a = max_xacttest() + 10 where a > 0;
--
2.45.2
From c41b3e5ebee92968f2f3276b02c46ca08c91973d Mon Sep 17 00:00:00 2001
From: "ok...@github.com" <pavel.steh...@gmail.com>
Date: Sun, 16 Jun 2024 08:13:53 +0200
Subject: [PATCH 2/3] simply check of strict-expr-check on regress test
This patch enable strict-expr-check by default to be possible to see
the impact of this option on regress test. Next commit will revert
this option. The strict-expr-check should not be enabled by default.
This commit is done just for testing.
---
.../basic_archive/expected/basic_archive.out | 4 +--
contrib/basic_archive/sql/basic_archive.sql | 4 +--
src/pl/plpgsql/src/expected/plpgsql_array.out | 34 +++++++++++--------
src/pl/plpgsql/src/pl_handler.c | 4 +--
src/pl/plpgsql/src/sql/plpgsql_array.sql | 14 ++++----
.../recovery/t/026_overwrite_contrecord.pl | 4 +--
src/test/regress/expected/alter_table.out | 2 +-
src/test/regress/expected/plancache.out | 2 +-
src/test/regress/expected/plpgsql.out | 12 +++----
src/test/regress/expected/stats_ext.out | 2 +-
src/test/regress/expected/transactions.out | 4 +--
src/test/regress/sql/alter_table.sql | 2 +-
src/test/regress/sql/plancache.sql | 2 +-
src/test/regress/sql/plpgsql.sql | 12 +++----
src/test/regress/sql/stats_ext.sql | 2 +-
src/test/regress/sql/transactions.sql | 4 +--
16 files changed, 56 insertions(+), 52 deletions(-)
diff --git a/contrib/basic_archive/expected/basic_archive.out b/contrib/basic_archive/expected/basic_archive.out
index 0015053e0f..280ff3e022 100644
--- a/contrib/basic_archive/expected/basic_archive.out
+++ b/contrib/basic_archive/expected/basic_archive.out
@@ -11,8 +11,8 @@ DECLARE
loops int := 0;
BEGIN
LOOP
- archived := count(*) > 0 FROM pg_ls_dir('.', false, false) a
- WHERE a ~ '^[0-9A-F]{24}$';
+ archived := (SELECT count(*) > 0 FROM pg_ls_dir('.', false, false) a
+ WHERE a ~ '^[0-9A-F]{24}$');
IF archived OR loops > 120 * 10 THEN EXIT; END IF;
PERFORM pg_sleep(0.1);
loops := loops + 1;
diff --git a/contrib/basic_archive/sql/basic_archive.sql b/contrib/basic_archive/sql/basic_archive.sql
index 14e236d57a..2c127a821f 100644
--- a/contrib/basic_archive/sql/basic_archive.sql
+++ b/contrib/basic_archive/sql/basic_archive.sql
@@ -7,8 +7,8 @@ DECLARE
loops int := 0;
BEGIN
LOOP
- archived := count(*) > 0 FROM pg_ls_dir('.', false, false) a
- WHERE a ~ '^[0-9A-F]{24}$';
+ archived := (SELECT count(*) > 0 FROM pg_ls_dir('.', false, false) a
+ WHERE a ~ '^[0-9A-F]{24}$');
IF archived OR loops > 120 * 10 THEN EXIT; END IF;
PERFORM pg_sleep(0.1);
loops := loops + 1;
diff --git a/src/pl/plpgsql/src/expected/plpgsql_array.out b/src/pl/plpgsql/src/expected/plpgsql_array.out
index ad60e0e8be..6819c0435f 100644
--- a/src/pl/plpgsql/src/expected/plpgsql_array.out
+++ b/src/pl/plpgsql/src/expected/plpgsql_array.out
@@ -50,34 +50,38 @@ do $$ declare a quadarray;
begin a.c1[1].i := 11; raise notice 'a = %, a.c1[1].i = %', a, a.c1[1].i; end$$;
NOTICE: a = ("{""(,11)""}",), a.c1[1].i = 11
do $$ declare a int[];
-begin a := array_agg(x) from (values(1),(2),(3)) v(x); raise notice 'a = %', a; end$$;
+begin a := (select array_agg(x) from (values(1),(2),(3)) v(x)); raise notice 'a = %', a; end$$;
NOTICE: a = {1,2,3}
create temp table onecol as select array[1,2] as f1;
do $$ declare a int[];
-begin a := f1 from onecol; raise notice 'a = %', a; end$$;
+begin a := (select f1 from onecol); raise notice 'a = %', a; end$$;
NOTICE: a = {1,2}
do $$ declare a int[];
-begin a := * from onecol for update; raise notice 'a = %', a; end$$;
+begin a := (select * from onecol for update); raise notice 'a = %', a; end$$;
NOTICE: a = {1,2}
-- error cases:
do $$ declare a int[];
-begin a := from onecol; raise notice 'a = %', a; end$$;
-ERROR: assignment source returned 0 columns
-CONTEXT: PL/pgSQL assignment "a := from onecol"
-PL/pgSQL function inline_code_block line 2 at assignment
+begin a := (select from onecol); raise notice 'a = %', a; end$$;
+ERROR: subquery must return only one column
+LINE 1: a := (select from onecol)
+ ^
+QUERY: a := (select from onecol)
+CONTEXT: PL/pgSQL function inline_code_block line 2 at assignment
do $$ declare a int[];
-begin a := f1, f1 from onecol; raise notice 'a = %', a; end$$;
-ERROR: assignment source returned 2 columns
-CONTEXT: PL/pgSQL assignment "a := f1, f1 from onecol"
-PL/pgSQL function inline_code_block line 2 at assignment
+begin a := (select f1, f1 from onecol); raise notice 'a = %', a; end$$;
+ERROR: subquery must return only one column
+LINE 1: a := (select f1, f1 from onecol)
+ ^
+QUERY: a := (select f1, f1 from onecol)
+CONTEXT: PL/pgSQL function inline_code_block line 2 at assignment
insert into onecol values(array[11]);
do $$ declare a int[];
-begin a := f1 from onecol; raise notice 'a = %', a; end$$;
-ERROR: query returned more than one row
-CONTEXT: query: a := f1 from onecol
+begin a := (select f1 from onecol); raise notice 'a = %', a; end$$;
+ERROR: more than one row returned by a subquery used as an expression
+CONTEXT: PL/pgSQL assignment "a := (select f1 from onecol)"
PL/pgSQL function inline_code_block line 2 at assignment
do $$ declare a int[];
-begin a := f1 from onecol limit 1; raise notice 'a = %', a; end$$;
+begin a := (select f1 from onecol limit 1); raise notice 'a = %', a; end$$;
NOTICE: a = {1,2}
do $$ declare a real;
begin a[1] := 2; raise notice 'a = %', a; end$$;
diff --git a/src/pl/plpgsql/src/pl_handler.c b/src/pl/plpgsql/src/pl_handler.c
index f34b3befc4..a17cfe01e6 100644
--- a/src/pl/plpgsql/src/pl_handler.c
+++ b/src/pl/plpgsql/src/pl_handler.c
@@ -50,7 +50,7 @@ bool plpgsql_check_asserts = true;
char *plpgsql_extra_warnings_string = NULL;
char *plpgsql_extra_errors_string = NULL;
int plpgsql_extra_warnings;
-int plpgsql_extra_errors;
+int plpgsql_extra_errors = PLPGSQL_XCHECK_STRICTEXPRCHECK;
/* Hook for plugins */
PLpgSQL_plugin **plpgsql_plugin_ptr = NULL;
@@ -193,7 +193,7 @@ _PG_init(void)
gettext_noop("List of programming constructs that should produce an error."),
NULL,
&plpgsql_extra_errors_string,
- "none",
+ "strict_expr_check",
PGC_USERSET, GUC_LIST_INPUT,
plpgsql_extra_checks_check_hook,
plpgsql_extra_errors_assign_hook,
diff --git a/src/pl/plpgsql/src/sql/plpgsql_array.sql b/src/pl/plpgsql/src/sql/plpgsql_array.sql
index 4b9ff51594..699713696d 100644
--- a/src/pl/plpgsql/src/sql/plpgsql_array.sql
+++ b/src/pl/plpgsql/src/sql/plpgsql_array.sql
@@ -46,31 +46,31 @@ do $$ declare a quadarray;
begin a.c1[1].i := 11; raise notice 'a = %, a.c1[1].i = %', a, a.c1[1].i; end$$;
do $$ declare a int[];
-begin a := array_agg(x) from (values(1),(2),(3)) v(x); raise notice 'a = %', a; end$$;
+begin a := (select array_agg(x) from (values(1),(2),(3)) v(x)); raise notice 'a = %', a; end$$;
create temp table onecol as select array[1,2] as f1;
do $$ declare a int[];
-begin a := f1 from onecol; raise notice 'a = %', a; end$$;
+begin a := (select f1 from onecol); raise notice 'a = %', a; end$$;
do $$ declare a int[];
-begin a := * from onecol for update; raise notice 'a = %', a; end$$;
+begin a := (select * from onecol for update); raise notice 'a = %', a; end$$;
-- error cases:
do $$ declare a int[];
-begin a := from onecol; raise notice 'a = %', a; end$$;
+begin a := (select from onecol); raise notice 'a = %', a; end$$;
do $$ declare a int[];
-begin a := f1, f1 from onecol; raise notice 'a = %', a; end$$;
+begin a := (select f1, f1 from onecol); raise notice 'a = %', a; end$$;
insert into onecol values(array[11]);
do $$ declare a int[];
-begin a := f1 from onecol; raise notice 'a = %', a; end$$;
+begin a := (select f1 from onecol); raise notice 'a = %', a; end$$;
do $$ declare a int[];
-begin a := f1 from onecol limit 1; raise notice 'a = %', a; end$$;
+begin a := (select f1 from onecol limit 1); raise notice 'a = %', a; end$$;
do $$ declare a real;
begin a[1] := 2; raise notice 'a = %', a; end$$;
diff --git a/src/test/recovery/t/026_overwrite_contrecord.pl b/src/test/recovery/t/026_overwrite_contrecord.pl
index f3e27c19af..da1add175a 100644
--- a/src/test/recovery/t/026_overwrite_contrecord.pl
+++ b/src/test/recovery/t/026_overwrite_contrecord.pl
@@ -33,7 +33,7 @@ $node->safe_psql(
'postgres', q{
DO $$
DECLARE
- wal_segsize int := setting::int FROM pg_settings WHERE name = 'wal_segment_size';
+ wal_segsize int := (SELECT setting::int FROM pg_settings WHERE name = 'wal_segment_size');
remain int;
iters int := 0;
BEGIN
@@ -43,7 +43,7 @@ BEGIN
from generate_series(1, 10) g;
remain := wal_segsize - (pg_current_wal_insert_lsn() - '0/0') % wal_segsize;
- IF remain < 2 * setting::int from pg_settings where name = 'block_size' THEN
+ IF (SELECT remain < 2 * setting::int from pg_settings where name = 'block_size') THEN
RAISE log 'exiting after % iterations, % bytes to end of WAL segment', iters, remain;
EXIT;
END IF;
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index 673361e840..1cf06e9ebf 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -2627,7 +2627,7 @@ LANGUAGE plpgsql AS $$
DECLARE
v_relfilenode oid;
BEGIN
- v_relfilenode := relfilenode FROM pg_class WHERE oid = p_tablename;
+ v_relfilenode := (SELECT relfilenode FROM pg_class WHERE oid = p_tablename);
EXECUTE p_ddl;
diff --git a/src/test/regress/expected/plancache.out b/src/test/regress/expected/plancache.out
index 4e59188196..faae99515f 100644
--- a/src/test/regress/expected/plancache.out
+++ b/src/test/regress/expected/plancache.out
@@ -139,7 +139,7 @@ create temp view v1 as
select 2+2 as f1;
create function cache_test_2() returns int as $$
begin
- return f1 from v1;
+ return (select f1 from v1);
end$$ language plpgsql;
select cache_test_2();
cache_test_2
diff --git a/src/test/regress/expected/plpgsql.out b/src/test/regress/expected/plpgsql.out
index 074af8f33a..3a9a1828f2 100644
--- a/src/test/regress/expected/plpgsql.out
+++ b/src/test/regress/expected/plpgsql.out
@@ -121,7 +121,7 @@ create trigger tg_room_ad after delete
-- ************************************************************
create function tg_wslot_biu() returns trigger as $$
begin
- if count(*) = 0 from Room where roomno = new.roomno then
+ if (select count(*) = 0 from Room where roomno = new.roomno) then
raise exception 'Room % does not exist', new.roomno;
end if;
return new;
@@ -286,7 +286,7 @@ begin
raise exception ''no manual manipulation of HSlot'';
end if;
if tg_op = ''UPDATE'' and new.hubname != old.hubname then
- if count(*) > 0 from Hub where name = old.hubname then
+ if (select count(*) > 0 from Hub where name = old.hubname) then
raise exception ''no manual manipulation of HSlot'';
end if;
end if;
@@ -942,12 +942,12 @@ begin
return retval || pslot_backlink_view(psrec.slotlink);
end if;
if sltype = ''HS'' then
- retval := comment from Hub H, HSlot HS
+ retval := (select comment from Hub H, HSlot HS
where HS.slotname = psrec.slotlink
- and H.name = HS.hubname;
+ and H.name = HS.hubname);
retval := retval || '' slot '';
- retval := retval || slotno::text from HSlot
- where slotname = psrec.slotlink;
+ retval := (select retval || slotno::text from HSlot
+ where slotname = psrec.slotlink);
return retval;
end if;
return psrec.slotlink;
diff --git a/src/test/regress/expected/stats_ext.out b/src/test/regress/expected/stats_ext.out
index 8c4da95508..e2899a0e88 100644
--- a/src/test/regress/expected/stats_ext.out
+++ b/src/test/regress/expected/stats_ext.out
@@ -327,7 +327,7 @@ CREATE STATISTICS tststats.s8 ON a, b FROM tststats.pt;
CREATE STATISTICS tststats.s9 ON a, b FROM tststats.pt1;
DO $$
DECLARE
- relname text := reltoastrelid::regclass FROM pg_class WHERE oid = 'tststats.t'::regclass;
+ relname text := (SELECT reltoastrelid::regclass FROM pg_class WHERE oid = 'tststats.t'::regclass);
BEGIN
EXECUTE 'CREATE STATISTICS tststats.s10 ON a, b FROM ' || relname;
EXCEPTION WHEN wrong_object_type THEN
diff --git a/src/test/regress/expected/transactions.out b/src/test/regress/expected/transactions.out
index 7f5757e89c..3458dad174 100644
--- a/src/test/regress/expected/transactions.out
+++ b/src/test/regress/expected/transactions.out
@@ -543,7 +543,7 @@ select * from xacttest;
rollback;
-- Now the same test with plpgsql (since it depends on SPI which is different)
create or replace function max_xacttest() returns smallint language plpgsql as
-'begin return max(a) from xacttest; end' stable;
+'begin return (select max(a) from xacttest); end' stable;
begin;
update xacttest set a = max_xacttest() + 10 where a > 0;
select * from xacttest;
@@ -558,7 +558,7 @@ select * from xacttest;
rollback;
create or replace function max_xacttest() returns smallint language plpgsql as
-'begin return max(a) from xacttest; end' volatile;
+'begin return (select max(a) from xacttest); end' volatile;
begin;
update xacttest set a = max_xacttest() + 10 where a > 0;
select * from xacttest;
diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql
index 8c8fa27a6a..8d89b06e38 100644
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -1667,7 +1667,7 @@ LANGUAGE plpgsql AS $$
DECLARE
v_relfilenode oid;
BEGIN
- v_relfilenode := relfilenode FROM pg_class WHERE oid = p_tablename;
+ v_relfilenode := (SELECT relfilenode FROM pg_class WHERE oid = p_tablename);
EXECUTE p_ddl;
diff --git a/src/test/regress/sql/plancache.sql b/src/test/regress/sql/plancache.sql
index 4b2f11dcc6..a3dbd93468 100644
--- a/src/test/regress/sql/plancache.sql
+++ b/src/test/regress/sql/plancache.sql
@@ -81,7 +81,7 @@ create temp view v1 as
create function cache_test_2() returns int as $$
begin
- return f1 from v1;
+ return (select f1 from v1);
end$$ language plpgsql;
select cache_test_2();
diff --git a/src/test/regress/sql/plpgsql.sql b/src/test/regress/sql/plpgsql.sql
index 18c91572ae..fbc21b1f52 100644
--- a/src/test/regress/sql/plpgsql.sql
+++ b/src/test/regress/sql/plpgsql.sql
@@ -161,7 +161,7 @@ create trigger tg_room_ad after delete
-- ************************************************************
create function tg_wslot_biu() returns trigger as $$
begin
- if count(*) = 0 from Room where roomno = new.roomno then
+ if (select count(*) = 0 from Room where roomno = new.roomno) then
raise exception 'Room % does not exist', new.roomno;
end if;
return new;
@@ -348,7 +348,7 @@ begin
raise exception ''no manual manipulation of HSlot'';
end if;
if tg_op = ''UPDATE'' and new.hubname != old.hubname then
- if count(*) > 0 from Hub where name = old.hubname then
+ if (select count(*) > 0 from Hub where name = old.hubname) then
raise exception ''no manual manipulation of HSlot'';
end if;
end if;
@@ -1071,12 +1071,12 @@ begin
return retval || pslot_backlink_view(psrec.slotlink);
end if;
if sltype = ''HS'' then
- retval := comment from Hub H, HSlot HS
+ retval := (select comment from Hub H, HSlot HS
where HS.slotname = psrec.slotlink
- and H.name = HS.hubname;
+ and H.name = HS.hubname);
retval := retval || '' slot '';
- retval := retval || slotno::text from HSlot
- where slotname = psrec.slotlink;
+ retval := (select retval || slotno::text from HSlot
+ where slotname = psrec.slotlink);
return retval;
end if;
return psrec.slotlink;
diff --git a/src/test/regress/sql/stats_ext.sql b/src/test/regress/sql/stats_ext.sql
index 0c08a6cc42..f2977c8ad3 100644
--- a/src/test/regress/sql/stats_ext.sql
+++ b/src/test/regress/sql/stats_ext.sql
@@ -209,7 +209,7 @@ CREATE STATISTICS tststats.s8 ON a, b FROM tststats.pt;
CREATE STATISTICS tststats.s9 ON a, b FROM tststats.pt1;
DO $$
DECLARE
- relname text := reltoastrelid::regclass FROM pg_class WHERE oid = 'tststats.t'::regclass;
+ relname text := (SELECT reltoastrelid::regclass FROM pg_class WHERE oid = 'tststats.t'::regclass);
BEGIN
EXECUTE 'CREATE STATISTICS tststats.s10 ON a, b FROM ' || relname;
EXCEPTION WHEN wrong_object_type THEN
diff --git a/src/test/regress/sql/transactions.sql b/src/test/regress/sql/transactions.sql
index 51ae1b31b3..1981955ac6 100644
--- a/src/test/regress/sql/transactions.sql
+++ b/src/test/regress/sql/transactions.sql
@@ -320,7 +320,7 @@ rollback;
-- Now the same test with plpgsql (since it depends on SPI which is different)
create or replace function max_xacttest() returns smallint language plpgsql as
-'begin return max(a) from xacttest; end' stable;
+'begin return (select max(a) from xacttest); end' stable;
begin;
update xacttest set a = max_xacttest() + 10 where a > 0;
@@ -328,7 +328,7 @@ select * from xacttest;
rollback;
create or replace function max_xacttest() returns smallint language plpgsql as
-'begin return max(a) from xacttest; end' volatile;
+'begin return (select max(a) from xacttest); end' volatile;
begin;
update xacttest set a = max_xacttest() + 10 where a > 0;
--
2.45.2
