Better handle indirect constraint drops

It is possible for certain cases to remove not-null constraints without
maintaining the attnotnull in its correct state; for example if you drop
a column that's part of the primary key, and the other columns of the PK don't
have not-null constraints, then we should reset the attnotnull flags for
those other columns; up to this commit, we didn't.  Handle those cases
better by doing the attnotnull reset in RemoveConstraintById() instead
of in dropconstraint_internal().

However, there are some cases where we must not do so.  For example if
those other columns are in replica identity indexes or are generated
identity columns, we must keep attnotnull set, even though it results in
the catalog inconsistency that no not-null constraint supports that.

Because the attnotnull reset now happens in more places than before, for
instance when a column of the primary key changes type, we need an
additional trick to reinstate it as necessary.  Introduce a new
alter-table pass that does this, which needs simply reschedule some
AT_SetAttNotNull subcommands that were already being generated and
ignored.

Because of the exceptions in which attnotnull is not reset noted above,
we also include a pg_dump hack to include a not-null constraint when the
attnotnull flag is set even if no pg_constraint row exists.  This part
is undesirable but necessary, because failing to handle the case can
result in unrestorable dumps.

Reported-by: Tender Wang <tndrwang@gmail.com>
Co-authored-by: Tender Wang <tndrwang@gmail.com>
Reviewed-by: jian he <jian.universality@gmail.com>
Discussion: https://postgr.es/m/CAHewXN=hMbNa3d43NOR=OCgdgpTt18S-1fmueCoEGesyeK4bqw@mail.gmail.com

2 files changed, 117 insertions, 0 deletions

diff --git a/src/test/regress/expected/constraints.out b/src/test/regress/expected/constraints.out
index d50dd1f61ab..2fc0be7925b 100644
--- a/src/test/regress/expected/constraints.out
+++ b/src/test/regress/expected/constraints.out
@@ -926,9 +926,87 @@ DROP TABLE notnull_tbl1;
 -- nope
 CREATE TABLE notnull_tbl2 (a INTEGER CONSTRAINT blah NOT NULL, b INTEGER CONSTRAINT blah NOT NULL);
 ERROR:  constraint "blah" for relation "notnull_tbl2" already exists
+-- can't drop not-null in primary key
 CREATE TABLE notnull_tbl2 (a INTEGER PRIMARY KEY);
 ALTER TABLE notnull_tbl2 ALTER a DROP NOT NULL;
 ERROR:  column "a" is in a primary key
