Feedback-ID: ia2694551:Fastmail
Date: Tue, 4 Mar 2025 19:22:22 +0100
From: =?utf-8?Q?=C3=81lvaro?= Herrera <alvherre@alvh.no-ip.org>
To: Pg Hackers <pgsql-hackers@lists.postgresql.org>
Subject: bogus error message for ALTER TABLE ALTER CONSTRAINT
Message-ID: <202503041822.aobpqke3igvb@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="vvywzef3pmz5robm"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
Archived-At: <https://www.postgresql.org/message-id/202503041822.aobpqke3igvb%40alvherre.pgsql>
Precedence: bulk


--vvywzef3pmz5robm
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit

I just discovered that trying to set a foreign key as NO INHERIT in
ALTER TABLE ALTER CONSTRAINT returns an absurd error message:

create table pk (a int primary key);
create table fk (a int references pk);

alter table fk alter constraint fk_a_fkey deferrable, alter constraint fk_a_fkey no inherit;
ERROR:  ALTER CONSTRAINT statement constraints cannot be marked NO INHERIT

The explanation is that somebody misunderstood what must be given to
processCASbits in 2013.  The intended message is:
  ERROR:  FOREIGN KEY constraints cannot be marked NO INHERIT

Here's the fix along with some additional cleanup.

-- 
Álvaro Herrera        Breisgau, Deutschland  —  https://www.EnterpriseDB.com/

--vvywzef3pmz5robm
Content-Type: text/x-diff; charset=utf-8
Content-Disposition: attachment;
	filename="0001-Fix-ALTER-TABLE-error-message.patch"

From ba95124d06e337c285a31b0376230702e6b1fc3d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=81lvaro=20Herrera?= <alvherre@alvh.no-ip.org>
Date: Tue, 4 Mar 2025 18:48:14 +0100
Subject: [PATCH] Fix ALTER TABLE error message

This bogus error message was introduced in 2013 by commit f177cbfe676d,
because of misunderstanding the processCASbits() API; at the time, no
test cases were added that would be affected by this change.  Only in
ca87c415e2fc was one added (along with a couple of typos), with an XXX
note that the error message was bogus.  Fix the whole, add some test
cases.

Backpatch all the way back.
---
 src/backend/parser/gram.y                 |  2 +-
 src/test/regress/expected/constraints.out |  7 +++----
 src/test/regress/expected/foreign_key.out | 10 +++++++++-
 src/test/regress/sql/constraints.sql      |  3 +--
 src/test/regress/sql/foreign_key.sql      |  4 +++-
 5 files changed, 17 insertions(+), 9 deletions(-)

diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 7d99c9355c6..c11a3beff06 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -2663,7 +2663,7 @@ alter_table_cmd:
 					n->def = (Node *) c;
 					c->conname = $3;
 					c->alterDeferrability = true;
