From 1bc9e9826b10bb162625a46c9cb15428637f8408 Mon Sep 17 00:00:00 2001
From: amit <amitlangote09@gmail.com>
Date: Wed, 12 Apr 2017 17:58:07 +0900
Subject: [PATCH 1/3] Fix ALTER TABLE ONLY to avoid unnecessarily failures

Currently, ALTER TABLE ONLY would fail in certain cases when applied
to partitioned tables, even if no partitions yet exist.  Although,
it seems perfectly OK to allow the same, especially because pg_dump
sometimes outputs constraints separately using ALTER TABLE ONLY.
---
 doc/src/sgml/ddl.sgml                     | 10 ------
 src/backend/commands/tablecmds.c          | 56 +++++++++++++++++++------------
 src/test/regress/expected/alter_table.out | 30 +++++++++++------
 src/test/regress/sql/alter_table.sql      | 24 +++++++++----
 4 files changed, 72 insertions(+), 48 deletions(-)

diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml
index 340c961b3f..375debb25e 100644
--- a/doc/src/sgml/ddl.sgml
+++ b/doc/src/sgml/ddl.sgml
@@ -2950,16 +2950,6 @@ VALUES ('Albany', NULL, NULL, 'NY');
 
      <listitem>
       <para>
-       The <literal>ONLY</literal> notation used to exclude child tables
-       will cause an error for partitioned tables in the case of
-       schema-modifying commands such as most <literal>ALTER TABLE</literal>
-       commands.  For example, dropping a column from only the parent does
-       not make sense for partitioned tables.
-      </para>
-     </listitem>
-
-     <listitem>
-      <para>
        Partitions cannot have columns that are not present in the parent.  It
        is neither possible to specify columns when creating partitions with
        <command>CREATE TABLE</> nor is it possible to add columns to
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index a6a9f54b13..f9aba10bb6 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -5579,13 +5579,19 @@ ATPrepDropNotNull(Relation rel, bool recurse, bool recursing)
 {
 	/*
 	 * If the parent is a partitioned table, like check constraints, NOT NULL
-	 * constraints must be dropped from child tables.
+	 * constraints must be dropped from partitions.  Complain if requested
+	 * otherwise and partitions exist.
 	 */
-	if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE &&
-		!recurse && !recursing)
-		ereport(ERROR,
-				(errcode(ERRCODE_INVALID_TABLE_DEFINITION),
-				 errmsg("constraint must be dropped from child tables too")));
+	if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
+	{
+		PartitionDesc	partdesc = RelationGetPartitionDesc(rel);
+
+		Assert(partdesc != NULL);
+		if (partdesc->nparts > 0 && !recurse && !recursing)
+			ereport(ERROR,
+					(errcode(ERRCODE_INVALID_TABLE_DEFINITION),
+					 errmsg("constraint must be dropped from partitions too")));
+	}
 }
 static ObjectAddress
 ATExecDropNotNull(Relation rel, const char *colName, LOCKMODE lockmode)
@@ -5746,13 +5752,18 @@ ATPrepSetNotNull(Relation rel, bool recurse, bool recursing)
 {
 	/*
 	 * If the parent is a partitioned table, like check constraints, NOT NULL
-	 * constraints must be added to the child tables.
+	 * constraints must be added to the child tables.  Complain if requested
+	 * otherwise and partitions exist.
 	 */
-	if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE &&
-		!recurse && !recursing)
-		ereport(ERROR,
-				(errcode(ERRCODE_INVALID_TABLE_DEFINITION),
-				 errmsg("constraint must be added to child tables too")));
+	if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
+	{
+		PartitionDesc	partdesc = RelationGetPartitionDesc(rel);
+
+		if (partdesc && partdesc->nparts > 0 && !recurse && !recursing)
+			ereport(ERROR,
+					(errcode(ERRCODE_INVALID_TABLE_DEFINITION),
+					 errmsg("constraint must be added to partitions too")));
+	}
 }
 
 static ObjectAddress
