From: Tom Lane <tgl@sss.pgh.pa.us>
To: Masahiko Sawada <sawada.mshk@gmail.com>
cc: 253540651@qq.com,
        PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
Subject: Re: BUG #17151: A SEGV in optimizer
In-reply-to: 
 <CAD21AoAC0qpejrwWynTBtzqbd2=xcqP_wJ8=ydFydjAwc4n9_w@mail.gmail.com>
References: <17151-c03a3e6e4ec9aadb@postgresql.org>
 <CAD21AoAC0qpejrwWynTBtzqbd2=xcqP_wJ8=ydFydjAwc4n9_w@mail.gmail.com>
Comments: In-reply-to Masahiko Sawada <sawada.mshk@gmail.com>
	message dated "Wed, 18 Aug 2021 20:52:51 +0900"
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0"
Content-ID: <1458225.1629302114.0@sss.pgh.pa.us>
Date: Wed, 18 Aug 2021 11:55:58 -0400
Message-ID: <1458262.1629302158@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/1458262.1629302158%40sss.pgh.pa.us>
Precedence: bulk

------- =_aaaaaaaaaa0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <1458225.1629302114.1@sss.pgh.pa.us>

Masahiko Sawada <sawada.mshk@gmail.com> writes:
> This seems to happen on all supported versions. A segmentation fault
> happens during "DELETE FROM v0". The minimum reproduce steps are:

> CREATE TABLE v0 ( v1 INTEGER ) ;
> CREATE RULE v1 AS
>     ON DELETE TO v0 DO SELECT
>          FOR UPDATE OF old;
> DELETE FROM v0 ;

> What happens in the DELETE statement is that LockRows node receives a
> tuple from Result node and attempts to acquire a row lock on it but it
> shouldn't. I guess that analyzing the SELECT query in rule v1 should
> raise an error in the first place since there is no 'old' table in its
> FROM clause. I'm still investigating this issue.

Agreed, this ought to be rejected.  In the original case, OLD wasn't
mentioned explicitly, but the parser still attempted to lock it,
which is a slightly different bug.

I think that the fix might be as easy as the attached.  In your
example, this would result in
    ERROR:  relation "old" in FOR UPDATE clause not found in FROM clause
which is not terribly specific, but it's not really wrong either.
Seeing that nobody has complained of this in ~20 years, I doubt
we need to work harder on the error message.

(This passes check-world, but I've not double-checked to make sure
that inFromCl will be set in exactly the cases we want.)

			regards, tom lane


------- =_aaaaaaaaaa0
Content-Type: text/x-diff; name="dont-lock-OLD-and-NEW.patch";
	charset="us-ascii"
Content-ID: <1458225.1629302114.2@sss.pgh.pa.us>
Content-Description: dont-lock-OLD-and-NEW.patch
Content-Transfer-Encoding: quoted-printable

diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c
index 438b077004..f715748d88 100644
--- a/src/backend/parser/analyze.c
+++ b/src/backend/parser/analyze.c
@@ -3177,6 +3177,8 @@ transformLockingClause(ParseState *pstate, Query *qr=
y, LockingClause *lc,
 			RangeTblEntry *rte =3D (RangeTblEntry *) lfirst(rt);
 =

 			++i;
+			if (!rte->inFromCl)
+				continue;		/* ignore OLD/NEW in rules */
 			switch (rte->rtekind)
 			{
 				case RTE_RELATION:
@@ -3227,6 +3229,8 @@ transformLockingClause(ParseState *pstate, Query *qr=
y, LockingClause *lc,
 				RangeTblEntry *rte =3D (RangeTblEntry *) lfirst(rt);
 =

 				++i;
+				if (!rte->inFromCl)
+					continue;	/* ignore OLD/NEW in rules */
 				if (strcmp(rte->eref->aliasname, thisrel->relname) =3D=3D 0)
 				{
 					switch (rte->rtekind)

------- =_aaaaaaaaaa0--