+DROP TABLE notnull_tbl2;
+-- make sure attnotnull is reset correctly when a PK is dropped indirectly,
+-- or kept if there's a reason for that
+CREATE TABLE notnull_tbl1 (c0 int, c1 int, PRIMARY KEY (c0, c1));
+ALTER TABLE  notnull_tbl1 DROP c1;
+\d+ notnull_tbl1
+                               Table "public.notnull_tbl1"
+ Column |  Type   | Collation | Nullable | Default | Storage | Stats target | Description 
+--------+---------+-----------+----------+---------+---------+--------------+-------------
+ c0     | integer |           |          |         | plain   |              | 
+
+DROP TABLE notnull_tbl1;
+-- same, via dropping a domain
+CREATE DOMAIN notnull_dom1 AS INTEGER;
+CREATE TABLE notnull_tbl1 (c0 notnull_dom1, c1 int, PRIMARY KEY (c0, c1));
+DROP DOMAIN notnull_dom1 CASCADE;
+NOTICE:  drop cascades to column c0 of table notnull_tbl1
+\d+ notnull_tbl1
+                               Table "public.notnull_tbl1"
+ Column |  Type   | Collation | Nullable | Default | Storage | Stats target | Description 
+--------+---------+-----------+----------+---------+---------+--------------+-------------
+ c1     | integer |           |          |         | plain   |              | 
+
+DROP TABLE notnull_tbl1;
+-- with a REPLICA IDENTITY column.  Here the not-nulls must be kept
+CREATE DOMAIN notnull_dom1 AS INTEGER;
+CREATE TABLE notnull_tbl1 (c0 notnull_dom1, c1 int UNIQUE, c2 int generated by default as identity, PRIMARY KEY (c0, c1, c2));
+ALTER TABLE notnull_tbl1 DROP CONSTRAINT notnull_tbl1_c2_not_null;
+ALTER TABLE notnull_tbl1 REPLICA IDENTITY USING INDEX notnull_tbl1_c1_key;
+DROP DOMAIN notnull_dom1 CASCADE;
+NOTICE:  drop cascades to column c0 of table notnull_tbl1
+ALTER TABLE  notnull_tbl1 ALTER c1 DROP NOT NULL;	-- can't be dropped
+ERROR:  column "c1" is in index used as replica identity
+ALTER TABLE  notnull_tbl1 ALTER c1 SET NOT NULL;	-- can be set right
+\d+ notnull_tbl1
+                                            Table "public.notnull_tbl1"
+ Column |  Type   | Collation | Nullable |             Default              | Storage | Stats target | Description 
+--------+---------+-----------+----------+----------------------------------+---------+--------------+-------------
+ c1     | integer |           | not null |                                  | plain   |              | 
+ c2     | integer |           | not null | generated by default as identity | plain   |              | 
+Indexes:
+    "notnull_tbl1_c1_key" UNIQUE CONSTRAINT, btree (c1) REPLICA IDENTITY
+Not-null constraints:
+    "notnull_tbl1_c1_not_null" NOT NULL "c1"
+
+DROP TABLE notnull_tbl1;
+CREATE DOMAIN notnull_dom2 AS INTEGER;
+CREATE TABLE notnull_tbl2 (c0 notnull_dom2, c1 int UNIQUE, c2 int generated by default as identity, PRIMARY KEY (c0, c1, c2));
+ALTER TABLE notnull_tbl2 DROP CONSTRAINT notnull_tbl2_c2_not_null;
+ALTER TABLE notnull_tbl2 REPLICA IDENTITY USING INDEX notnull_tbl2_c1_key;
+DROP DOMAIN notnull_dom2 CASCADE;
+NOTICE:  drop cascades to column c0 of table notnull_tbl2
+\d+ notnull_tbl2
+                                            Table "public.notnull_tbl2"
+ Column |  Type   | Collation | Nullable |             Default              | Storage | Stats target | Description 
+--------+---------+-----------+----------+----------------------------------+---------+--------------+-------------
+ c1     | integer |           | not null |                                  | plain   |              | 
+ c2     | integer |           | not null | generated by default as identity | plain   |              | 
+Indexes:
+    "notnull_tbl2_c1_key" UNIQUE CONSTRAINT, btree (c1) REPLICA IDENTITY
+
+BEGIN;
+/* make sure the table can be put right, but roll that back */
+ALTER TABLE notnull_tbl2 REPLICA IDENTITY FULL, ALTER c2 DROP IDENTITY;
+ALTER TABLE notnull_tbl2 ALTER c1 DROP NOT NULL, ALTER c2 DROP NOT NULL;
+\d+ notnull_tbl2
+                               Table "public.notnull_tbl2"
+ Column |  Type   | Collation | Nullable | Default | Storage | Stats target | Description 
+--------+---------+-----------+----------+---------+---------+--------------+-------------
+ c1     | integer |           |          |         | plain   |              | 
+ c2     | integer |           |          |         | plain   |              | 
+Indexes:
+    "notnull_tbl2_c1_key" UNIQUE CONSTRAINT, btree (c1)
+Replica Identity: FULL
+
+ROLLBACK;
+-- Leave this table around for pg_upgrade testing
 CREATE TABLE notnull_tbl3 (a INTEGER NOT NULL, CHECK (a IS NOT NULL));
 ALTER TABLE notnull_tbl3 ALTER A DROP NOT NULL;
 ALTER TABLE notnull_tbl3 ADD b int, ADD CONSTRAINT pk PRIMARY KEY (a, b);