@@ -8558,16 +8569,6 @@ ATExecDropConstraint(Relation rel, const char *constrName,
 	}
 
 	/*
-	 * In case of a partitioned table, the constraint must be dropped from the
-	 * partitions too.  There is no such thing as NO INHERIT constraints in
-	 * case of partitioned tables.
-	 */
-	if (!recurse && rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
-		ereport(ERROR,
-				(errcode(ERRCODE_INVALID_TABLE_DEFINITION),
-				 errmsg("constraint must be dropped from child tables too")));
-
-	/*
 	 * Propagate to children as appropriate.  Unlike most other ALTER
 	 * routines, we have to do this one level of recursion at a time; we can't
 	 * use find_all_inheritors to do it in one pass.
@@ -8577,6 +8578,17 @@ ATExecDropConstraint(Relation rel, const char *constrName,
 	else
 		children = NIL;
 
+	/*
+	 * For a partitioned table, if partitions exist and we are told not to
+	 * recurse, it's a user error.  It doesn't make sense to have a constraint
+	 * be defined only on the parent, especially if it's a partitioned table.
+	 */
+	if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE &&
+		children != NIL && !recurse)
+		ereport(ERROR,
+				(errcode(ERRCODE_INVALID_TABLE_DEFINITION),
+				 errmsg("constraint must be dropped from partitions too")));
+
 	foreach(child, children)
 	{
 		Oid			childrelid = lfirst_oid(child);
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index 2227f2d977..eff9615653 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -3306,24 +3306,34 @@ ALTER TABLE part_2 RENAME COLUMN b to c;
 ERROR:  cannot rename inherited column "b"
 ALTER TABLE part_2 ALTER COLUMN b TYPE text;
 ERROR:  cannot alter inherited column "b"
--- cannot add NOT NULL or check constraints to *only* the parent (ie, non-inherited)
+-- cannot add/drop NOT NULL or check constraints to *only* the parent, when
+-- partitions exist
 ALTER TABLE ONLY list_parted2 ALTER b SET NOT NULL;
+ERROR:  constraint must be added to partitions too
+ALTER TABLE ONLY list_parted2 ADD CONSTRAINT check_b CHECK (b <> 'zz');
 ERROR:  constraint must be added to child tables too
-ALTER TABLE ONLY list_parted2 add constraint check_b check (b <> 'zz');
-ERROR:  constraint must be added to child tables too
-ALTER TABLE list_parted2 add constraint check_b check (b <> 'zz') NO INHERIT;
-ERROR:  cannot add NO INHERIT constraint to partitioned table "list_parted2"
+ALTER TABLE list_parted2 ALTER b SET NOT NULL;
+ALTER TABLE ONLY list_parted2 ALTER b DROP NOT NULL;
+ERROR:  constraint must be dropped from partitions too
+ALTER TABLE list_parted2 ADD CONSTRAINT check_b CHECK (b <> 'zz');
+ALTER TABLE ONLY list_parted2 DROP CONSTRAINT check_b;
+ERROR:  constraint must be dropped from partitions too
+-- It's alright though, if no partitions are yet created
+CREATE TABLE parted_no_parts (a int) PARTITION BY LIST (a);
+ALTER TABLE ONLY parted_no_parts ALTER a SET NOT NULL;
+ALTER TABLE ONLY parted_no_parts ADD CONSTRAINT check_a CHECK (a > 0);
+ALTER TABLE ONLY parted_no_parts ALTER a DROP NOT NULL;
+ALTER TABLE ONLY parted_no_parts DROP CONSTRAINT check_a;
+DROP TABLE parted_no_parts;
 -- cannot drop inherited NOT NULL or check constraints from partition
 ALTER TABLE list_parted2 ALTER b SET NOT NULL, ADD CONSTRAINT check_a2 CHECK (a > 0);
 ALTER TABLE part_2 ALTER b DROP NOT NULL;
 ERROR:  column "b" is marked NOT NULL in parent table
 ALTER TABLE part_2 DROP CONSTRAINT check_a2;
 ERROR:  cannot drop inherited constraint "check_a2" of relation "part_2"
--- cannot drop NOT NULL or check constraints from *only* the parent
-ALTER TABLE ONLY list_parted2 ALTER a DROP NOT NULL;
-ERROR:  constraint must be dropped from child tables too
-ALTER TABLE ONLY list_parted2 DROP CONSTRAINT check_a2;
-ERROR:  constraint must be dropped from child tables too
+-- Doesn't make sense to add NO INHERIT constraints on partitioned tables
+ALTER TABLE list_parted2 add constraint check_b2 check (b <> 'zz') NO INHERIT;
+ERROR:  cannot add NO INHERIT constraint to partitioned table "list_parted2"
 -- check that a partition cannot participate in regular inheritance
 CREATE TABLE inh_test () INHERITS (part_2);
 ERROR:  cannot inherit from partition "part_2"
diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql
index 8cd6786a90..23e6928b70 100644
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -2173,19 +2173,31 @@ ALTER TABLE part_2 DROP COLUMN b;
 ALTER TABLE part_2 RENAME COLUMN b to c;
 ALTER TABLE part_2 ALTER COLUMN b TYPE text;
 
--- cannot add NOT NULL or check constraints to *only* the parent (ie, non-inherited)
+-- cannot add/drop NOT NULL or check constraints to *only* the parent, when
+-- partitions exist
 ALTER TABLE ONLY list_parted2 ALTER b SET NOT NULL;
-ALTER TABLE ONLY list_parted2 add constraint check_b check (b <> 'zz');
-ALTER TABLE list_parted2 add constraint check_b check (b <> 'zz') NO INHERIT;
+ALTER TABLE ONLY list_parted2 ADD CONSTRAINT check_b CHECK (b <> 'zz');
+
+ALTER TABLE list_parted2 ALTER b SET NOT NULL;
+ALTER TABLE ONLY list_parted2 ALTER b DROP NOT NULL;
+ALTER TABLE list_parted2 ADD CONSTRAINT check_b CHECK (b <> 'zz');
+ALTER TABLE ONLY list_parted2 DROP CONSTRAINT check_b;
+
+-- It's alright though, if no partitions are yet created
+CREATE TABLE parted_no_parts (a int) PARTITION BY LIST (a);
+ALTER TABLE ONLY parted_no_parts ALTER a SET NOT NULL;
+ALTER TABLE ONLY parted_no_parts ADD CONSTRAINT check_a CHECK (a > 0);
+ALTER TABLE ONLY parted_no_parts ALTER a DROP NOT NULL;
+ALTER TABLE ONLY parted_no_parts DROP CONSTRAINT check_a;
+DROP TABLE parted_no_parts;
 
 -- cannot drop inherited NOT NULL or check constraints from partition
 ALTER TABLE list_parted2 ALTER b SET NOT NULL, ADD CONSTRAINT check_a2 CHECK (a > 0);
 ALTER TABLE part_2 ALTER b DROP NOT NULL;
 ALTER TABLE part_2 DROP CONSTRAINT check_a2;
 
--- cannot drop NOT NULL or check constraints from *only* the parent
-ALTER TABLE ONLY list_parted2 ALTER a DROP NOT NULL;
-ALTER TABLE ONLY list_parted2 DROP CONSTRAINT check_a2;
+-- Doesn't make sense to add NO INHERIT constraints on partitioned tables
+ALTER TABLE list_parted2 add constraint check_b2 check (b <> 'zz') NO INHERIT;
 
 -- check that a partition cannot participate in regular inheritance
 CREATE TABLE inh_test () INHERITS (part_2);
-- 
2.11.0

