From: Tom Lane <tgl@sss.pgh.pa.us>
To: Richard Guo <guofenglinux@gmail.com>
cc: cyg0810@gmail.com,
        PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
Subject: Re: BUG #17094: FailedAssertion at planner.c
In-reply-to: <1974959.1625761759@sss.pgh.pa.us>
References: <17094-bf15dff55eaf2e28@postgresql.org>
 <CAMbWs48ypEC6i6eZ77rbBbpmYa5ehuQ_M3TkDTWdieqBWAg=XQ@mail.gmail.com>
 <1974959.1625761759@sss.pgh.pa.us>
Comments: In-reply-to Tom Lane <tgl@sss.pgh.pa.us>
	message dated "Thu, 08 Jul 2021 12:29:19 -0400"
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0"
Content-ID: <2032511.1625764917.0@sss.pgh.pa.us>
Date: Thu, 08 Jul 2021 13:22:06 -0400
Message-ID: <2032521.1625764926@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/2032521.1625764926%40sss.pgh.pa.us>
Precedence: bulk

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

I wrote:
> I'm inclined to think we have to reject cases like this.

Concretely, about like this.  I noticed that the adjacent error
cases in RewriteQuery were mostly not covered either by the
regression tests, so I added tests to hit them all.

			regards, tom lane


------- =_aaaaaaaaaa0
Content-Type: text/x-diff;
	name="tighten-rule-rewrite-in-WITH-restrictions.patch";
	charset="us-ascii"
Content-ID: <2032511.1625764917.2@sss.pgh.pa.us>
Content-Description: tighten-rule-rewrite-in-WITH-restrictions.patch
Content-Transfer-Encoding: quoted-printable

diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/re=
writeHandler.c
index 88a9e95e33..54fd6d6fb2 100644
--- a/src/backend/rewrite/rewriteHandler.c
+++ b/src/backend/rewrite/rewriteHandler.c
@@ -3585,15 +3585,29 @@ RewriteQuery(Query *parsetree, List *rewrite_event=
s)
 =

 		/*
 		 * Currently we can only handle unconditional, single-statement DO
-		 * INSTEAD rules correctly; we have to get exactly one Query out of
-		 * the rewrite operation to stuff back into the CTE node.
+		 * INSTEAD rules correctly; we have to get exactly one non-utility
+		 * Query out of the rewrite operation to stuff back into the CTE node.
 		 */
 		if (list_length(newstuff) =3D=3D 1)
 		{
-			/* Push the single Query back into the CTE node */
+			/* Must check it's not a utility command */
 			ctequery =3D linitial_node(Query, newstuff);
+			if (!(ctequery->commandType =3D=3D CMD_SELECT ||
+				  ctequery->commandType =3D=3D CMD_UPDATE ||
+				  ctequery->commandType =3D=3D CMD_INSERT ||
+				  ctequery->commandType =3D=3D CMD_DELETE))
+			{
+				/*
+				 * Currently it could only be NOTIFY; this error message will
+				 * need work if we ever allow other utility commands in rules.
+				 */
+				ereport(ERROR,
+						(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+						 errmsg("DO INSTEAD NOTIFY rules are not supported for data-modifyi=
ng statements in WITH")));
+			}
 			/* WITH queries should never be canSetTag */
 			Assert(!ctequery->canSetTag);
+			/* Push the single Query back into the CTE node */
 			cte->ctequery =3D (Node *) ctequery;
 		}
 		else if (newstuff =3D=3D NIL)
diff --git a/src/test/regress/expected/with.out b/src/test/regress/expecte=
d/with.out
index 584bdc6600..3523a7dcc1 100644
--- a/src/test/regress/expected/with.out
+++ b/src/test/regress/expected/with.out
@@ -2969,6 +2969,31 @@ WITH t AS (
 )
 VALUES(FALSE);
 ERROR:  conditional DO INSTEAD rules are not supported for data-modifying=
 statements in WITH
+CREATE OR REPLACE RULE y_rule AS ON INSERT TO y DO INSTEAD NOTHING;
+WITH t AS (
+	INSERT INTO y VALUES(0)
+)
+VALUES(FALSE);
+ERROR:  DO INSTEAD NOTHING rules are not supported for data-modifying sta=
tements in WITH
+CREATE OR REPLACE RULE y_rule AS ON INSERT TO y DO INSTEAD NOTIFY foo;
+WITH t AS (
+	INSERT INTO y VALUES(0)
+)
+VALUES(FALSE);
+ERROR:  DO INSTEAD NOTIFY rules are not supported for data-modifying stat=
ements in WITH
+CREATE OR REPLACE RULE y_rule AS ON INSERT TO y DO ALSO NOTIFY foo;
+WITH t AS (
+	INSERT INTO y VALUES(0)
+)
+VALUES(FALSE);
+ERROR:  DO ALSO rules are not supported for data-modifying statements in =
WITH
+CREATE OR REPLACE RULE y_rule AS ON INSERT TO y
+  DO INSTEAD (NOTIFY foo; NOTIFY bar);
+WITH t AS (
+	INSERT INTO y VALUES(0)
+)
+VALUES(FALSE);
+ERROR:  multi-statement DO INSTEAD rules are not supported for data-modif=
ying statements in WITH
 DROP RULE y_rule ON y;
 -- check that parser lookahead for WITH doesn't cause any odd behavior
 create table foo (with baz);  -- fail, WITH is a reserved word
diff --git a/src/test/regress/sql/with.sql b/src/test/regress/sql/with.sql
index 1a9bdc9f3e..8b213ee408 100644
--- a/src/test/regress/sql/with.sql
+++ b/src/test/regress/sql/with.sql
@@ -1375,6 +1375,27 @@ WITH t AS (
 	INSERT INTO y VALUES(0)
 )
 VALUES(FALSE);
+CREATE OR REPLACE RULE y_rule AS ON INSERT TO y DO INSTEAD NOTHING;
+WITH t AS (
+	INSERT INTO y VALUES(0)
+)
+VALUES(FALSE);
+CREATE OR REPLACE RULE y_rule AS ON INSERT TO y DO INSTEAD NOTIFY foo;
+WITH t AS (
+	INSERT INTO y VALUES(0)
+)
+VALUES(FALSE);
+CREATE OR REPLACE RULE y_rule AS ON INSERT TO y DO ALSO NOTIFY foo;
+WITH t AS (
+	INSERT INTO y VALUES(0)
+)
+VALUES(FALSE);
+CREATE OR REPLACE RULE y_rule AS ON INSERT TO y
+  DO INSTEAD (NOTIFY foo; NOTIFY bar);
+WITH t AS (
+	INSERT INTO y VALUES(0)
+)
+VALUES(FALSE);
 DROP RULE y_rule ON y;
 =

 -- check that parser lookahead for WITH doesn't cause any odd behavior

------- =_aaaaaaaaaa0--