postgres.git / summary / log / commit / refs
commit cc7fae5c2d9effad8a907ffacb4190aca136f87b
Author: Tom Lane <tgl@sss.pgh.pa.us>
Date: Thu Aug 19 16:12:36 2021 +0000
First released in: 15.0, 9.6.24, 10.19, 11.14, 12.9, 13.5, 14.0
Avoid trying to lock OLD/NEW in a rule with FOR UPDATE.
transformLockingClause neglected to exclude the pseudo-RTEs for
OLD/NEW when processing a rule's query. This led to odd errors
or even crashes later on. This bug is very ancient, but it's
not terribly surprising that nobody noticed, since the use-case
for SELECT FOR UPDATE in a non-view rule is somewhere between
thin and non-existent. Still, crashing is not OK.
Per bug #17151 from Zhiyong Wu. Thanks to Masahiko Sawada
for analysis of the problem.
Discussion: https://postgr.es/m/17151-c03a3e6e4ec9aadb@postgresql.org
src/backend/parser/analyze.c | 19 +++++++++++++++++--
src/include/nodes/parsenodes.h | 8 ++++----
src/test/regress/expected/rules.out | 25 +++++++++++++++++++++++++
src/test/regress/sql/rules.sql | 14 ++++++++++++++
4 files changed, 60 insertions(+), 6 deletions(-)
diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c
index b903b89466c..b61eeb9e5c8 100644
--- a/src/backend/parser/analyze.c
+++ b/src/backend/parser/analyze.c
@@ -2631,13 +2631,22 @@ transformLockingClause(ParseState *pstate, Query *qry, LockingClause *lc,
if (lockedRels == NIL)
{
- /* all regular tables used in query */
+ /*
+ * Lock all regular tables used in query and its subqueries. We
+ * examine inFromCl to exclude auto-added RTEs, particularly NEW/OLD
+ * in rules. This is a bit of an abuse of a mostly-obsolete flag, but
+ * it's convenient. We can't rely on the namespace mechanism that has
+ * largely replaced inFromCl, since for example we need to lock
+ * base-relation RTEs even if they are masked by upper joins.
+ */
i = 0;
foreach(rt, qry->rtable)
{
RangeTblEntry *rte = (RangeTblEntry *) lfirst(rt);
++i;
+ if (!rte->inFromCl)
+ continue;
switch (rte->rtekind)
{
case RTE_RELATION:
@@ -2667,7 +2676,11 @@ transformLockingClause(ParseState *pstate, Query *qry, LockingClause *lc,
}
else
{
- /* just the named tables */
+ /*
+ * Lock just the named tables. As above, we allow locking any base
+ * relation regardless of alias-visibility rules, so we need to
+ * examine inFromCl to exclude OLD/NEW.
+ */
foreach(l, lockedRels)
{
RangeVar *thisrel = (RangeVar *) lfirst(l);
@@ -2688,6 +2701,8 @@ transformLockingClause(ParseState *pstate, Query *qry, LockingClause *lc,
RangeTblEntry *rte = (RangeTblEntry *) lfirst(rt);
++i;
+ if (!rte->inFromCl)
+ continue;
if (strcmp(rte->eref->aliasname, thisrel->relname) == 0)
{
switch (rte->rtekind)
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 4dd2d4ecfb7..44bcca62616 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -752,10 +752,10 @@ typedef struct XmlSerialize
* inFromCl marks those range variables that are listed in the FROM clause.
* It's false for RTEs that are added to a query behind the scenes, such
* as the NEW and OLD variables for a rule, or the subqueries of a UNION.
- * This flag is not used anymore during parsing, since the parser now uses
- * a separate "namespace" data structure to control visibility, but it is
- * needed by ruleutils.c to determine whether RTEs should be shown in
- * decompiled queries.
+ * This flag is not used during parsing (except in transformLockingClause,
+ * q.v.); the parser now uses a separate "namespace" data structure to
+ * control visibility. But it is needed by ruleutils.c to determine
+ * whether RTEs should be shown in decompiled queries.
*
* requiredPerms and checkAsUser specify run-time access permissions
* checks to be performed at query startup. The user must have *all*
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index 56b3e8ddc93..aa95c1ff7f4 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -2614,6 +2614,31 @@ select * from only t1_2;
(10 rows)
reset constraint_exclusion;
+-- test FOR UPDATE in rules
+create table rules_base(f1 int, f2 int);
+insert into rules_base values(1,2), (11,12);
+create rule r1 as on update to rules_base do instead
+ select * from rules_base where f1 = 1 for update;
+update rules_base set f2 = f2 + 1;
+ f1 | f2
+----+----
+ 1 | 2
+(1 row)
+
+create or replace rule r1 as on update to rules_base do instead
+ select * from rules_base where f1 = 11 for update of rules_base;
+update rules_base set f2 = f2 + 1;
+ f1 | f2
+----+----
+ 11 | 12
+(1 row)
+
+create or replace rule r1 as on update to rules_base do instead
+ select * from rules_base where f1 = 11 for update of old; -- error
+ERROR: relation "old" in FOR UPDATE clause not found in FROM clause
+LINE 2: select * from rules_base where f1 = 11 for update of old;
+ ^
+drop table rules_base;
-- test various flavors of pg_get_viewdef()
select pg_get_viewdef('shoe'::regclass) as unpretty;
unpretty
diff --git a/src/test/regress/sql/rules.sql b/src/test/regress/sql/rules.sql
index 945503d9153..7bdeb2f5d1c 100644
--- a/src/test/regress/sql/rules.sql
+++ b/src/test/regress/sql/rules.sql
@@ -980,6 +980,20 @@ select * from only t1_2;
reset constraint_exclusion;
+-- test FOR UPDATE in rules
+
+create table rules_base(f1 int, f2 int);
+insert into rules_base values(1,2), (11,12);
+create rule r1 as on update to rules_base do instead
+ select * from rules_base where f1 = 1 for update;
+update rules_base set f2 = f2 + 1;
+create or replace rule r1 as on update to rules_base do instead
+ select * from rules_base where f1 = 11 for update of rules_base;
+update rules_base set f2 = f2 + 1;
+create or replace rule r1 as on update to rules_base do instead
+ select * from rules_base where f1 = 11 for update of old; -- error
+drop table rules_base;
+
-- test various flavors of pg_get_viewdef()
select pg_get_viewdef('shoe'::regclass) as unpretty;
[parent: c09f56fedc73]