Обсуждение: BUG #17094: FailedAssertion at planner.c

The following bug has been logged on the website:

Bug reference:      17094
Logged by:          yaoguang chen
Email address:      cyg0810@gmail.com
PostgreSQL version: 14beta1
Operating system:   Linux supersix 5.4.0-39-generic #43-Ubuntu SMP Fri
Description:

run the following sql command through client and the PostgreSQL database
process will crash:

CREATE TABLE v0 ( v4 INT , v3 INT UNIQUE , v2 INT , v1 INT UNIQUE ) ; 
 CREATE OR REPLACE RULE v1 AS ON INSERT TO v0 DO INSTEAD NOTIFY COMPRESSION
;
 COPY ( SELECT 32 EXCEPT SELECT v3 + 16 FROM v0 ) TO STDOUT CSV HEADER ;
 WITH v2 AS ( INSERT INTO v0 VALUES ( 0 ) ) DELETE FROM v0 WHERE v3 = - - -
- 48 ;


asan report:

AddressSanitizer:DEADLYSIGNAL=================================================================
==453870==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000008
(pc 0x00000163d9d5 bp 0x7fff5a35ce50 sp 0x7fff5a35ce20 T0)
==453870==The signal is caused by a READ memory access.
==453870==Hint: address points to the zero page.
    #0 0x163d9d4 in replace_empty_jointree
/home/supersix/fuzz/security/PostgreSQL/postgres/build/../src/backend/optimizer/prep/prepjointree.c:157:23
    #1 0x15adbaa in subquery_planner
/home/supersix/fuzz/security/PostgreSQL/postgres/build/../src/backend/optimizer/plan/planner.c:650:2
    #2 0x1620b06 in SS_process_ctes
/home/supersix/fuzz/security/PostgreSQL/postgres/build/../src/backend/optimizer/plan/subselect.c:982:13
    #3 0x15adb5a in subquery_planner
/home/supersix/fuzz/security/PostgreSQL/postgres/build/../src/backend/optimizer/plan/planner.c:644:3
    #4 0x15aa555 in standard_planner
/home/supersix/fuzz/security/PostgreSQL/postgres/build/../src/backend/optimizer/plan/planner.c:400:9
    #5 0x15aa03d in planner
/home/supersix/fuzz/security/PostgreSQL/postgres/build/../src/backend/optimizer/plan/planner.c:271:12
    #6 0x1c6113c in pg_plan_query
/home/supersix/fuzz/security/PostgreSQL/postgres/build/../src/backend/tcop/postgres.c:847:9
    #7 0x1c6113c in pg_plan_queries
/home/supersix/fuzz/security/PostgreSQL/postgres/build/../src/backend/tcop/postgres.c:939:11
    #8 0x1c7ad9b in exec_simple_query
/home/supersix/fuzz/security/PostgreSQL/postgres/build/../src/backend/tcop/postgres.c:1133:19
    #9 0x1c6bb67 in PostgresMain
/home/supersix/fuzz/security/PostgreSQL/postgres/build/../src/backend/tcop/postgres.c
   #10 0x17ff0ba in BackendRun
/home/supersix/fuzz/security/PostgreSQL/postgres/build/../src/backend/postmaster/postmaster.c:4507:2
    #11 0x17fb72f in BackendStartup
/home/supersix/fuzz/security/PostgreSQL/postgres/build/../src/backend/postmaster/postmaster.c:4229:3
    #12 0x17fb72f in ServerLoop
/home/supersix/fuzz/security/PostgreSQL/postgres/build/../src/backend/postmaster/postmaster.c:1745:7
    #13 0x17e616c in PostmasterMain
/home/supersix/fuzz/security/PostgreSQL/postgres/build/../src/backend/postmaster/postmaster.c:1417:11
    #14 0x131bac5 in main
/home/supersix/fuzz/security/PostgreSQL/postgres/build/../src/backend/main/main.c:209:3
    #15 0x7f7f004ef0b2 in __libc_start_main
/build/glibc-ZN95T4/glibc-2.31/csu/../csu/libc-start.c:308:16
    #16 0x4aec2d in _start
(/home/supersix/fuzz/security/PostgreSQL/install/bin/postgres+0x4aec2d)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV
/home/supersix/fuzz/security/PostgreSQL/postgres/build/../src/backend/optimizer/prep/prepjointree.c:157:23
in replace_empty_jointree
==453870==ABORTING

log detail:

