From e0e0d792dedf855e01d82f2f608ee02b9cded039 Mon Sep 17 00:00:00 2001 From: amit 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