diff --git a/src/test/regress/sql/constraints.sql b/src/test/regress/sql/constraints.sql
index 7a39b504a31..8f85e72050f 100644
--- a/src/test/regress/sql/constraints.sql
+++ b/src/test/regress/sql/constraints.sql
@@ -624,8 +624,47 @@ DROP TABLE notnull_tbl1;
 -- nope
 CREATE TABLE notnull_tbl2 (a INTEGER CONSTRAINT blah NOT NULL, b INTEGER CONSTRAINT blah NOT NULL);
 
+-- can't drop not-null in primary key
 CREATE TABLE notnull_tbl2 (a INTEGER PRIMARY KEY);
 ALTER TABLE notnull_tbl2 ALTER a DROP NOT NULL;
+DROP TABLE notnull_tbl2;
+
+-- make sure attnotnull is reset correctly when a PK is dropped indirectly,
+-- or kept if there's a reason for that
+CREATE TABLE notnull_tbl1 (c0 int, c1 int, PRIMARY KEY (c0, c1));
+ALTER TABLE  notnull_tbl1 DROP c1;
+\d+ notnull_tbl1
+DROP TABLE notnull_tbl1;
+-- same, via dropping a domain
+CREATE DOMAIN notnull_dom1 AS INTEGER;
+CREATE TABLE notnull_tbl1 (c0 notnull_dom1, c1 int, PRIMARY KEY (c0, c1));
+DROP DOMAIN notnull_dom1 CASCADE;
+\d+ notnull_tbl1
+DROP TABLE notnull_tbl1;
+-- with a REPLICA IDENTITY column.  Here the not-nulls must be kept
+CREATE DOMAIN notnull_dom1 AS INTEGER;
+CREATE TABLE notnull_tbl1 (c0 notnull_dom1, c1 int UNIQUE, c2 int generated by default as identity, PRIMARY KEY (c0, c1, c2));
+ALTER TABLE notnull_tbl1 DROP CONSTRAINT notnull_tbl1_c2_not_null;
+ALTER TABLE notnull_tbl1 REPLICA IDENTITY USING INDEX notnull_tbl1_c1_key;
+DROP DOMAIN notnull_dom1 CASCADE;
+ALTER TABLE  notnull_tbl1 ALTER c1 DROP NOT NULL;	-- can't be dropped
+ALTER TABLE  notnull_tbl1 ALTER c1 SET NOT NULL;	-- can be set right
+\d+ notnull_tbl1
+DROP TABLE notnull_tbl1;
+
+CREATE DOMAIN notnull_dom2 AS INTEGER;
+CREATE TABLE notnull_tbl2 (c0 notnull_dom2, c1 int UNIQUE, c2 int generated by default as identity, PRIMARY KEY (c0, c1, c2));
+ALTER TABLE notnull_tbl2 DROP CONSTRAINT notnull_tbl2_c2_not_null;
+ALTER TABLE notnull_tbl2 REPLICA IDENTITY USING INDEX notnull_tbl2_c1_key;
+DROP DOMAIN notnull_dom2 CASCADE;
+\d+ notnull_tbl2
+BEGIN;
+/* make sure the table can be put right, but roll that back */
+ALTER TABLE notnull_tbl2 REPLICA IDENTITY FULL, ALTER c2 DROP IDENTITY;
+ALTER TABLE notnull_tbl2 ALTER c1 DROP NOT NULL, ALTER c2 DROP NOT NULL;
+\d+ notnull_tbl2
+ROLLBACK;
+-- Leave this table around for pg_upgrade testing
 
 CREATE TABLE notnull_tbl3 (a INTEGER NOT NULL, CHECK (a IS NOT NULL));
 ALTER TABLE notnull_tbl3 ALTER A DROP NOT NULL;