TRAP: FailedAssertion("!parse->rowMarks && parse->commandType ==
CMD_SELECT", File:
"/home/supersix/fuzz/security/PostgreSQL/postgres/build/../src/backend/optimizer/plan/planner.c",
Line: 1868, PID: 4042079)postgres: supersix x 127.0.0.1(12402)
DELETE(ExceptionalCondition+0xbb)[0x55cb69dbdffb]
postgres: supersix x 127.0.0.1(12402) DELETE(+0x59480f)[0x55cb699c880f]
postgres: supersix x 127.0.0.1(12402)
DELETE(subquery_planner+0xf63)[0x55cb699c98e3]
postgres: supersix x 127.0.0.1(12402)
DELETE(SS_process_ctes+0xb9)[0x55cb699d6b39]
postgres: supersix x 127.0.0.1(12402)
DELETE(subquery_planner+0x1f9)[0x55cb699c8b79]
postgres: supersix x 127.0.0.1(12402)
DELETE(standard_planner+0x165)[0x55cb699ca535]
postgres: supersix x 127.0.0.1(12402)
DELETE(pg_plan_query+0x6a)[0x55cb69b67eaa]
postgres: supersix x 127.0.0.1(12402)
DELETE(pg_plan_queries+0x4d)[0x55cb69b67ffd]
postgres: supersix x 127.0.0.1(12402) DELETE(+0x7359f2)[0x55cb69b699f2]
postgres: supersix x 127.0.0.1(12402)
DELETE(PostgresMain+0x1ae7)[0x55cb69b6bd57]
postgres: supersix x 127.0.0.1(12402) DELETE(+0x61671f)[0x55cb69a4a71f]
postgres: supersix x 127.0.0.1(12402)
DELETE(PostmasterMain+0x1182)[0x55cb69a4d672]
postgres: supersix x 127.0.0.1(12402) DELETE(main+0x533)[0x55cb694fd133]
/lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xf3)[0x7fdbbf97d0b3]
postgres: supersix x 127.0.0.1(12402) DELETE(_start+0x2e)[0x55cb694fd28e]

Richard Guo <guofenglinux@gmail.com> writes:
> On Thu, Jul 8, 2021 at 5:27 PM PG Bug reporting form <noreply@postgresql.org>
> wrote:
>> CREATE TABLE v0 ( v4 INT , v3 INT UNIQUE , v2 INT , v1 INT UNIQUE ) ;
>> CREATE OR REPLACE RULE v1 AS ON INSERT TO v0 DO INSTEAD NOTIFY COMPRESSION
>> ;
>> WITH v2 AS ( INSERT INTO v0 VALUES ( 0 ) ) DELETE FROM v0 WHERE v3 = - - -
>> - 48 ;

> Seems the Query from RewriteRule->actions may have a NULL jointree, and
> that triggers the SEGV in replace_empty_jointree().

I'm inclined to think we have to reject cases like this.  In HEAD,
replace_empty_jointree is the first thing to crash, but if you
try it in back branches it crashes someplace else.  Even if we
patched all the planner issues, there's no way for the executor
to handle this, because it has no provisions for a utility statement
as part of a plan tree.

Conceivably the NOTIFY could be removed from the query altogether and
then executed as a separate top-level command, but I'm not sure that
would have desirable semantics.

We do already reject some related cases:

regression=# WITH v2 AS ( INSERT INTO v0 VALUES ( 0 ) ) SELECT * FROM v2;
ERROR:  WITH query "v2" does not have a RETURNING clause
LINE 1: WITH v2 AS ( INSERT INTO v0 VALUES ( 0 ) ) SELECT * FROM v2;
                                                                 ^
regression=# WITH v2 AS ( INSERT INTO v0 VALUES ( 0 ) returning * ) SELECT * FROM v2;
ERROR:  cannot perform INSERT RETURNING on relation "v0"
HINT:  You need an unconditional ON INSERT DO INSTEAD rule with a RETURNING clause.

Also, if you make it an ALSO not INSTEAD rule:

regression=# CREATE OR REPLACE RULE v1 AS ON INSERT TO v0 DO also NOTIFY COMPRESSION;
CREATE RULE
regression=# WITH v2 AS ( INSERT INTO v0 VALUES ( 0 ) ) DELETE FROM v0 WHERE v3 = - - -
regression-# 48;
ERROR:  DO ALSO rules are not supported for data-modifying statements in WITH

So it feels to me like this is just a missed case.  We don't really
support any cases involving a rule-added NOTIFY in WITH.

            regards, tom lane

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

diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.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_events)

         /*
          * 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) == 1)
         {
-            /* Push the single Query back into the CTE node */
+            /* Must check it's not a utility command */
             ctequery = linitial_node(Query, newstuff);
+            if (!(ctequery->commandType == CMD_SELECT ||
+                  ctequery->commandType == CMD_UPDATE ||
+                  ctequery->commandType == CMD_INSERT ||
+                  ctequery->commandType == 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-modifying statements in WITH")));
+            }
             /* WITH queries should never be canSetTag */
             Assert(!ctequery->canSetTag);
+            /* Push the single Query back into the CTE node */
             cte->ctequery = (Node *) ctequery;
         }
         else if (newstuff == NIL)
diff --git a/src/test/regress/expected/with.out b/src/test/regress/expected/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 statements 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 statements 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-modifying 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

Richard Guo <guofenglinux@gmail.com> writes:
> On Fri, Jul 9, 2021 at 1:22 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> I'm inclined to think we have to reject cases like this.

> It makes sense to me we reject this case. Tested the patch and it works
> as expected.

Pushed, thanks for looking at it!

            regards, tom lane