Subject: [PATCH] Fix few places still scanning partitioned tables

From: amit <[email-masked]>
Date: Tue, 11 Apr 2017 16:04:50 +0900

Commit c94e6942ce missed the following instances:

 - validateCheckConstraint
 - DefineQueryRewrite
---
 src/backend/commands/tablecmds.c          |  8 ++++++--
 src/backend/rewrite/rewriteDefine.c       | 32 +++++++++++++++++++++----------
 src/test/regress/expected/alter_table.out |  6 ++++++
 src/test/regress/expected/rules.out       | 29 ++++++++++++++++++++++++++++
 src/test/regress/sql/alter_table.sql      |  7 +++++++
 src/test/regress/sql/rules.sql            | 22 +++++++++++++++++++++
 6 files changed, 92 insertions(+), 12 deletions(-)

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index abb262b851..4438a7b95c 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -8090,8 +8090,12 @@ validateCheckConstraint(Relation rel, HeapTuple constrtup)
 	bool		isnull;
 	Snapshot	snapshot;
 
-	/* VALIDATE CONSTRAINT is a no-op for foreign tables */
-	if (rel->rd_rel->relkind == RELKIND_FOREIGN_TABLE)
+	/*
+	 * VALIDATE CONSTRAINT is a no-op for foreign tables and partitioned
+	 * tables.
+	 */
+	if (rel->rd_rel->relkind == RELKIND_FOREIGN_TABLE ||
+		rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
 		return;
 
 	constrForm = (Form_pg_constraint) GETSTRUCT(constrtup);
diff --git a/src/backend/rewrite/rewriteDefine.c b/src/backend/rewrite/rewriteDefine.c
index 86d588bba5..407c92d1f9 100644
--- a/src/backend/rewrite/rewriteDefine.c
+++ b/src/backend/rewrite/rewriteDefine.c
@@ -419,18 +419,26 @@ DefineQueryRewrite(char *rulename,
 		if (event_relation->rd_rel->relkind != RELKIND_VIEW &&
 			event_relation->rd_rel->relkind != RELKIND_MATVIEW)
 		{
-			HeapScanDesc scanDesc;
-			Snapshot	snapshot;
+			/*
+			 * Must not try to scan a partitioned table; if there is any data,
+			 * it would be in partitions (child tables), which if there are
+			 * any, we will throw an error below.
+			 */
+			if (event_relation->rd_rel->relkind != RELKIND_PARTITIONED_TABLE)
+			{
+				HeapScanDesc scanDesc;
+				Snapshot	snapshot;
 
-			snapshot = RegisterSnapshot(GetLatestSnapshot());
-			scanDesc = heap_beginscan(event_relation, snapshot, 0, NULL);
-			if (heap_getnext(scanDesc, ForwardScanDirection) != NULL)
-				ereport(ERROR,
+				snapshot = RegisterSnapshot(GetLatestSnapshot());
+				scanDesc = heap_beginscan(event_relation, snapshot, 0, NULL);
+				if (heap_getnext(scanDesc, ForwardScanDirection) != NULL)
+					ereport(ERROR,
 						(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
 						 errmsg("could not convert table \"%s\" to a view because it is not empty",
 								RelationGetRelationName(event_relation))));
-			heap_endscan(scanDesc);
-			UnregisterSnapshot(snapshot);
+				heap_endscan(scanDesc);
+				UnregisterSnapshot(snapshot);
+			}
 
 			if (event_relation->rd_rel->relhastriggers)
 				ereport(ERROR,
@@ -551,8 +559,12 @@ DefineQueryRewrite(char *rulename,
 		relationRelation = heap_open(RelationRelationId, RowExclusiveLock);
 		toastrelid = event_relation->rd_rel->reltoastrelid;
 
-		/* drop storage while table still looks like a table  */
-		RelationDropStorage(event_relation);
+		/*
+		 * Drop storage while table still looks like a table; must not try
+		 * if it's a partitioned table.
+		 */
+		if (event_relation->rd_rel->relkind != RELKIND_PARTITIONED_TABLE)
+			RelationDropStorage(event_relation);
 		DeleteSystemAttributeTuples(event_relid);
 
 		/*
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index 2227f2d977..883a5c9864 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -3372,3 +3372,9 @@ ERROR:  partition constraint is violated by some row
 -- cleanup
 drop table p;
 drop table p1;
+-- validate constraint on partitioned tables should only scan leaf partitions
+create table parted_validate_test (a int) partition by list (a);
+create table parted_validate_test_1 partition of parted_validate_test for values in (0, 1);
+alter table parted_validate_test add constraint parted_validate_test_chka check (a > 0) not valid;
+alter table parted_validate_test validate constraint parted_validate_test_chka;
+drop table parted_validate_test;
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index cba82bb114..4d794ac4ba 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -2574,6 +2574,35 @@ select reltoastrelid, relkind, relfrozenxid
 (1 row)
 
 drop view fooview;
+-- check the same for a partitioned table
+create table partedfooview (x int, y text) partition by list (x);
+create table partedfooview_part partition of partedfooview for values in (1);
+-- nope, cannot convert if there exists a child table
+create rule "_RETURN" as on select to partedfooview do instead
+  select 1 as x, 'aaa'::text as y;
+ERROR:  could not convert table "partedfooview" to a view because it has child tables
+drop table partedfooview_part;
+analyze partedfooview;	-- reset relhassubclass
+create rule "_RETURN" as on select to partedfooview do instead
+  select 1 as x, 'aaa'::text as y;
+select * from partedfooview;
+ x |  y  
+---+-----
+ 1 | aaa
+(1 row)
+
+select xmin, * from partedfooview;  -- fail, views don't have such a column
+ERROR:  column "xmin" does not exist
+LINE 1: select xmin, * from partedfooview;
+               ^
+select reltoastrelid, relkind, relfrozenxid
+  from pg_class where oid = 'partedfooview'::regclass;
+ reltoastrelid | relkind | relfrozenxid 
+---------------+---------+--------------
+             0 | v       |            0
+(1 row)
+
+drop view partedfooview;
 --
 -- check for planner problems with complex inherited UPDATES
 --
diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql
index 8cd6786a90..eb1b4b536f 100644
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -2228,3 +2228,10 @@ alter table p attach partition p1 for values from (1, 2) to (1, 10);
 -- cleanup
 drop table p;
 drop table p1;
+
+-- validate constraint on partitioned tables should only scan leaf partitions
+create table parted_validate_test (a int) partition by list (a);
+create table parted_validate_test_1 partition of parted_validate_test for values in (0, 1);
+alter table parted_validate_test add constraint parted_validate_test_chka check (a > 0) not valid;
+alter table parted_validate_test validate constraint parted_validate_test_chka;
+drop table parted_validate_test;
diff --git a/src/test/regress/sql/rules.sql b/src/test/regress/sql/rules.sql
index dcff0de2a5..c86ea83837 100644
--- a/src/test/regress/sql/rules.sql
+++ b/src/test/regress/sql/rules.sql
@@ -898,6 +898,28 @@ select reltoastrelid, relkind, relfrozenxid
 
 drop view fooview;
 
+-- check the same for a partitioned table
+create table partedfooview (x int, y text) partition by list (x);
+create table partedfooview_part partition of partedfooview for values in (1);
+
+-- nope, cannot convert if there exists a child table
+create rule "_RETURN" as on select to partedfooview do instead
+  select 1 as x, 'aaa'::text as y;
+
+drop table partedfooview_part;
+analyze partedfooview;	-- reset relhassubclass
+
+create rule "_RETURN" as on select to partedfooview do instead
+  select 1 as x, 'aaa'::text as y;
+
+select * from partedfooview;
+select xmin, * from partedfooview;  -- fail, views don't have such a column
+
+select reltoastrelid, relkind, relfrozenxid
+  from pg_class where oid = 'partedfooview'::regclass;
+
+drop view partedfooview;
+
 --
 -- check for planner problems with complex inherited UPDATES
 --
-- 
2.11.0

--------------DDCCCD2FCF013392508D20C5
Content-Type: text/plain
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0

-- 
Sent via pgsql-hackers mailing list ([email-masked])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

--------------DDCCCD2FCF013392508D20C5--