-					processCASbits($4, @4, "ALTER CONSTRAINT statement",
+					processCASbits($4, @4, "FOREIGN KEY",
 									&c->deferrable,
 									&c->initdeferred,
 									NULL, NULL, NULL, yyscanner);
diff --git a/src/test/regress/expected/constraints.out b/src/test/regress/expected/constraints.out
index 692a69fe457..4f39100fcdf 100644
--- a/src/test/regress/expected/constraints.out
+++ b/src/test/regress/expected/constraints.out
@@ -735,7 +735,7 @@ SELECT * FROM unique_tbl;
  3 | threex
 (5 rows)
 
--- enforcibility cannot be specified or set for unique constrain
+-- enforceability cannot be specified or set for unique constraint
 CREATE TABLE UNIQUE_EN_TBL(i int UNIQUE ENFORCED);
 ERROR:  misplaced ENFORCED clause
 LINE 1: CREATE TABLE UNIQUE_EN_TBL(i int UNIQUE ENFORCED);
@@ -744,13 +744,12 @@ CREATE TABLE UNIQUE_NOTEN_TBL(i int UNIQUE NOT ENFORCED);
 ERROR:  misplaced NOT ENFORCED clause
 LINE 1: CREATE TABLE UNIQUE_NOTEN_TBL(i int UNIQUE NOT ENFORCED);
                                                    ^
--- XXX: error message is misleading here
 ALTER TABLE unique_tbl ALTER CONSTRAINT unique_tbl_i_key ENFORCED;
-ERROR:  ALTER CONSTRAINT statement constraints cannot be marked ENFORCED
+ERROR:  FOREIGN KEY constraints cannot be marked ENFORCED
 LINE 1: ...TABLE unique_tbl ALTER CONSTRAINT unique_tbl_i_key ENFORCED;
                                                               ^
 ALTER TABLE unique_tbl ALTER CONSTRAINT unique_tbl_i_key NOT ENFORCED;
-ERROR:  ALTER CONSTRAINT statement constraints cannot be marked NOT ENFORCED
+ERROR:  FOREIGN KEY constraints cannot be marked NOT ENFORCED
 LINE 1: ...ABLE unique_tbl ALTER CONSTRAINT unique_tbl_i_key NOT ENFORC...
                                                              ^
 DROP TABLE unique_tbl;
diff --git a/src/test/regress/expected/foreign_key.out b/src/test/regress/expected/foreign_key.out
index 374dcb266e7..6a3374d5152 100644
--- a/src/test/regress/expected/foreign_key.out
+++ b/src/test/regress/expected/foreign_key.out
@@ -1278,11 +1278,19 @@ DETAIL:  Key (fk)=(20) is not present in table "pktable".
 COMMIT;
 -- try additional syntax
 ALTER TABLE fktable ALTER CONSTRAINT fktable_fk_fkey NOT DEFERRABLE;
--- illegal option
+-- illegal options
 ALTER TABLE fktable ALTER CONSTRAINT fktable_fk_fkey NOT DEFERRABLE INITIALLY DEFERRED;
 ERROR:  constraint declared INITIALLY DEFERRED must be DEFERRABLE
 LINE 1: ...e ALTER CONSTRAINT fktable_fk_fkey NOT DEFERRABLE INITIALLY ...
                                                              ^
+ALTER TABLE fktable ALTER CONSTRAINT fktable_fk_fkey NO INHERIT;
+ERROR:  FOREIGN KEY constraints cannot be marked NO INHERIT
+LINE 1: ...ER TABLE fktable ALTER CONSTRAINT fktable_fk_fkey NO INHERIT...
+                                                             ^
+ALTER TABLE fktable ALTER CONSTRAINT fktable_fk_fkey NOT VALID;
+ERROR:  FOREIGN KEY constraints cannot be marked NOT VALID
+LINE 1: ...ER TABLE fktable ALTER CONSTRAINT fktable_fk_fkey NOT VALID;
+                                                             ^
 -- test order of firing of FK triggers when several RI-induced changes need to
 -- be made to the same row.  This was broken by subtransaction-related
 -- changes in 8.0.
diff --git a/src/test/regress/sql/constraints.sql b/src/test/regress/sql/constraints.sql
index d6742f83fb9..21ce4177de4 100644
--- a/src/test/regress/sql/constraints.sql
+++ b/src/test/regress/sql/constraints.sql
@@ -531,10 +531,9 @@ COMMIT;
 
 SELECT * FROM unique_tbl;
 
--- enforcibility cannot be specified or set for unique constrain
+-- enforceability cannot be specified or set for unique constraint
 CREATE TABLE UNIQUE_EN_TBL(i int UNIQUE ENFORCED);
 CREATE TABLE UNIQUE_NOTEN_TBL(i int UNIQUE NOT ENFORCED);
--- XXX: error message is misleading here
 ALTER TABLE unique_tbl ALTER CONSTRAINT unique_tbl_i_key ENFORCED;
 ALTER TABLE unique_tbl ALTER CONSTRAINT unique_tbl_i_key NOT ENFORCED;
 
diff --git a/src/test/regress/sql/foreign_key.sql b/src/test/regress/sql/foreign_key.sql
index bc0adb8cfe9..44945b0453a 100644
--- a/src/test/regress/sql/foreign_key.sql
+++ b/src/test/regress/sql/foreign_key.sql
@@ -970,8 +970,10 @@ COMMIT;
 
 -- try additional syntax
 ALTER TABLE fktable ALTER CONSTRAINT fktable_fk_fkey NOT DEFERRABLE;
--- illegal option
+-- illegal options
 ALTER TABLE fktable ALTER CONSTRAINT fktable_fk_fkey NOT DEFERRABLE INITIALLY DEFERRED;
+ALTER TABLE fktable ALTER CONSTRAINT fktable_fk_fkey NO INHERIT;
+ALTER TABLE fktable ALTER CONSTRAINT fktable_fk_fkey NOT VALID;
 
 -- test order of firing of FK triggers when several RI-induced changes need to
 -- be made to the same row.  This was broken by subtransaction-related
-- 
2.39.5


--vvywzef3pmz5robm--