Thread: BUG #18664: Assert in BeginCopyTo() fails when source DML query rewritten with notifying rule

The following bug has been logged on the website:

Bug reference:      18664
Logged by:          Alexander Lakhin
Email address:      exclusion@gmail.com
PostgreSQL version: 17.0
Operating system:   Ubuntu 22.04
Description:

The following script:
CREATE TABLE t(i int);
CREATE RULE r AS ON INSERT TO t DO INSTEAD NOTIFY c;
COPY (INSERT INTO t VALUES (1)) TO stdout;

triggers an Assert in BeginCopyTo():
TRAP: failed Assert("query->utilityStmt == NULL"), File: "copyto.c", Line:
495, PID: 1689825
ExceptionalCondition at assert.c:52:13
BeginCopyTo at copyto.c:501:12
DoCopy at copy.c:313:12
standard_ProcessUtility at utility.c:738:8
ProcessUtility at utility.c:523:3
PortalRunUtility at pquery.c:1158:2
PortalRunMulti at pquery.c:1315:5
PortalRun at pquery.c:795:5
...

Reproduced starting from 92e38182d.

Tender Wang <tndrwang@gmail.com> writes:
> PG Bug reporting form <noreply@postgresql.org> 于2024年10月21日周一 19:32写道：
>> The following script:
>> CREATE TABLE t(i int);
>> CREATE RULE r AS ON INSERT TO t DO INSTEAD NOTIFY c;
>> COPY (INSERT INTO t VALUES (1)) TO stdout;
>>
>> triggers an Assert in BeginCopyTo():

> I take a quick look.  It seems 92e38182d ignored this case.
> I gave a fix that remove the Assert and add an  Assert in another if blocks.
> Any thoughts?

I don't like this fix, because the error message is just completely
misleading for this case.  "COPY query must have a RETURNING clause"
makes it look like your mistake was to not write

COPY (INSERT INTO t VALUES (1) RETURNING *) TO stdout;

Of course doing that will fix nothing, and you'd still get the exact
same error message, leaving the user quite confused.  So I think
it's worth issuing a more specific message.

I also noted somebody's faulty grammar in the nearby message
"DO ALSO rules are not supported for the COPY".

So I think we want the attached.  I went back and forth about
whether to make the new message be specifically "COPY query must
not be NOTIFY", but decided that we'd surely forget to update it
if we ever allow any other sort of utility command in rules.
So better to word it generically.

            regards, tom lane

diff --git a/src/backend/commands/copyto.c b/src/backend/commands/copyto.c
index 463083e645..f55e6d9675 100644
--- a/src/backend/commands/copyto.c
+++ b/src/backend/commands/copyto.c
@@ -475,7 +475,7 @@ BeginCopyTo(ParseState *pstate,
                 if (q->querySource == QSRC_NON_INSTEAD_RULE)
                     ereport(ERROR,
                             (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-                             errmsg("DO ALSO rules are not supported for the COPY")));
+                             errmsg("DO ALSO rules are not supported for COPY")));
             }

             ereport(ERROR,
@@ -492,7 +492,11 @@ BeginCopyTo(ParseState *pstate,
                     (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
                      errmsg("COPY (SELECT INTO) is not supported")));

-        Assert(query->utilityStmt == NULL);
+        /* The only other utility command we could see is NOTIFY */
+        if (query->utilityStmt != NULL)
+            ereport(ERROR,
+                    (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+                     errmsg("COPY query must not be a utility command")));

         /*
          * Similarly the grammar doesn't enforce the presence of a RETURNING
diff --git a/src/test/regress/expected/copydml.out b/src/test/regress/expected/copydml.out
index b5a225628f..e91e83260a 100644
--- a/src/test/regress/expected/copydml.out
+++ b/src/test/regress/expected/copydml.out
@@ -38,7 +38,7 @@ ERROR:  DO INSTEAD NOTHING rules are not supported for COPY
 drop rule qqq on copydml_test;
 create rule qqq as on insert to copydml_test do also delete from copydml_test;
 copy (insert into copydml_test default values) to stdout;
-ERROR:  DO ALSO rules are not supported for the COPY
+ERROR:  DO ALSO rules are not supported for COPY
 drop rule qqq on copydml_test;
 create rule qqq as on insert to copydml_test do instead (delete from copydml_test; delete from copydml_test);
 copy (insert into copydml_test default values) to stdout;
@@ -54,7 +54,7 @@ ERROR:  DO INSTEAD NOTHING rules are not supported for COPY
 drop rule qqq on copydml_test;
 create rule qqq as on update to copydml_test do also delete from copydml_test;
 copy (update copydml_test set t = 'f') to stdout;
-ERROR:  DO ALSO rules are not supported for the COPY
+ERROR:  DO ALSO rules are not supported for COPY
 drop rule qqq on copydml_test;
 create rule qqq as on update to copydml_test do instead (delete from copydml_test; delete from copydml_test);
 copy (update copydml_test set t = 'f') to stdout;
@@ -70,7 +70,7 @@ ERROR:  DO INSTEAD NOTHING rules are not supported for COPY
 drop rule qqq on copydml_test;
 create rule qqq as on delete to copydml_test do also insert into copydml_test default values;
 copy (delete from copydml_test) to stdout;
-ERROR:  DO ALSO rules are not supported for the COPY
+ERROR:  DO ALSO rules are not supported for COPY
 drop rule qqq on copydml_test;
 create rule qqq as on delete to copydml_test do instead (insert into copydml_test default values; insert into
copydml_testdefault values); 
 copy (delete from copydml_test) to stdout;
@@ -80,6 +80,10 @@ create rule qqq as on delete to copydml_test where old.t <> 'f' do instead inser
 copy (delete from copydml_test) to stdout;
 ERROR:  conditional DO INSTEAD rules are not supported for COPY
 drop rule qqq on copydml_test;
+create rule qqq as on insert to copydml_test do instead notify copydml_test;
+copy (insert into copydml_test default values) to stdout;
+ERROR:  COPY query must not be a utility command
+drop rule qqq on copydml_test;
 -- triggers
 create function qqq_trig() returns trigger as $$
 begin
diff --git a/src/test/regress/sql/copydml.sql b/src/test/regress/sql/copydml.sql
index 4578342253..b7eeb0eed8 100644
--- a/src/test/regress/sql/copydml.sql
+++ b/src/test/regress/sql/copydml.sql
@@ -66,6 +66,10 @@ create rule qqq as on delete to copydml_test where old.t <> 'f' do instead inser
 copy (delete from copydml_test) to stdout;
 drop rule qqq on copydml_test;

+create rule qqq as on insert to copydml_test do instead notify copydml_test;
+copy (insert into copydml_test default values) to stdout;
+drop rule qqq on copydml_test;
+
 -- triggers
 create function qqq_trig() returns trigger as $$
 begin