Hi
čt 27. 2. 2025 v 16:33 odesílatel Gilles Darold <gil...@darold.net> napsal:
> Le 07/02/2025 à 23:00, Pavel Stehule a écrit :
> > Hi
> >
> > I rewrote this patch. Instead of enhancing the main SQL parser, it
> > does post parser checks of the parse tree.
> >
> > Now the patch is significantly less invasive (changes are just in
> > plpgsql - mostly in grammar), and it is smaller (without regress tests
> > it has half size).
> >
> > This patch allows the detection of usage of undocumented syntax for
> > plpgsql expressions. Using this undocumented
> > syntax can be the reason why badly written code (missing semicolon)
> > can be quietly executed without any raising of error.
> >
> > Only patch 01 is important - patches 02, 03 are prepared for review.
> > Patch 02 activates a new check by default, and fixes the regress test
> > to be executed. This is important for checking for possible false alarms.
> > Patch 03 disables this check and returns regress tests to their
> > original state.
> >
> > Regards
> >
> > Pavel
>
> Hi Pavel,
>
> I'm reviewing this patch too and I'm facing some documentation issues
> in patch
> v20250207-0001-use-strict-rules-for-parsing-PL-pgSQL-expressions.patch
>
> + it doesn't to allow to detect broken code.
>
> I'm not very good at english but I think it should be: it doesn't allow
> to detect broken code.
>
fixed
>
> Here I think the sentence is not complete:
> + This check is allowed only
> <varname>plpgsql.extra_errors</varname>.
>
> Do you mean: This check is allowed only when
> <varname>plpgsql.extra_errors</varname> is set to 'strict_expr_check'.
>
> Please fix these to be sure of what the code is supposed to do.
>
fixed
Regards
Pavel
>
>
> Thanks
>
> --
> Gilles Darold
> http://www.darold.net/
>
>
From 1c068d87037687114a80b612d9adc11a174cc392 Mon Sep 17 00:00:00 2001
From: "ok...@github.com" <ok...@github.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 280ff3e022e..0015053e0f2 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 2c127a821f1..14e236d57ab 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 caf07e834e5..4c6b3ce998a 100644
--- a/src/pl/plpgsql/src/expected/plpgsql_array.out
+++ b/src/pl/plpgsql/src/expected/plpgsql_array.out
@@ -50,7 +50,7 @@ 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}
do $$ declare a int[] := array[1,2,3];
begin
@@ -64,34 +64,30 @@ end$$;
NOTICE: a = {1,1,2,3,42,3,1,1,2,3,42,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 5d5b25b8308..3ce196de58f 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;
static char *plpgsql_extra_warnings_string = NULL;
static 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 09a76a8416b..da984a99414 100644
--- a/src/pl/plpgsql/src/sql/plpgsql_array.sql
+++ b/src/pl/plpgsql/src/sql/plpgsql_array.sql
@@ -46,7 +46,7 @@ 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$$;
do $$ declare a int[] := array[1,2,3];
begin
@@ -61,26 +61,26 @@ 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 7d004b04031..f408d4f69b6 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 67b28c98902..362f38856d2 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -2622,7 +2622,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 faae99515f7..4e59188196c 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 9930bf5f13c..4e21bd714c7 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 5fc57590126..904d3e623f5 100644
--- a/src/test/regress/expected/stats_ext.out
+++ b/src/test/regress/expected/stats_ext.out
@@ -350,7 +350,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 3458dad1749..7f5757e89c4 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 76b7519678b..84e93ef575e 100644
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -1664,7 +1664,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 a3dbd93468e..4b2f11dcc64 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 ef102a75378..a5d40bd95c7 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 dc6c99951be..88b33ccaef8 100644
--- a/src/test/regress/sql/stats_ext.sql
+++ b/src/test/regress/sql/stats_ext.sql
@@ -223,7 +223,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 1981955ac65..51ae1b31b30 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.48.1
From 9b28e940dd6b8e905f5e091faa38ec35832fff1c Mon Sep 17 00:00:00 2001
From: "ok...@github.com" <ok...@github.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 | 19 ++++
src/pl/plpgsql/src/pl_comp.c | 7 ++
src/pl/plpgsql/src/pl_gram.y | 138 ++++++++++++++++++++++----
src/pl/plpgsql/src/pl_handler.c | 2 +
src/pl/plpgsql/src/plpgsql.h | 1 +
src/test/regress/expected/plpgsql.out | 14 +++
src/test/regress/sql/plpgsql.sql | 14 +++
7 files changed, 177 insertions(+), 18 deletions(-)
diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml
index 78e4983139b..5e45915e7d2 100644
--- a/doc/src/sgml/plpgsql.sgml
+++ b/doc/src/sgml/plpgsql.sgml
@@ -5386,6 +5386,25 @@ 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 allow to detect broken code.
+ </para>
+
+ <para>
+ This check is allowed only when <varname>plpgsql.extra_errors</varname>
+ is set to <literal>"strict_expr_check"</literal>.
+ </para>
+ </listitem>
+ </varlistentry>
</variablelist>
The following example shows the effect of <varname>plpgsql.extra_warnings</varname>
diff --git a/src/pl/plpgsql/src/pl_comp.c b/src/pl/plpgsql/src/pl_comp.c
index f36a244140e..90900ec378b 100644
--- a/src/pl/plpgsql/src/pl_comp.c
+++ b/src/pl/plpgsql/src/pl_comp.c
@@ -908,6 +908,13 @@ 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->extra_errors = PLPGSQL_XCHECK_STRICTEXPRCHECK;
+
function->nstatements = 0;
function->requires_procedure_resowner = false;
function->has_exception_block = false;
diff --git a/src/pl/plpgsql/src/pl_gram.y b/src/pl/plpgsql/src/pl_gram.y
index 8048e040f81..8a6508b0249 100644
--- a/src/pl/plpgsql/src/pl_gram.y
+++ b/src/pl/plpgsql/src/pl_gram.y
@@ -18,6 +18,7 @@
#include "catalog/namespace.h"
#include "catalog/pg_proc.h"
#include "catalog/pg_type.h"
+#include "nodes/nodeFuncs.h"
#include "parser/parser.h"
#include "parser/parse_type.h"
#include "parser/scanner.h"
@@ -71,6 +72,7 @@ static PLpgSQL_expr *read_sql_construct(int until,
const char *expected,
RawParseMode parsemode,
bool isexpression,
+ bool allowlist,
bool valid_sql,
int *startloc,
int *endtoken,
@@ -106,7 +108,7 @@ static PLpgSQL_row *make_scalar_list1(char *initial_name,
PLpgSQL_datum *initial_datum,
int lineno, int location, yyscan_t yyscanner);
static void check_sql_expr(const char *stmt,
- RawParseMode parseMode, int location, yyscan_t yyscanner);
+ RawParseMode parseMode, bool allowlist, int location, yyscan_t yyscanner);
static void plpgsql_sql_error_callback(void *arg);
static PLpgSQL_type *parse_datatype(const char *string, int location, yyscan_t yyscanner);
static void check_labels(const char *start_label,
@@ -117,6 +119,7 @@ static PLpgSQL_expr *read_cursor_args(PLpgSQL_var *cursor, int until,
YYSTYPE *yylvalp, YYLTYPE *yyllocp, yyscan_t yyscanner);
static List *read_raise_options(YYSTYPE *yylvalp, YYLTYPE *yyllocp, yyscan_t yyscanner);
static void check_raise_parameters(PLpgSQL_stmt_raise *stmt);
+static bool is_strict_expr(List *parsetree, int *errpos, bool allowlist);
%}
@@ -193,6 +196,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
@@ -914,7 +918,7 @@ stmt_perform : K_PERFORM
*/
new->expr = read_sql_construct(';', 0, 0, ";",
RAW_PARSE_DEFAULT,
- false, false,
+ false, false, false,
&startloc, NULL,
&yylval, &yylloc, yyscanner);
/* overwrite "perform" ... */
@@ -924,7 +928,7 @@ stmt_perform : K_PERFORM
strlen(new->expr->query));
/* offset syntax error position to account for that */
check_sql_expr(new->expr->query, new->expr->parseMode,
- startloc + 1, yyscanner);
+ false, startloc + 1, yyscanner);
$$ = (PLpgSQL_stmt *) new;
}
@@ -1001,7 +1005,7 @@ stmt_assign : T_DATUM
plpgsql_push_back_token(T_DATUM, &yylval, &yylloc, yyscanner);
new->expr = read_sql_construct(';', 0, 0, ";",
pmode,
- false, true,
+ false, false, true,
NULL, NULL,
&yylval, &yylloc, yyscanner);
mark_expr_as_assignment_source(new->expr, $1.datum);
@@ -1262,7 +1266,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));
@@ -1292,6 +1296,15 @@ opt_case_else :
}
;
+expressions_until_then :
+ {
+ $$ = read_sql_construct(K_THEN, 0, 0, "THEN",
+ RAW_PARSE_PLPGSQL_EXPR, /* expr_list */
+ true, true, true, NULL, NULL,
+ &yylval, &yylloc, yyscanner);
+ }
+ ;
+
stmt_loop : opt_loop_label K_LOOP loop_body
{
PLpgSQL_stmt_loop *new;
@@ -1495,6 +1508,7 @@ for_control : for_variable K_IN
RAW_PARSE_DEFAULT,
true,
false,
+ false,
&expr1loc,
&tok,
&yylval, &yylloc, yyscanner);
@@ -1513,7 +1527,7 @@ for_control : for_variable K_IN
*/
expr1->parseMode = RAW_PARSE_PLPGSQL_EXPR;
check_sql_expr(expr1->query, expr1->parseMode,
- expr1loc, yyscanner);
+ false, expr1loc, yyscanner);
/* Read and check the second one */
expr2 = read_sql_expression2(K_LOOP, K_BY,
@@ -1570,7 +1584,7 @@ for_control : for_variable K_IN
/* Check syntax as a regular query */
check_sql_expr(expr1->query, expr1->parseMode,
- expr1loc, yyscanner);
+ false, expr1loc, yyscanner);
new = palloc0(sizeof(PLpgSQL_stmt_fors));
new->cmd_type = PLPGSQL_STMT_FORS;
@@ -1902,7 +1916,7 @@ stmt_raise : K_RAISE
expr = read_sql_construct(',', ';', K_USING,
", or ; or USING",
RAW_PARSE_PLPGSQL_EXPR,
- true, true,
+ true, false, true,
NULL, &tok,
&yylval, &yylloc, yyscanner);
new->params = lappend(new->params, expr);
@@ -2040,7 +2054,7 @@ stmt_dynexecute : K_EXECUTE
expr = read_sql_construct(K_INTO, K_USING, ';',
"INTO or USING or ;",
RAW_PARSE_PLPGSQL_EXPR,
- true, true,
+ true, false, true,
NULL, &endtoken,
&yylval, &yylloc, yyscanner);
@@ -2080,7 +2094,7 @@ stmt_dynexecute : K_EXECUTE
expr = read_sql_construct(',', ';', K_INTO,
", or ; or INTO",
RAW_PARSE_PLPGSQL_EXPR,
- true, true,
+ true, false, true,
NULL, &endtoken,
&yylval, &yylloc, yyscanner);
new->params = lappend(new->params, expr);
@@ -2713,7 +2727,7 @@ read_sql_expression(int until, const char *expected, YYSTYPE *yylvalp, YYLTYPE *
{
return read_sql_construct(until, 0, 0, expected,
RAW_PARSE_PLPGSQL_EXPR,
- true, true, NULL, NULL,
+ true, false, true, NULL, NULL,
yylvalp, yyllocp, yyscanner);
}
@@ -2724,7 +2738,7 @@ read_sql_expression2(int until, int until2, const char *expected,
{
return read_sql_construct(until, until2, 0, expected,
RAW_PARSE_PLPGSQL_EXPR,
- true, true, NULL, endtoken,
+ true, false, true, NULL, endtoken,
yylvalp, yyllocp, yyscanner);
}
@@ -2734,7 +2748,7 @@ read_sql_stmt(YYSTYPE *yylvalp, YYLTYPE *yyllocp, yyscan_t yyscanner)
{
return read_sql_construct(';', 0, 0, ";",
RAW_PARSE_DEFAULT,
- false, true, NULL, NULL,
+ false, false, true, NULL, NULL,
yylvalp, yyllocp, yyscanner);
}
@@ -2747,6 +2761,7 @@ read_sql_stmt(YYSTYPE *yylvalp, YYLTYPE *yyllocp, yyscan_t yyscanner)
* expected: text to use in complaining that terminator was not found
* parsemode: raw_parser() mode to use
* isexpression: whether to say we're reading an "expression" or a "statement"
+ * allowlist: the result can be list of expressions
* valid_sql: whether to check the syntax of the expr
* startloc: if not NULL, location of first token is stored at *startloc
* endtoken: if not NULL, ending token is stored at *endtoken
@@ -2759,6 +2774,7 @@ read_sql_construct(int until,
const char *expected,
RawParseMode parsemode,
bool isexpression,
+ bool allowlist,
bool valid_sql,
int *startloc,
int *endtoken,
@@ -2854,7 +2870,7 @@ read_sql_construct(int until,
pfree(ds.data);
if (valid_sql)
- check_sql_expr(expr->query, expr->parseMode, startlocation, yyscanner);
+ check_sql_expr(expr->query, expr->parseMode, allowlist, startlocation, yyscanner);
return expr;
}
@@ -3175,7 +3191,7 @@ make_execsql_stmt(int firsttoken, int location, PLword *word, YYSTYPE *yylvalp,
expr = make_plpgsql_expr(ds.data, RAW_PARSE_DEFAULT);
pfree(ds.data);
- check_sql_expr(expr->query, expr->parseMode, location, yyscanner);
+ check_sql_expr(expr->query, expr->parseMode, false, location, yyscanner);
execsql = palloc0(sizeof(PLpgSQL_stmt_execsql));
execsql->cmd_type = PLPGSQL_STMT_EXECSQL;
@@ -3775,11 +3791,15 @@ make_scalar_list1(char *initial_name,
* If no error cursor is provided, we'll just point at "location".
*/
static void
-check_sql_expr(const char *stmt, RawParseMode parseMode, int location, yyscan_t yyscanner)
+check_sql_expr(const char *stmt,
+ RawParseMode parseMode, bool allowlist,
+ int location, yyscan_t yyscanner)
{
sql_error_callback_arg cbarg;
ErrorContextCallback syntax_errcontext;
MemoryContext oldCxt;
+ List *parsetree;
+ int errpos;
if (!plpgsql_check_syntax)
return;
@@ -3793,11 +3813,25 @@ check_sql_expr(const char *stmt, RawParseMode parseMode, int location, yyscan_t
error_context_stack = &syntax_errcontext;
oldCxt = MemoryContextSwitchTo(plpgsql_compile_tmp_cxt);
- (void) raw_parser(stmt, parseMode);
+ parsetree = raw_parser(stmt, parseMode);
MemoryContextSwitchTo(oldCxt);
/* Restore former ereport callback */
error_context_stack = syntax_errcontext.previous;
+
+ if (plpgsql_curr_compile->extra_warnings & PLPGSQL_XCHECK_STRICTEXPRCHECK ||
+ plpgsql_curr_compile->extra_errors & PLPGSQL_XCHECK_STRICTEXPRCHECK)
+ {
+ /* do this check only for expressions */
+ if (parseMode == RAW_PARSE_DEFAULT)
+ return;
+
+ if (!is_strict_expr(parsetree, &errpos, allowlist))
+ ereport(plpgsql_curr_compile->extra_errors & PLPGSQL_XCHECK_STRICTEXPRCHECK ? ERROR : WARNING,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("syntax of expression is not strict"),
+ parser_errposition(errpos != -1 ? location + errpos : location)));
+ }
}
static void
@@ -3831,6 +3865,74 @@ plpgsql_sql_error_callback(void *arg)
errposition(0);
}
+/*
+ * Returns true, when the only targetList is in parsetree. Cursors
+ * can require list of expressions or list of named expressions.
+ */
+static bool
+is_strict_expr(List *parsetree, int *errpos, bool allowlist)
+{
+ RawStmt *rawstmt;
+ SelectStmt *select;
+ int targets = 0;
+ ListCell *lc;
+
+ /* Top should be RawStmt */
+ rawstmt = castNode(RawStmt, linitial(parsetree));
+
+ if (IsA(rawstmt->stmt, SelectStmt))
+ {
+ select = (SelectStmt *) rawstmt->stmt;
+ }
+ else if (IsA(rawstmt->stmt, PLAssignStmt))
+ {
+ select = castNode(SelectStmt, ((PLAssignStmt *) rawstmt->stmt)->val);
+ }
+ else
+ elog(ERROR, "unexpected node type");
+
+ if (!select->targetList)
+ {
+ *errpos = -1;
+ return false;
+ }
+ else
+ *errpos = exprLocation((Node *) select->targetList);
+
+ if (select->distinctClause ||
+ select->fromClause ||
+ select->whereClause ||
+ select->groupClause ||
+ select->groupDistinct ||
+ select->havingClause ||
+ select->windowClause ||
+ select->sortClause ||
+ select->limitOffset ||
+ select->limitCount ||
+ select->limitOption ||
+ select->lockingClause)
+ return false;
+
+ foreach(lc, select->targetList)
+ {
+ ResTarget *rt = castNode(ResTarget, lfirst(lc));
+
+ if (targets++ >= 1 && !allowlist)
+ {
+ *errpos = exprLocation((Node *) rt);
+ return false;
+ }
+
+ if (rt->name)
+ {
+ *errpos = exprLocation((Node *) rt);
+ return false;
+ }
+ }
+
+ return true;
+}
+
/*
* Parse a SQL datatype name and produce a PLpgSQL_type structure.
*
@@ -4011,7 +4113,7 @@ read_cursor_args(PLpgSQL_var *cursor, int until, YYSTYPE *yylvalp, YYLTYPE *yyll
item = read_sql_construct(',', ')', 0,
",\" or \")",
RAW_PARSE_PLPGSQL_EXPR,
- true, true,
+ true, false, true,
NULL, &endtoken,
yylvalp, yyllocp, yyscanner);
diff --git a/src/pl/plpgsql/src/pl_handler.c b/src/pl/plpgsql/src/pl_handler.c
index 5af38d5773b..3ce196de58f 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 d73996e09c0..93dee19313b 100644
--- a/src/pl/plpgsql/src/plpgsql.h
+++ b/src/pl/plpgsql/src/plpgsql.h
@@ -1227,6 +1227,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;
diff --git a/src/test/regress/expected/plpgsql.out b/src/test/regress/expected/plpgsql.out
index 0a6945581bd..4e21bd714c7 100644
--- a/src/test/regress/expected/plpgsql.out
+++ b/src/test/regress/expected/plpgsql.out
@@ -3083,6 +3083,20 @@ select shadowtest(1);
t
(1 row)
+-- test of strict expression check
+set plpgsql.extra_errors to 'strict_expr_check';
+create or replace function strict_expr_check_func()
+returns void as $$
+declare var int;
+begin
+ var = 1
+ delete from pg_class where false;
+end;
+$$ language plpgsql;
+ERROR: syntax of expression is not strict
+LINE 5: var = 1
+ ^
+reset plpgsql.extra_errors;
-- runtime extra checks
set plpgsql.extra_warnings to 'too_many_rows';
do $$
diff --git a/src/test/regress/sql/plpgsql.sql b/src/test/regress/sql/plpgsql.sql
index 18c91572ae1..a5d40bd95c7 100644
--- a/src/test/regress/sql/plpgsql.sql
+++ b/src/test/regress/sql/plpgsql.sql
@@ -2617,6 +2617,20 @@ declare f1 int; begin return 1; end $$ language plpgsql;
select shadowtest(1);
+-- test of strict expression check
+set plpgsql.extra_errors to 'strict_expr_check';
+
+create or replace function strict_expr_check_func()
+returns void as $$
+declare var int;
+begin
+ var = 1
+ delete from pg_class where false;
+end;
+$$ language plpgsql;
+
+reset plpgsql.extra_errors;
+
-- runtime extra checks
set plpgsql.extra_warnings to 'too_many_rows';
--
2.48.1
From f0093580c395542588c658cf61fed8d33260de97 Mon Sep 17 00:00:00 2001
From: "ok...@github.com" <ok...@github.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 0015053e0f2..280ff3e022e 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 14e236d57ab..2c127a821f1 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 4c6b3ce998a..caf07e834e5 100644
--- a/src/pl/plpgsql/src/expected/plpgsql_array.out
+++ b/src/pl/plpgsql/src/expected/plpgsql_array.out
@@ -50,7 +50,7 @@ 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}
do $$ declare a int[] := array[1,2,3];
begin
@@ -64,30 +64,34 @@ end$$;
NOTICE: a = {1,1,2,3,42,3,1,1,2,3,42,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 3ce196de58f..5d5b25b8308 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;
static char *plpgsql_extra_warnings_string = NULL;
static 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 da984a99414..09a76a8416b 100644
--- a/src/pl/plpgsql/src/sql/plpgsql_array.sql
+++ b/src/pl/plpgsql/src/sql/plpgsql_array.sql
@@ -46,7 +46,7 @@ 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$$;
do $$ declare a int[] := array[1,2,3];
begin
@@ -61,26 +61,26 @@ 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 f408d4f69b6..7d004b04031 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 362f38856d2..67b28c98902 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -2622,7 +2622,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 4e59188196c..faae99515f7 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 4e21bd714c7..9930bf5f13c 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 904d3e623f5..5fc57590126 100644
--- a/src/test/regress/expected/stats_ext.out
+++ b/src/test/regress/expected/stats_ext.out
@@ -350,7 +350,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 7f5757e89c4..3458dad1749 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 84e93ef575e..76b7519678b 100644
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -1664,7 +1664,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 4b2f11dcc64..a3dbd93468e 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 a5d40bd95c7..ef102a75378 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 88b33ccaef8..dc6c99951be 100644
--- a/src/test/regress/sql/stats_ext.sql
+++ b/src/test/regress/sql/stats_ext.sql
@@ -223,7 +223,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 51ae1b31b30..1981955ac65 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.48.1
