ne 16. 6. 2024 v 16:11 odesílatel Pavel Stehule <pavel.steh...@gmail.com>
napsal:

> 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?
>

fresh rebase



>
> Regards
>
> Pavel
>
>
>
From ebe7c73534f160d75356ac605cdb6a147e806f8f 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 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 ad60e0e8be3..6819c0435f6 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 e7037fa8e81..fa6e2ab1d1f 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 4b9ff515948..699713696d4 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 f3e27c19af5..da1add175ac 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 79cf82b5aed..fd9e10e909c 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 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 0a6945581bd..0e25138b918 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 8c4da955084..e2899a0e888 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 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 28cabc49e9f..845e23d8ae2 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 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 18c91572ae1..fbc21b1f52e 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 0c08a6cc42e..f2977c8ad38 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 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.46.0

From 86b6dcb7a7e5dec3e5796270fa83a2465e803fed 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 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 6819c0435f6..ad60e0e8be3 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 fa6e2ab1d1f..e7037fa8e81 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 699713696d4..4b9ff515948 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 da1add175ac..f3e27c19af5 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 fd9e10e909c..79cf82b5aed 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 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 0e25138b918..0a6945581bd 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 e2899a0e888..8c4da955084 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 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 845e23d8ae2..28cabc49e9f 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 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 fbc21b1f52e..18c91572ae1 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 f2977c8ad38..0c08a6cc42e 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 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.46.0

From 6c61739847d5d1af9da1153e1429c81bb8706aab 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           |   6 ++
 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, 297 insertions(+), 14 deletions(-)

diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml
index 78e4983139b..a27700d6cb3 100644
--- a/doc/src/sgml/plpgsql.sgml
+++ b/doc/src/sgml/plpgsql.sgml
@@ -5386,6 +5386,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 90d98345764..76c25c9948d 100644
--- a/src/backend/executor/spi.c
+++ b/src/backend/executor/spi.c
@@ -2978,11 +2978,17 @@ _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:
 				errcontext("PL/pgSQL 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 84cef57a707..5abce73b030 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -323,6 +323,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
@@ -824,6 +829,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 */
@@ -955,6 +966,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));
+			}
+
 		;
 
 /*
@@ -17426,6 +17477,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 118488c3f30..9d2dde67939 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 be184ec5066..8e41907bd4d 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 fe8d3e51780..433f4067784 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 f1bce708d62..8f9d3ed64d2 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 8182ce28aa1..d3bf9ab6a5f 100644
--- a/src/pl/plpgsql/src/pl_gram.y
+++ b/src/pl/plpgsql/src/pl_gram.y
@@ -183,6 +183,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
@@ -962,13 +963,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");
@@ -1244,7 +1245,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));
 
@@ -1274,6 +1275,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;
@@ -1493,7 +1502,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);
 
@@ -1863,6 +1873,8 @@ stmt_raise		: K_RAISE
 							 */
 							if (tok == SCONST)
 							{
+								RawParseMode pmode_expr;
+
 								/* old style message and parameters */
 								new->message = yylval.str;
 								/*
@@ -1875,13 +1887,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);
@@ -2015,10 +2029,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);
 
@@ -2057,7 +2074,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);
@@ -2642,7 +2659,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);
 }
 
@@ -2652,7 +2669,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);
 }
 
@@ -3845,6 +3862,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)
@@ -3871,6 +3889,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.
 	 */
@@ -3941,7 +3961,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);
 
@@ -3982,7 +4002,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;
@@ -4156,7 +4178,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 980f0961bc8..e7037fa8e81 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 50c3b28472b..b2fb67e8514 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.46.0

Reply via email to