On 2017/04/03 11:39, Amit Langote wrote:
> On 2017/04/01 5:29, Robert Haas wrote:
>> Thanks for reviewing, and thanks to Maksim as well, and thanks to Amit
>> for writing the patch.
>
> Thanks for committing. :)
I noticed that I had missed a couple of places that would try to scan
partitioned tables, resulting in file access.
1. In validateCheckConstraint(), along with foreign tables, must ignore
partitioned tables.
2. DefineQueryRewrite() may try to scan a partitioned table in the case of
converting a table to view, where we must make sure that the table being
converted is empty. It's checked by scanning the heap, which we should
not do for a partitioned table. Nor should we try to drop the storage
once ready to make the table into a REKIND_VIEW relation (because all
other checks passed okaying the conversion).
Tests are added for both the cases.
Thanks,
Amit
>From e0e0d792dedf855e01d82f2f608ee02b9cded039 Mon Sep 17 00:00:00 2001
From: amit <amitlangot...@gmail.com>
Date: Tue, 11 Apr 2017 16:04:50 +0900
Subject: [PATCH] Fix few places still scanning partitioned tables
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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
