dropping partitioned tables without CASCADE
Re-posting the patch I posted in a nearby thread [0]/messages/by-id/ca132b99-0d18-439a-fe65-024085449259@lab.ntt.co.jp.
On 2017/02/16 2:08, Robert Haas wrote:
On Wed, Feb 15, 2017 at 11:34 AM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:I think new-style partitioning is supposed to consider each partition as
an implementation detail of the table; the fact that you can manipulate
partitions separately does not really mean that they are their own
independent object. You don't stop to think "do I really want to drop
the TOAST table attached to this main table?" and attach a CASCADE
clause if so. You just drop the main table, and the toast one is
dropped automatically. I think new-style partitions should behave
equivalently.That's a reasonable point of view. I'd like to get some more opinions
on this topic. I'm happy to have us do whatever most people want, but
I'm worried that having table inheritance and table partitioning work
differently will be create confusion. I'm also suspicious that there
may be some implementation difficulties. On the hand, it does seem a
little silly to say that DROP TABLE partitioned_table should always
fail except in the degenerate case where there are no partitions, so
maybe changing it is for the best.
So I count more than a few votes saying that we should be able to DROP
partitioned tables without specifying CASCADE.
I tried to implement that using the attached patch by having
StoreCatalogInheritance1() create DEPENDENCY_AUTO dependency between
parent and child if the child is a partition, instead of DEPENDENCY_NORMAL
that would otherwise be created. Now it seems that that is one way of
making sure that partitions are dropped when the root partitioned table is
dropped, not sure if the best; why create the pg_depend entries at all one
might ask. I chose it for now because that's the one with fewest lines of
change. Adjusted regression tests as well, since we recently tweaked
tests [1]https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=c397814 to work around the irregularities of test output when using CASCADE.
Thanks,
Amit
[0]: /messages/by-id/ca132b99-0d18-439a-fe65-024085449259@lab.ntt.co.jp
[1]: https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=c397814
Attachments:
0001-Allow-dropping-partitioned-table-without-CASCADE.patchtext/x-diff; name=0001-Allow-dropping-partitioned-table-without-CASCADE.patchDownload
From 68133e249156a36be15a6e2e02f702e10f356db5 Mon Sep 17 00:00:00 2001
From: amit <amitlangote09@gmail.com>
Date: Thu, 16 Feb 2017 15:56:44 +0900
Subject: [PATCH] Allow dropping partitioned table without CASCADE
Currently, a normal dependency is created between a inheritance
parent and child when creating the child. That means one must
specify CASCADE to drop the parent table if a child table exists.
When creating partitions as inheritance children, create auto
dependency instead, so that partitions are dropped automatically
when the parent is dropped i.e., without specifying CASCADE.
---
src/backend/commands/tablecmds.c | 26 ++++++++++++++++++--------
src/test/regress/expected/alter_table.out | 10 ++++------
src/test/regress/expected/create_table.out | 9 ++-------
src/test/regress/expected/inherit.out | 18 ------------------
src/test/regress/expected/insert.out | 7 ++-----
src/test/regress/expected/update.out | 5 -----
src/test/regress/sql/alter_table.sql | 10 ++++------
src/test/regress/sql/create_table.sql | 9 ++-------
src/test/regress/sql/insert.sql | 7 ++-----
9 files changed, 34 insertions(+), 67 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 3cea220421..31b50ad77f 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -289,9 +289,11 @@ static List *MergeAttributes(List *schema, List *supers, char relpersistence,
static bool MergeCheckConstraint(List *constraints, char *name, Node *expr);
static void MergeAttributesIntoExisting(Relation child_rel, Relation parent_rel);
static void MergeConstraintsIntoExisting(Relation child_rel, Relation parent_rel);
-static void StoreCatalogInheritance(Oid relationId, List *supers);
+static void StoreCatalogInheritance(Oid relationId, List *supers,
+ bool child_is_partition);
static void StoreCatalogInheritance1(Oid relationId, Oid parentOid,
- int16 seqNumber, Relation inhRelation);
+ int16 seqNumber, Relation inhRelation,
+ bool child_is_partition);
static int findAttrByName(const char *attributeName, List *schema);
static void AlterIndexNamespaces(Relation classRel, Relation rel,
Oid oldNspOid, Oid newNspOid, ObjectAddresses *objsMoved);
@@ -725,7 +727,7 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
typaddress);
/* Store inheritance information for new rel. */
- StoreCatalogInheritance(relationId, inheritOids);
+ StoreCatalogInheritance(relationId, inheritOids, stmt->partbound != NULL);
/*
* We must bump the command counter to make the newly-created relation
@@ -2240,7 +2242,8 @@ MergeCheckConstraint(List *constraints, char *name, Node *expr)
* supers is a list of the OIDs of the new relation's direct ancestors.
*/
static void
-StoreCatalogInheritance(Oid relationId, List *supers)
+StoreCatalogInheritance(Oid relationId, List *supers,
+ bool child_is_partition)
{
Relation relation;
int16 seqNumber;
@@ -2270,7 +2273,8 @@ StoreCatalogInheritance(Oid relationId, List *supers)
{
Oid parentOid = lfirst_oid(entry);
- StoreCatalogInheritance1(relationId, parentOid, seqNumber, relation);
+ StoreCatalogInheritance1(relationId, parentOid, seqNumber, relation,
+ child_is_partition);
seqNumber++;
}
@@ -2283,7 +2287,8 @@ StoreCatalogInheritance(Oid relationId, List *supers)
*/
static void
StoreCatalogInheritance1(Oid relationId, Oid parentOid,
- int16 seqNumber, Relation inhRelation)
+ int16 seqNumber, Relation inhRelation,
+ bool child_is_partition)
{
TupleDesc desc = RelationGetDescr(inhRelation);
Datum values[Natts_pg_inherits];
@@ -2317,7 +2322,10 @@ StoreCatalogInheritance1(Oid relationId, Oid parentOid,
childobject.objectId = relationId;
childobject.objectSubId = 0;
- recordDependencyOn(&childobject, &parentobject, DEPENDENCY_NORMAL);
+ if (!child_is_partition)
+ recordDependencyOn(&childobject, &parentobject, DEPENDENCY_NORMAL);
+ else
+ recordDependencyOn(&childobject, &parentobject, DEPENDENCY_AUTO);
/*
* Post creation hook of this inheritance. Since object_access_hook
@@ -10744,7 +10752,9 @@ CreateInheritance(Relation child_rel, Relation parent_rel)
StoreCatalogInheritance1(RelationGetRelid(child_rel),
RelationGetRelid(parent_rel),
inhseqno + 1,
- catalogRelation);
+ catalogRelation,
+ parent_rel->rd_rel->relkind ==
+ RELKIND_PARTITIONED_TABLE);
/* Now we're done with pg_inherits */
heap_close(catalogRelation, RowExclusiveLock);
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index e84af67fb2..ca66158ee3 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -3339,10 +3339,8 @@ ALTER TABLE list_parted2 DROP COLUMN b;
ERROR: cannot drop column named in partition key
ALTER TABLE list_parted2 ALTER COLUMN b TYPE text;
ERROR: cannot alter type of column named in partition key
--- cleanup: avoid using CASCADE
-DROP TABLE list_parted, part_1;
-DROP TABLE list_parted2, part_2, part_5, part_5_a;
-DROP TABLE range_parted, part1, part2;
+-- cleanup
+DROP TABLE list_parted, list_parted2, range_parted;
-- more tests for certain multi-level partitioning scenarios
create table p (a int, b int) partition by range (a, b);
create table p1 (b int, a int not null) partition by range (b);
@@ -3371,5 +3369,5 @@ insert into p1 (a, b) values (2, 3);
-- check that partition validation scan correctly detects violating rows
alter table p attach partition p1 for values from (1, 2) to (1, 10);
ERROR: partition constraint is violated by some row
--- cleanup: avoid using CASCADE
-drop table p, p1, p11;
+-- cleanup
+drop table p;
diff --git a/src/test/regress/expected/create_table.out b/src/test/regress/expected/create_table.out
index 20eb3d35f9..c07a474b3d 100644
--- a/src/test/regress/expected/create_table.out
+++ b/src/test/regress/expected/create_table.out
@@ -667,10 +667,5 @@ Check constraints:
"check_a" CHECK (length(a) > 0)
Number of partitions: 3 (Use \d+ to list them.)
--- cleanup: avoid using CASCADE
-DROP TABLE parted, part_a, part_b, part_c, part_c_1_10;
-DROP TABLE list_parted, part_1, part_2, part_null;
-DROP TABLE range_parted;
-DROP TABLE list_parted2, part_ab, part_null_z;
-DROP TABLE range_parted2, part0, part1, part2, part3;
-DROP TABLE range_parted3, part00, part10, part11, part12;
+-- cleanup
+DROP TABLE parted, list_parted, range_parted, list_parted2, range_parted2, range_parted3;
diff --git a/src/test/regress/expected/inherit.out b/src/test/regress/expected/inherit.out
index a8c8b28a75..623aa1db93 100644
--- a/src/test/regress/expected/inherit.out
+++ b/src/test/regress/expected/inherit.out
@@ -1844,22 +1844,4 @@ explain (costs off) select * from range_list_parted where a >= 30;
(11 rows)
drop table list_parted cascade;
-NOTICE: drop cascades to 3 other objects
-DETAIL: drop cascades to table part_ab_cd
-drop cascades to table part_ef_gh
-drop cascades to table part_null_xy
drop table range_list_parted cascade;
-NOTICE: drop cascades to 13 other objects
-DETAIL: drop cascades to table part_1_10
-drop cascades to table part_1_10_ab
-drop cascades to table part_1_10_cd
-drop cascades to table part_10_20
-drop cascades to table part_10_20_ab
-drop cascades to table part_10_20_cd
-drop cascades to table part_21_30
-drop cascades to table part_21_30_ab
-drop cascades to table part_21_30_cd
-drop cascades to table part_40_inf
-drop cascades to table part_40_inf_ab
-drop cascades to table part_40_inf_cd
-drop cascades to table part_40_inf_null
diff --git a/src/test/regress/expected/insert.out b/src/test/regress/expected/insert.out
index 81af3ef497..31cfa4e76e 100644
--- a/src/test/regress/expected/insert.out
+++ b/src/test/regress/expected/insert.out
@@ -314,10 +314,7 @@ select tableoid::regclass::text, a, min(b) as min_b, max(b) as max_b from list_p
(9 rows)
-- cleanup
-drop table part1, part2, part3, part4, range_parted;
-drop table part_ee_ff3_1, part_ee_ff3_2, part_ee_ff1, part_ee_ff2, part_ee_ff3;
-drop table part_ee_ff, part_gg2_2, part_gg2_1, part_gg2, part_gg1, part_gg;
-drop table part_aa_bb, part_cc_dd, part_null, list_parted;
+drop table range_parted, list_parted;
-- more tests for certain multi-level partitioning scenarios
create table p (a int, b int) partition by range (a, b);
create table p1 (b int not null, a int not null) partition by range ((b+0));
@@ -387,4 +384,4 @@ with ins (a, b, c) as
(5 rows)
-- cleanup
-drop table p, p1, p11, p12, p2, p3, p4;
+drop table p;
diff --git a/src/test/regress/expected/update.out b/src/test/regress/expected/update.out
index a1e9255450..af0d5bfffe 100644
--- a/src/test/regress/expected/update.out
+++ b/src/test/regress/expected/update.out
@@ -220,8 +220,3 @@ DETAIL: Failing row contains (b, 9).
update range_parted set b = b + 1 where b = 10;
-- cleanup
drop table range_parted cascade;
-NOTICE: drop cascades to 4 other objects
-DETAIL: drop cascades to table part_a_1_a_10
-drop cascades to table part_a_10_a_20
-drop cascades to table part_b_1_b_10
-drop cascades to table part_b_10_b_20
diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql
index a403fd8cb4..fbcc739f41 100644
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -2199,10 +2199,8 @@ ALTER TABLE part_2 INHERIT inh_test;
ALTER TABLE list_parted2 DROP COLUMN b;
ALTER TABLE list_parted2 ALTER COLUMN b TYPE text;
--- cleanup: avoid using CASCADE
-DROP TABLE list_parted, part_1;
-DROP TABLE list_parted2, part_2, part_5, part_5_a;
-DROP TABLE range_parted, part1, part2;
+-- cleanup
+DROP TABLE list_parted, list_parted2, range_parted;
-- more tests for certain multi-level partitioning scenarios
create table p (a int, b int) partition by range (a, b);
@@ -2227,5 +2225,5 @@ insert into p1 (a, b) values (2, 3);
-- check that partition validation scan correctly detects violating rows
alter table p attach partition p1 for values from (1, 2) to (1, 10);
--- cleanup: avoid using CASCADE
-drop table p, p1, p11;
+-- cleanup
+drop table p;
diff --git a/src/test/regress/sql/create_table.sql b/src/test/regress/sql/create_table.sql
index f41dd71475..1f0fa8e16d 100644
--- a/src/test/regress/sql/create_table.sql
+++ b/src/test/regress/sql/create_table.sql
@@ -595,10 +595,5 @@ CREATE TABLE part_c_1_10 PARTITION OF part_c FOR VALUES FROM (1) TO (10);
-- returned.
\d parted
--- cleanup: avoid using CASCADE
-DROP TABLE parted, part_a, part_b, part_c, part_c_1_10;
-DROP TABLE list_parted, part_1, part_2, part_null;
-DROP TABLE range_parted;
-DROP TABLE list_parted2, part_ab, part_null_z;
-DROP TABLE range_parted2, part0, part1, part2, part3;
-DROP TABLE range_parted3, part00, part10, part11, part12;
+-- cleanup
+DROP TABLE parted, list_parted, range_parted, list_parted2, range_parted2, range_parted3;
diff --git a/src/test/regress/sql/insert.sql b/src/test/regress/sql/insert.sql
index 454e1ce2e7..dfdc24eba8 100644
--- a/src/test/regress/sql/insert.sql
+++ b/src/test/regress/sql/insert.sql
@@ -186,10 +186,7 @@ insert into list_parted (b) values (1);
select tableoid::regclass::text, a, min(b) as min_b, max(b) as max_b from list_parted group by 1, 2 order by 1;
-- cleanup
-drop table part1, part2, part3, part4, range_parted;
-drop table part_ee_ff3_1, part_ee_ff3_2, part_ee_ff1, part_ee_ff2, part_ee_ff3;
-drop table part_ee_ff, part_gg2_2, part_gg2_1, part_gg2, part_gg1, part_gg;
-drop table part_aa_bb, part_cc_dd, part_null, list_parted;
+drop table range_parted, list_parted;
-- more tests for certain multi-level partitioning scenarios
create table p (a int, b int) partition by range (a, b);
@@ -241,4 +238,4 @@ with ins (a, b, c) as
select a, b, min(c), max(c) from ins group by a, b order by 1;
-- cleanup
-drop table p, p1, p11, p12, p2, p3, p4;
+drop table p;
--
2.11.0
Thanks for working on all the follow on work for partitioning feature.
May be you should add all those patches in the next commitfest, so
that we don't forget those.
On Mon, Feb 20, 2017 at 7:46 AM, Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:
Re-posting the patch I posted in a nearby thread [0].
On 2017/02/16 2:08, Robert Haas wrote:
On Wed, Feb 15, 2017 at 11:34 AM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:I think new-style partitioning is supposed to consider each partition as
an implementation detail of the table; the fact that you can manipulate
partitions separately does not really mean that they are their own
independent object. You don't stop to think "do I really want to drop
the TOAST table attached to this main table?" and attach a CASCADE
clause if so. You just drop the main table, and the toast one is
dropped automatically. I think new-style partitions should behave
equivalently.That's a reasonable point of view. I'd like to get some more opinions
on this topic. I'm happy to have us do whatever most people want, but
I'm worried that having table inheritance and table partitioning work
differently will be create confusion. I'm also suspicious that there
may be some implementation difficulties. On the hand, it does seem a
little silly to say that DROP TABLE partitioned_table should always
fail except in the degenerate case where there are no partitions, so
maybe changing it is for the best.So I count more than a few votes saying that we should be able to DROP
partitioned tables without specifying CASCADE.I tried to implement that using the attached patch by having
StoreCatalogInheritance1() create DEPENDENCY_AUTO dependency between
parent and child if the child is a partition, instead of DEPENDENCY_NORMAL
that would otherwise be created. Now it seems that that is one way of
making sure that partitions are dropped when the root partitioned table is
dropped, not sure if the best; why create the pg_depend entries at all one
might ask. I chose it for now because that's the one with fewest lines of
change. Adjusted regression tests as well, since we recently tweaked
tests [1] to work around the irregularities of test output when using CASCADE.
The patch applies cleanly and regression does not show any failures.
Here are some comments
For the sake of readability you may want reverse the if and else order.
- recordDependencyOn(&childobject, &parentobject, DEPENDENCY_NORMAL);
+ if (!child_is_partition)
+ recordDependencyOn(&childobject, &parentobject, DEPENDENCY_NORMAL);
+ else
+ recordDependencyOn(&childobject, &parentobject, DEPENDENCY_AUTO);
like
+ if (child_is_partition)
+ recordDependencyOn(&childobject, &parentobject, DEPENDENCY_AUTO);
+ else
+ recordDependencyOn(&childobject, &parentobject, DEPENDENCY_NORMAL);It's weird that somebody can perform DROP TABLE on the partition without
referring to its parent. That may be a useful feature as it allows one to
detach the partition as well as remove the table in one command. But it looks
wierd for someone familiar with partitioning features of other DBMSes. But then
our partition creation command is CREATE TABLE .... So may be this is expected
difference.
--- cleanup: avoid using CASCADE
-DROP TABLE list_parted, part_1;
-DROP TABLE list_parted2, part_2, part_5, part_5_a;
-DROP TABLE range_parted, part1, part2;
+-- cleanup
+DROP TABLE list_parted, list_parted2, range_parted;
Testcases usually drop one table at a time, I guess, to reduce the differences
when we add or remove tables from testcases. All such blocks should probably
follow same policy. drop table list_parted cascade;
-NOTICE: drop cascades to 3 other objects
-DETAIL: drop cascades to table part_ab_cd
probably we should remove cascade from there, unless you are testing CASCADE
functionality. Similarly for other blocks like
drop table range_parted cascade;
BTW, I noticed that although we are allowing foreign tables to be
partitions, there are no tests in foreign_data.sql for testing it. If
there would have been we would tests DROP TABLE on a partitioned table
with foreign partitions as well. That file has testcases for testing
foreign table inheritance, and should have tests for foreign table
partitions.
--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Hi Ashutosh,
Thanks for taking a look at the patch.
On 2017/02/20 21:49, Ashutosh Bapat wrote:
Thanks for working on all the follow on work for partitioning feature.
May be you should add all those patches in the next commitfest, so
that we don't forget those.
I think adding these as one of the PostgreSQL 10 Open Items [0]https://wiki.postgresql.org/wiki/PostgreSQL_10_Open_Items might be
better. I've done that.
On Mon, Feb 20, 2017 at 7:46 AM, Amit Langote wrote:
So I count more than a few votes saying that we should be able to DROP
partitioned tables without specifying CASCADE.I tried to implement that using the attached patch by having
StoreCatalogInheritance1() create DEPENDENCY_AUTO dependency between
parent and child if the child is a partition, instead of DEPENDENCY_NORMAL
that would otherwise be created. Now it seems that that is one way of
making sure that partitions are dropped when the root partitioned table is
dropped, not sure if the best; why create the pg_depend entries at all one
might ask. I chose it for now because that's the one with fewest lines of
change. Adjusted regression tests as well, since we recently tweaked
tests [1] to work around the irregularities of test output when using CASCADE.The patch applies cleanly and regression does not show any failures.
Here are some comments
For the sake of readability you may want reverse the if and else order. - recordDependencyOn(&childobject, &parentobject, DEPENDENCY_NORMAL); + if (!child_is_partition) + recordDependencyOn(&childobject, &parentobject, DEPENDENCY_NORMAL); + else + recordDependencyOn(&childobject, &parentobject, DEPENDENCY_AUTO); like + if (child_is_partition) + recordDependencyOn(&childobject, &parentobject, DEPENDENCY_AUTO); + else + recordDependencyOn(&childobject, &parentobject, DEPENDENCY_NORMAL);
Sure, done.
It's weird that somebody can perform DROP TABLE on the partition without
referring to its parent. That may be a useful feature as it allows one to
detach the partition as well as remove the table in one command. But it looks
wierd for someone familiar with partitioning features of other DBMSes. But then
our partition creation command is CREATE TABLE .... So may be this is expected
difference.
There is a line on the CREATE TABLE page in the description of PARTITION
OF clause:
"Note that dropping a partition with DROP TABLE requires taking an ACCESS
EXCLUSIVE lock on the parent table."
In earlier proposals I had included the ALTER TABLE parent ADD/DROP
PARTITION commands, but CRAETE TABLE PARTITION OF / DROP TABLE prevailed.
--- cleanup: avoid using CASCADE -DROP TABLE list_parted, part_1; -DROP TABLE list_parted2, part_2, part_5, part_5_a; -DROP TABLE range_parted, part1, part2; +-- cleanup +DROP TABLE list_parted, list_parted2, range_parted; Testcases usually drop one table at a time, I guess, to reduce the differences when we add or remove tables from testcases. All such blocks should probably follow same policy.
Hmm, I see this in src/test/regress/sql/inherit.sql:141
DROP TABLE firstparent, secondparent, jointchild, thirdparent, otherchild;
drop table list_parted cascade;
-NOTICE: drop cascades to 3 other objects
-DETAIL: drop cascades to table part_ab_cd
probably we should remove cascade from there, unless you are testing CASCADE
functionality. Similarly for other blocks like
drop table range_parted cascade;BTW, I noticed that although we are allowing foreign tables to be
partitions, there are no tests in foreign_data.sql for testing it. If
there would have been we would tests DROP TABLE on a partitioned table
with foreign partitions as well. That file has testcases for testing
foreign table inheritance, and should have tests for foreign table
partitions.
That makes sense. Patch 0002 is for that (I'm afraid this should be
posted separately though). I didn't add/repeat all the tests that were
added by the foreign table inheritance patch again for foreign partitions
(common inheritance rules apply to both cases), only added those for the
new partitioning commands and certain new rules.
Thanks,
Amit
[0]: https://wiki.postgresql.org/wiki/PostgreSQL_10_Open_Items
Attachments:
0001-Allow-dropping-partitioned-table-without-CASCADE.patchtext/x-diff; name=0001-Allow-dropping-partitioned-table-without-CASCADE.patchDownload
From c7cff17ab7861dbd0a4fb329115b3f99fd800325 Mon Sep 17 00:00:00 2001
From: amit <amitlangote09@gmail.com>
Date: Thu, 16 Feb 2017 15:56:44 +0900
Subject: [PATCH 1/2] Allow dropping partitioned table without CASCADE
Currently, a normal dependency is created between a inheritance
parent and child when creating the child. That means one must
specify CASCADE to drop the parent table if a child table exists.
When creating partitions as inheritance children, create auto
dependency instead, so that partitions are dropped automatically
when the parent is dropped i.e., without specifying CASCADE.
---
src/backend/commands/tablecmds.c | 26 ++++++++++++++++++--------
src/test/regress/expected/alter_table.out | 10 ++++------
src/test/regress/expected/create_table.out | 9 ++-------
src/test/regress/expected/inherit.out | 18 ------------------
src/test/regress/expected/insert.out | 7 ++-----
src/test/regress/expected/update.out | 5 -----
src/test/regress/sql/alter_table.sql | 10 ++++------
src/test/regress/sql/create_table.sql | 9 ++-------
src/test/regress/sql/insert.sql | 7 ++-----
9 files changed, 34 insertions(+), 67 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 3cea220421..31b50ad77f 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -289,9 +289,11 @@ static List *MergeAttributes(List *schema, List *supers, char relpersistence,
static bool MergeCheckConstraint(List *constraints, char *name, Node *expr);
static void MergeAttributesIntoExisting(Relation child_rel, Relation parent_rel);
static void MergeConstraintsIntoExisting(Relation child_rel, Relation parent_rel);
-static void StoreCatalogInheritance(Oid relationId, List *supers);
+static void StoreCatalogInheritance(Oid relationId, List *supers,
+ bool child_is_partition);
static void StoreCatalogInheritance1(Oid relationId, Oid parentOid,
- int16 seqNumber, Relation inhRelation);
+ int16 seqNumber, Relation inhRelation,
+ bool child_is_partition);
static int findAttrByName(const char *attributeName, List *schema);
static void AlterIndexNamespaces(Relation classRel, Relation rel,
Oid oldNspOid, Oid newNspOid, ObjectAddresses *objsMoved);
@@ -725,7 +727,7 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
typaddress);
/* Store inheritance information for new rel. */
- StoreCatalogInheritance(relationId, inheritOids);
+ StoreCatalogInheritance(relationId, inheritOids, stmt->partbound != NULL);
/*
* We must bump the command counter to make the newly-created relation
@@ -2240,7 +2242,8 @@ MergeCheckConstraint(List *constraints, char *name, Node *expr)
* supers is a list of the OIDs of the new relation's direct ancestors.
*/
static void
-StoreCatalogInheritance(Oid relationId, List *supers)
+StoreCatalogInheritance(Oid relationId, List *supers,
+ bool child_is_partition)
{
Relation relation;
int16 seqNumber;
@@ -2270,7 +2273,8 @@ StoreCatalogInheritance(Oid relationId, List *supers)
{
Oid parentOid = lfirst_oid(entry);
- StoreCatalogInheritance1(relationId, parentOid, seqNumber, relation);
+ StoreCatalogInheritance1(relationId, parentOid, seqNumber, relation,
+ child_is_partition);
seqNumber++;
}
@@ -2283,7 +2287,8 @@ StoreCatalogInheritance(Oid relationId, List *supers)
*/
static void
StoreCatalogInheritance1(Oid relationId, Oid parentOid,
- int16 seqNumber, Relation inhRelation)
+ int16 seqNumber, Relation inhRelation,
+ bool child_is_partition)
{
TupleDesc desc = RelationGetDescr(inhRelation);
Datum values[Natts_pg_inherits];
@@ -2317,7 +2322,10 @@ StoreCatalogInheritance1(Oid relationId, Oid parentOid,
childobject.objectId = relationId;
childobject.objectSubId = 0;
- recordDependencyOn(&childobject, &parentobject, DEPENDENCY_NORMAL);
+ if (!child_is_partition)
+ recordDependencyOn(&childobject, &parentobject, DEPENDENCY_NORMAL);
+ else
+ recordDependencyOn(&childobject, &parentobject, DEPENDENCY_AUTO);
/*
* Post creation hook of this inheritance. Since object_access_hook
@@ -10744,7 +10752,9 @@ CreateInheritance(Relation child_rel, Relation parent_rel)
StoreCatalogInheritance1(RelationGetRelid(child_rel),
RelationGetRelid(parent_rel),
inhseqno + 1,
- catalogRelation);
+ catalogRelation,
+ parent_rel->rd_rel->relkind ==
+ RELKIND_PARTITIONED_TABLE);
/* Now we're done with pg_inherits */
heap_close(catalogRelation, RowExclusiveLock);
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index e84af67fb2..ca66158ee3 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -3339,10 +3339,8 @@ ALTER TABLE list_parted2 DROP COLUMN b;
ERROR: cannot drop column named in partition key
ALTER TABLE list_parted2 ALTER COLUMN b TYPE text;
ERROR: cannot alter type of column named in partition key
--- cleanup: avoid using CASCADE
-DROP TABLE list_parted, part_1;
-DROP TABLE list_parted2, part_2, part_5, part_5_a;
-DROP TABLE range_parted, part1, part2;
+-- cleanup
+DROP TABLE list_parted, list_parted2, range_parted;
-- more tests for certain multi-level partitioning scenarios
create table p (a int, b int) partition by range (a, b);
create table p1 (b int, a int not null) partition by range (b);
@@ -3371,5 +3369,5 @@ insert into p1 (a, b) values (2, 3);
-- check that partition validation scan correctly detects violating rows
alter table p attach partition p1 for values from (1, 2) to (1, 10);
ERROR: partition constraint is violated by some row
--- cleanup: avoid using CASCADE
-drop table p, p1, p11;
+-- cleanup
+drop table p;
diff --git a/src/test/regress/expected/create_table.out b/src/test/regress/expected/create_table.out
index 20eb3d35f9..c07a474b3d 100644
--- a/src/test/regress/expected/create_table.out
+++ b/src/test/regress/expected/create_table.out
@@ -667,10 +667,5 @@ Check constraints:
"check_a" CHECK (length(a) > 0)
Number of partitions: 3 (Use \d+ to list them.)
--- cleanup: avoid using CASCADE
-DROP TABLE parted, part_a, part_b, part_c, part_c_1_10;
-DROP TABLE list_parted, part_1, part_2, part_null;
-DROP TABLE range_parted;
-DROP TABLE list_parted2, part_ab, part_null_z;
-DROP TABLE range_parted2, part0, part1, part2, part3;
-DROP TABLE range_parted3, part00, part10, part11, part12;
+-- cleanup
+DROP TABLE parted, list_parted, range_parted, list_parted2, range_parted2, range_parted3;
diff --git a/src/test/regress/expected/inherit.out b/src/test/regress/expected/inherit.out
index a8c8b28a75..623aa1db93 100644
--- a/src/test/regress/expected/inherit.out
+++ b/src/test/regress/expected/inherit.out
@@ -1844,22 +1844,4 @@ explain (costs off) select * from range_list_parted where a >= 30;
(11 rows)
drop table list_parted cascade;
-NOTICE: drop cascades to 3 other objects
-DETAIL: drop cascades to table part_ab_cd
-drop cascades to table part_ef_gh
-drop cascades to table part_null_xy
drop table range_list_parted cascade;
-NOTICE: drop cascades to 13 other objects
-DETAIL: drop cascades to table part_1_10
-drop cascades to table part_1_10_ab
-drop cascades to table part_1_10_cd
-drop cascades to table part_10_20
-drop cascades to table part_10_20_ab
-drop cascades to table part_10_20_cd
-drop cascades to table part_21_30
-drop cascades to table part_21_30_ab
-drop cascades to table part_21_30_cd
-drop cascades to table part_40_inf
-drop cascades to table part_40_inf_ab
-drop cascades to table part_40_inf_cd
-drop cascades to table part_40_inf_null
diff --git a/src/test/regress/expected/insert.out b/src/test/regress/expected/insert.out
index 81af3ef497..31cfa4e76e 100644
--- a/src/test/regress/expected/insert.out
+++ b/src/test/regress/expected/insert.out
@@ -314,10 +314,7 @@ select tableoid::regclass::text, a, min(b) as min_b, max(b) as max_b from list_p
(9 rows)
-- cleanup
-drop table part1, part2, part3, part4, range_parted;
-drop table part_ee_ff3_1, part_ee_ff3_2, part_ee_ff1, part_ee_ff2, part_ee_ff3;
-drop table part_ee_ff, part_gg2_2, part_gg2_1, part_gg2, part_gg1, part_gg;
-drop table part_aa_bb, part_cc_dd, part_null, list_parted;
+drop table range_parted, list_parted;
-- more tests for certain multi-level partitioning scenarios
create table p (a int, b int) partition by range (a, b);
create table p1 (b int not null, a int not null) partition by range ((b+0));
@@ -387,4 +384,4 @@ with ins (a, b, c) as
(5 rows)
-- cleanup
-drop table p, p1, p11, p12, p2, p3, p4;
+drop table p;
diff --git a/src/test/regress/expected/update.out b/src/test/regress/expected/update.out
index a1e9255450..af0d5bfffe 100644
--- a/src/test/regress/expected/update.out
+++ b/src/test/regress/expected/update.out
@@ -220,8 +220,3 @@ DETAIL: Failing row contains (b, 9).
update range_parted set b = b + 1 where b = 10;
-- cleanup
drop table range_parted cascade;
-NOTICE: drop cascades to 4 other objects
-DETAIL: drop cascades to table part_a_1_a_10
-drop cascades to table part_a_10_a_20
-drop cascades to table part_b_1_b_10
-drop cascades to table part_b_10_b_20
diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql
index a403fd8cb4..fbcc739f41 100644
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -2199,10 +2199,8 @@ ALTER TABLE part_2 INHERIT inh_test;
ALTER TABLE list_parted2 DROP COLUMN b;
ALTER TABLE list_parted2 ALTER COLUMN b TYPE text;
--- cleanup: avoid using CASCADE
-DROP TABLE list_parted, part_1;
-DROP TABLE list_parted2, part_2, part_5, part_5_a;
-DROP TABLE range_parted, part1, part2;
+-- cleanup
+DROP TABLE list_parted, list_parted2, range_parted;
-- more tests for certain multi-level partitioning scenarios
create table p (a int, b int) partition by range (a, b);
@@ -2227,5 +2225,5 @@ insert into p1 (a, b) values (2, 3);
-- check that partition validation scan correctly detects violating rows
alter table p attach partition p1 for values from (1, 2) to (1, 10);
--- cleanup: avoid using CASCADE
-drop table p, p1, p11;
+-- cleanup
+drop table p;
diff --git a/src/test/regress/sql/create_table.sql b/src/test/regress/sql/create_table.sql
index f41dd71475..1f0fa8e16d 100644
--- a/src/test/regress/sql/create_table.sql
+++ b/src/test/regress/sql/create_table.sql
@@ -595,10 +595,5 @@ CREATE TABLE part_c_1_10 PARTITION OF part_c FOR VALUES FROM (1) TO (10);
-- returned.
\d parted
--- cleanup: avoid using CASCADE
-DROP TABLE parted, part_a, part_b, part_c, part_c_1_10;
-DROP TABLE list_parted, part_1, part_2, part_null;
-DROP TABLE range_parted;
-DROP TABLE list_parted2, part_ab, part_null_z;
-DROP TABLE range_parted2, part0, part1, part2, part3;
-DROP TABLE range_parted3, part00, part10, part11, part12;
+-- cleanup
+DROP TABLE parted, list_parted, range_parted, list_parted2, range_parted2, range_parted3;
diff --git a/src/test/regress/sql/insert.sql b/src/test/regress/sql/insert.sql
index 454e1ce2e7..dfdc24eba8 100644
--- a/src/test/regress/sql/insert.sql
+++ b/src/test/regress/sql/insert.sql
@@ -186,10 +186,7 @@ insert into list_parted (b) values (1);
select tableoid::regclass::text, a, min(b) as min_b, max(b) as max_b from list_parted group by 1, 2 order by 1;
-- cleanup
-drop table part1, part2, part3, part4, range_parted;
-drop table part_ee_ff3_1, part_ee_ff3_2, part_ee_ff1, part_ee_ff2, part_ee_ff3;
-drop table part_ee_ff, part_gg2_2, part_gg2_1, part_gg2, part_gg1, part_gg;
-drop table part_aa_bb, part_cc_dd, part_null, list_parted;
+drop table range_parted, list_parted;
-- more tests for certain multi-level partitioning scenarios
create table p (a int, b int) partition by range (a, b);
@@ -241,4 +238,4 @@ with ins (a, b, c) as
select a, b, min(c), max(c) from ins group by a, b order by 1;
-- cleanup
-drop table p, p1, p11, p12, p2, p3, p4;
+drop table p;
--
2.11.0
0002-Add-regression-tests-foreign-partition-DDL.patchtext/x-diff; name=0002-Add-regression-tests-foreign-partition-DDL.patchDownload
From 688ef3c1d7584adadef25e78dd679972b817c8d6 Mon Sep 17 00:00:00 2001
From: amit <amitlangote09@gmail.com>
Date: Tue, 21 Feb 2017 15:06:04 +0900
Subject: [PATCH 2/2] Add regression tests foreign partition DDL
Commands like CREATE FOREIGN TABLE .. PARTITION OF, ATTACH PARTITION,
DETACH PARTITION foreign_table didn't get any tests so far. Per
suggestion from Ashutosh Bapat.
---
src/test/regress/expected/foreign_data.out | 194 +++++++++++++++++++++++++++++
src/test/regress/sql/foreign_data.sql | 70 +++++++++++
2 files changed, 264 insertions(+)
diff --git a/src/test/regress/expected/foreign_data.out b/src/test/regress/expected/foreign_data.out
index 3a9fb8f558..541f2eefe1 100644
--- a/src/test/regress/expected/foreign_data.out
+++ b/src/test/regress/expected/foreign_data.out
@@ -1751,6 +1751,200 @@ DETAIL: user mapping for regress_test_role on server s5 depends on server s5
HINT: Use DROP ... CASCADE to drop the dependent objects too.
DROP OWNED BY regress_test_role2 CASCADE;
NOTICE: drop cascades to user mapping for regress_test_role on server s5
+-- Foreign partition DDL stuff
+CREATE TABLE pt2 (
+ c1 integer NOT NULL,
+ c2 text,
+ c3 date
+) PARTITION BY LIST (c1);
+CREATE FOREIGN TABLE pt2_1 PARTITION OF pt2 FOR VALUES IN (1)
+ SERVER s0 OPTIONS (delimiter ',', quote '"', "be quoted" 'value');
+\d+ pt2
+ Table "public.pt2"
+ Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
+--------+---------+-----------+----------+---------+----------+--------------+-------------
+ c1 | integer | | not null | | plain | |
+ c2 | text | | | | extended | |
+ c3 | date | | | | plain | |
+Partition key: LIST (c1)
+Partitions: pt2_1 FOR VALUES IN (1)
+
+\d+ pt2_1
+ Foreign table "public.pt2_1"
+ Column | Type | Collation | Nullable | Default | FDW Options | Storage | Stats target | Description
+--------+---------+-----------+----------+---------+-------------+----------+--------------+-------------
+ c1 | integer | | not null | | | plain | |
+ c2 | text | | | | | extended | |
+ c3 | date | | | | | plain | |
+Partition of: pt2 FOR VALUES IN (1)
+Server: s0
+FDW Options: (delimiter ',', quote '"', "be quoted" 'value')
+
+-- partition cannot have additional columns
+DROP FOREIGN TABLE pt2_1;
+CREATE FOREIGN TABLE pt2_1 (
+ c1 integer NOT NULL,
+ c2 text,
+ c3 date,
+ c4 char
+) SERVER s0 OPTIONS (delimiter ',', quote '"', "be quoted" 'value');
+\d+ pt2_1
+ Foreign table "public.pt2_1"
+ Column | Type | Collation | Nullable | Default | FDW Options | Storage | Stats target | Description
+--------+--------------+-----------+----------+---------+-------------+----------+--------------+-------------
+ c1 | integer | | not null | | | plain | |
+ c2 | text | | | | | extended | |
+ c3 | date | | | | | plain | |
+ c4 | character(1) | | | | | extended | |
+Server: s0
+FDW Options: (delimiter ',', quote '"', "be quoted" 'value')
+
+ALTER TABLE pt2 ATTACH PARTITION pt2_1 FOR VALUES IN (1); -- ERROR
+ERROR: table "pt2_1" contains column "c4" not found in parent "pt2"
+DETAIL: New partition should contain only the columns present in parent.
+DROP FOREIGN TABLE pt2_1;
+\d+ pt2
+ Table "public.pt2"
+ Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
+--------+---------+-----------+----------+---------+----------+--------------+-------------
+ c1 | integer | | not null | | plain | |
+ c2 | text | | | | extended | |
+ c3 | date | | | | plain | |
+Partition key: LIST (c1)
+
+CREATE FOREIGN TABLE pt2_1 (
+ c1 integer NOT NULL,
+ c2 text,
+ c3 date
+) SERVER s0 OPTIONS (delimiter ',', quote '"', "be quoted" 'value');
+\d+ pt2_1
+ Foreign table "public.pt2_1"
+ Column | Type | Collation | Nullable | Default | FDW Options | Storage | Stats target | Description
+--------+---------+-----------+----------+---------+-------------+----------+--------------+-------------
+ c1 | integer | | not null | | | plain | |
+ c2 | text | | | | | extended | |
+ c3 | date | | | | | plain | |
+Server: s0
+FDW Options: (delimiter ',', quote '"', "be quoted" 'value')
+
+-- no attach partition validation occurs for foreign tables
+ALTER TABLE pt2 ATTACH PARTITION pt2_1 FOR VALUES IN (1);
+\d+ pt2
+ Table "public.pt2"
+ Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
+--------+---------+-----------+----------+---------+----------+--------------+-------------
+ c1 | integer | | not null | | plain | |
+ c2 | text | | | | extended | |
+ c3 | date | | | | plain | |
+Partition key: LIST (c1)
+Partitions: pt2_1 FOR VALUES IN (1)
+
+\d+ pt2_1
+ Foreign table "public.pt2_1"
+ Column | Type | Collation | Nullable | Default | FDW Options | Storage | Stats target | Description
+--------+---------+-----------+----------+---------+-------------+----------+--------------+-------------
+ c1 | integer | | not null | | | plain | |
+ c2 | text | | | | | extended | |
+ c3 | date | | | | | plain | |
+Partition of: pt2 FOR VALUES IN (1)
+Server: s0
+FDW Options: (delimiter ',', quote '"', "be quoted" 'value')
+
+-- cannot add column to a partition
+ALTER TABLE pt2_1 ADD c4 char;
+ERROR: cannot add column to a partition
+-- ok to have a partition's own constraints though
+ALTER TABLE pt2_1 ALTER c3 SET NOT NULL;
+ALTER TABLE pt2_1 ADD CONSTRAINT p21chk CHECK (c2 <> '');
+\d+ pt2
+ Table "public.pt2"
+ Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
+--------+---------+-----------+----------+---------+----------+--------------+-------------
+ c1 | integer | | not null | | plain | |
+ c2 | text | | | | extended | |
+ c3 | date | | | | plain | |
+Partition key: LIST (c1)
+Partitions: pt2_1 FOR VALUES IN (1)
+
+\d+ pt2_1
+ Foreign table "public.pt2_1"
+ Column | Type | Collation | Nullable | Default | FDW Options | Storage | Stats target | Description
+--------+---------+-----------+----------+---------+-------------+----------+--------------+-------------
+ c1 | integer | | not null | | | plain | |
+ c2 | text | | | | | extended | |
+ c3 | date | | not null | | | plain | |
+Partition of: pt2 FOR VALUES IN (1)
+Check constraints:
+ "p21chk" CHECK (c2 <> ''::text)
+Server: s0
+FDW Options: (delimiter ',', quote '"', "be quoted" 'value')
+
+-- cannot drop inherited NOT NULL constraint from a partition
+ALTER TABLE pt2_1 ALTER c1 DROP NOT NULL;
+ERROR: column "c1" is marked NOT NULL in parent table
+-- partition must have parent's constraints
+ALTER TABLE pt2 DETACH PARTITION pt2_1;
+ALTER TABLE pt2 ALTER c2 SET NOT NULL;
+\d+ pt2
+ Table "public.pt2"
+ Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
+--------+---------+-----------+----------+---------+----------+--------------+-------------
+ c1 | integer | | not null | | plain | |
+ c2 | text | | not null | | extended | |
+ c3 | date | | | | plain | |
+Partition key: LIST (c1)
+
+\d+ pt2_1
+ Foreign table "public.pt2_1"
+ Column | Type | Collation | Nullable | Default | FDW Options | Storage | Stats target | Description
+--------+---------+-----------+----------+---------+-------------+----------+--------------+-------------
+ c1 | integer | | not null | | | plain | |
+ c2 | text | | | | | extended | |
+ c3 | date | | not null | | | plain | |
+Check constraints:
+ "p21chk" CHECK (c2 <> ''::text)
+Server: s0
+FDW Options: (delimiter ',', quote '"', "be quoted" 'value')
+
+ALTER TABLE pt2 ATTACH PARTITION pt2_1 FOR VALUES IN (1); -- ERROR
+ERROR: column "c2" in child table must be marked NOT NULL
+ALTER FOREIGN TABLE pt2_1 ALTER c2 SET NOT NULL;
+ALTER TABLE pt2 ATTACH PARTITION pt2_1 FOR VALUES IN (1);
+ALTER TABLE pt2 DETACH PARTITION pt2_1;
+ALTER TABLE pt2 ADD CONSTRAINT pt2chk1 CHECK (c1 > 0);
+\d+ pt2
+ Table "public.pt2"
+ Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
+--------+---------+-----------+----------+---------+----------+--------------+-------------
+ c1 | integer | | not null | | plain | |
+ c2 | text | | not null | | extended | |
+ c3 | date | | | | plain | |
+Partition key: LIST (c1)
+Check constraints:
+ "pt2chk1" CHECK (c1 > 0)
+
+\d+ pt2_1
+ Foreign table "public.pt2_1"
+ Column | Type | Collation | Nullable | Default | FDW Options | Storage | Stats target | Description
+--------+---------+-----------+----------+---------+-------------+----------+--------------+-------------
+ c1 | integer | | not null | | | plain | |
+ c2 | text | | not null | | | extended | |
+ c3 | date | | not null | | | plain | |
+Check constraints:
+ "p21chk" CHECK (c2 <> ''::text)
+Server: s0
+FDW Options: (delimiter ',', quote '"', "be quoted" 'value')
+
+ALTER TABLE pt2 ATTACH PARTITION pt2_1 FOR VALUES IN (1); -- ERROR
+ERROR: child table is missing constraint "pt2chk1"
+ALTER FOREIGN TABLE pt2_1 ADD CONSTRAINT pt2chk1 CHECK (c1 > 0);
+ALTER TABLE pt2 ATTACH PARTITION pt2_1 FOR VALUES IN (1);
+-- TRUNCATE doesn't work on foreign tables, either directly or recursively
+TRUNCATE pt2_1; -- ERROR
+ERROR: "pt2_1" is not a table
+TRUNCATE pt2; -- ERROR
+ERROR: "pt2_1" is not a table
+DROP TABLE pt2;
-- Cleanup
DROP SCHEMA foreign_schema CASCADE;
DROP ROLE regress_test_role; -- ERROR
diff --git a/src/test/regress/sql/foreign_data.sql b/src/test/regress/sql/foreign_data.sql
index 38e1d41a5f..57e38515db 100644
--- a/src/test/regress/sql/foreign_data.sql
+++ b/src/test/regress/sql/foreign_data.sql
@@ -684,6 +684,76 @@ REASSIGN OWNED BY regress_test_role TO regress_test_role2;
DROP OWNED BY regress_test_role2;
DROP OWNED BY regress_test_role2 CASCADE;
+-- Foreign partition DDL stuff
+CREATE TABLE pt2 (
+ c1 integer NOT NULL,
+ c2 text,
+ c3 date
+) PARTITION BY LIST (c1);
+CREATE FOREIGN TABLE pt2_1 PARTITION OF pt2 FOR VALUES IN (1)
+ SERVER s0 OPTIONS (delimiter ',', quote '"', "be quoted" 'value');
+\d+ pt2
+\d+ pt2_1
+
+-- partition cannot have additional columns
+DROP FOREIGN TABLE pt2_1;
+CREATE FOREIGN TABLE pt2_1 (
+ c1 integer NOT NULL,
+ c2 text,
+ c3 date,
+ c4 char
+) SERVER s0 OPTIONS (delimiter ',', quote '"', "be quoted" 'value');
+\d+ pt2_1
+ALTER TABLE pt2 ATTACH PARTITION pt2_1 FOR VALUES IN (1); -- ERROR
+
+DROP FOREIGN TABLE pt2_1;
+\d+ pt2
+CREATE FOREIGN TABLE pt2_1 (
+ c1 integer NOT NULL,
+ c2 text,
+ c3 date
+) SERVER s0 OPTIONS (delimiter ',', quote '"', "be quoted" 'value');
+\d+ pt2_1
+-- no attach partition validation occurs for foreign tables
+ALTER TABLE pt2 ATTACH PARTITION pt2_1 FOR VALUES IN (1);
+\d+ pt2
+\d+ pt2_1
+
+-- cannot add column to a partition
+ALTER TABLE pt2_1 ADD c4 char;
+
+-- ok to have a partition's own constraints though
+ALTER TABLE pt2_1 ALTER c3 SET NOT NULL;
+ALTER TABLE pt2_1 ADD CONSTRAINT p21chk CHECK (c2 <> '');
+\d+ pt2
+\d+ pt2_1
+
+-- cannot drop inherited NOT NULL constraint from a partition
+ALTER TABLE pt2_1 ALTER c1 DROP NOT NULL;
+
+-- partition must have parent's constraints
+ALTER TABLE pt2 DETACH PARTITION pt2_1;
+ALTER TABLE pt2 ALTER c2 SET NOT NULL;
+\d+ pt2
+\d+ pt2_1
+ALTER TABLE pt2 ATTACH PARTITION pt2_1 FOR VALUES IN (1); -- ERROR
+ALTER FOREIGN TABLE pt2_1 ALTER c2 SET NOT NULL;
+ALTER TABLE pt2 ATTACH PARTITION pt2_1 FOR VALUES IN (1);
+
+ALTER TABLE pt2 DETACH PARTITION pt2_1;
+ALTER TABLE pt2 ADD CONSTRAINT pt2chk1 CHECK (c1 > 0);
+\d+ pt2
+\d+ pt2_1
+ALTER TABLE pt2 ATTACH PARTITION pt2_1 FOR VALUES IN (1); -- ERROR
+ALTER FOREIGN TABLE pt2_1 ADD CONSTRAINT pt2chk1 CHECK (c1 > 0);
+ALTER TABLE pt2 ATTACH PARTITION pt2_1 FOR VALUES IN (1);
+
+-- TRUNCATE doesn't work on foreign tables, either directly or recursively
+TRUNCATE pt2_1; -- ERROR
+TRUNCATE pt2; -- ERROR
+
+DROP TABLE pt2;
+
-- Cleanup
DROP SCHEMA foreign_schema CASCADE;
DROP ROLE regress_test_role; -- ERROR
--
2.11.0
On 2017/02/21 15:35, Amit Langote wrote:
drop table list_parted cascade;
-NOTICE: drop cascades to 3 other objects
-DETAIL: drop cascades to table part_ab_cd
probably we should remove cascade from there, unless you are testing CASCADE
functionality. Similarly for other blocks like
drop table range_parted cascade;
Oops, failed to address this in the last email. Updated patches attached.
Thanks,
Amit
Attachments:
0001-Allow-dropping-partitioned-table-without-CASCADE.patchtext/x-diff; name=0001-Allow-dropping-partitioned-table-without-CASCADE.patchDownload
From 0dc550d501805bcea81da9b2a06a39430427927c Mon Sep 17 00:00:00 2001
From: amit <amitlangote09@gmail.com>
Date: Thu, 16 Feb 2017 15:56:44 +0900
Subject: [PATCH 1/2] Allow dropping partitioned table without CASCADE
Currently, a normal dependency is created between a inheritance
parent and child when creating the child. That means one must
specify CASCADE to drop the parent table if a child table exists.
When creating partitions as inheritance children, create auto
dependency instead, so that partitions are dropped automatically
when the parent is dropped i.e., without specifying CASCADE.
---
src/backend/commands/tablecmds.c | 26 ++++++++++++++++++--------
src/test/regress/expected/alter_table.out | 10 ++++------
src/test/regress/expected/create_table.out | 9 ++-------
src/test/regress/expected/inherit.out | 22 ++--------------------
src/test/regress/expected/insert.out | 7 ++-----
src/test/regress/expected/update.out | 7 +------
src/test/regress/sql/alter_table.sql | 10 ++++------
src/test/regress/sql/create_table.sql | 9 ++-------
src/test/regress/sql/inherit.sql | 4 ++--
src/test/regress/sql/insert.sql | 7 ++-----
src/test/regress/sql/update.sql | 2 +-
11 files changed, 40 insertions(+), 73 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 3cea220421..31b50ad77f 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -289,9 +289,11 @@ static List *MergeAttributes(List *schema, List *supers, char relpersistence,
static bool MergeCheckConstraint(List *constraints, char *name, Node *expr);
static void MergeAttributesIntoExisting(Relation child_rel, Relation parent_rel);
static void MergeConstraintsIntoExisting(Relation child_rel, Relation parent_rel);
-static void StoreCatalogInheritance(Oid relationId, List *supers);
+static void StoreCatalogInheritance(Oid relationId, List *supers,
+ bool child_is_partition);
static void StoreCatalogInheritance1(Oid relationId, Oid parentOid,
- int16 seqNumber, Relation inhRelation);
+ int16 seqNumber, Relation inhRelation,
+ bool child_is_partition);
static int findAttrByName(const char *attributeName, List *schema);
static void AlterIndexNamespaces(Relation classRel, Relation rel,
Oid oldNspOid, Oid newNspOid, ObjectAddresses *objsMoved);
@@ -725,7 +727,7 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
typaddress);
/* Store inheritance information for new rel. */
- StoreCatalogInheritance(relationId, inheritOids);
+ StoreCatalogInheritance(relationId, inheritOids, stmt->partbound != NULL);
/*
* We must bump the command counter to make the newly-created relation
@@ -2240,7 +2242,8 @@ MergeCheckConstraint(List *constraints, char *name, Node *expr)
* supers is a list of the OIDs of the new relation's direct ancestors.
*/
static void
-StoreCatalogInheritance(Oid relationId, List *supers)
+StoreCatalogInheritance(Oid relationId, List *supers,
+ bool child_is_partition)
{
Relation relation;
int16 seqNumber;
@@ -2270,7 +2273,8 @@ StoreCatalogInheritance(Oid relationId, List *supers)
{
Oid parentOid = lfirst_oid(entry);
- StoreCatalogInheritance1(relationId, parentOid, seqNumber, relation);
+ StoreCatalogInheritance1(relationId, parentOid, seqNumber, relation,
+ child_is_partition);
seqNumber++;
}
@@ -2283,7 +2287,8 @@ StoreCatalogInheritance(Oid relationId, List *supers)
*/
static void
StoreCatalogInheritance1(Oid relationId, Oid parentOid,
- int16 seqNumber, Relation inhRelation)
+ int16 seqNumber, Relation inhRelation,
+ bool child_is_partition)
{
TupleDesc desc = RelationGetDescr(inhRelation);
Datum values[Natts_pg_inherits];
@@ -2317,7 +2322,10 @@ StoreCatalogInheritance1(Oid relationId, Oid parentOid,
childobject.objectId = relationId;
childobject.objectSubId = 0;
- recordDependencyOn(&childobject, &parentobject, DEPENDENCY_NORMAL);
+ if (!child_is_partition)
+ recordDependencyOn(&childobject, &parentobject, DEPENDENCY_NORMAL);
+ else
+ recordDependencyOn(&childobject, &parentobject, DEPENDENCY_AUTO);
/*
* Post creation hook of this inheritance. Since object_access_hook
@@ -10744,7 +10752,9 @@ CreateInheritance(Relation child_rel, Relation parent_rel)
StoreCatalogInheritance1(RelationGetRelid(child_rel),
RelationGetRelid(parent_rel),
inhseqno + 1,
- catalogRelation);
+ catalogRelation,
+ parent_rel->rd_rel->relkind ==
+ RELKIND_PARTITIONED_TABLE);
/* Now we're done with pg_inherits */
heap_close(catalogRelation, RowExclusiveLock);
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index e84af67fb2..ca66158ee3 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -3339,10 +3339,8 @@ ALTER TABLE list_parted2 DROP COLUMN b;
ERROR: cannot drop column named in partition key
ALTER TABLE list_parted2 ALTER COLUMN b TYPE text;
ERROR: cannot alter type of column named in partition key
--- cleanup: avoid using CASCADE
-DROP TABLE list_parted, part_1;
-DROP TABLE list_parted2, part_2, part_5, part_5_a;
-DROP TABLE range_parted, part1, part2;
+-- cleanup
+DROP TABLE list_parted, list_parted2, range_parted;
-- more tests for certain multi-level partitioning scenarios
create table p (a int, b int) partition by range (a, b);
create table p1 (b int, a int not null) partition by range (b);
@@ -3371,5 +3369,5 @@ insert into p1 (a, b) values (2, 3);
-- check that partition validation scan correctly detects violating rows
alter table p attach partition p1 for values from (1, 2) to (1, 10);
ERROR: partition constraint is violated by some row
--- cleanup: avoid using CASCADE
-drop table p, p1, p11;
+-- cleanup
+drop table p;
diff --git a/src/test/regress/expected/create_table.out b/src/test/regress/expected/create_table.out
index 20eb3d35f9..c07a474b3d 100644
--- a/src/test/regress/expected/create_table.out
+++ b/src/test/regress/expected/create_table.out
@@ -667,10 +667,5 @@ Check constraints:
"check_a" CHECK (length(a) > 0)
Number of partitions: 3 (Use \d+ to list them.)
--- cleanup: avoid using CASCADE
-DROP TABLE parted, part_a, part_b, part_c, part_c_1_10;
-DROP TABLE list_parted, part_1, part_2, part_null;
-DROP TABLE range_parted;
-DROP TABLE list_parted2, part_ab, part_null_z;
-DROP TABLE range_parted2, part0, part1, part2, part3;
-DROP TABLE range_parted3, part00, part10, part11, part12;
+-- cleanup
+DROP TABLE parted, list_parted, range_parted, list_parted2, range_parted2, range_parted3;
diff --git a/src/test/regress/expected/inherit.out b/src/test/regress/expected/inherit.out
index a8c8b28a75..795d9f575c 100644
--- a/src/test/regress/expected/inherit.out
+++ b/src/test/regress/expected/inherit.out
@@ -1843,23 +1843,5 @@ explain (costs off) select * from range_list_parted where a >= 30;
Filter: (a >= 30)
(11 rows)
-drop table list_parted cascade;
-NOTICE: drop cascades to 3 other objects
-DETAIL: drop cascades to table part_ab_cd
-drop cascades to table part_ef_gh
-drop cascades to table part_null_xy
-drop table range_list_parted cascade;
-NOTICE: drop cascades to 13 other objects
-DETAIL: drop cascades to table part_1_10
-drop cascades to table part_1_10_ab
-drop cascades to table part_1_10_cd
-drop cascades to table part_10_20
-drop cascades to table part_10_20_ab
-drop cascades to table part_10_20_cd
-drop cascades to table part_21_30
-drop cascades to table part_21_30_ab
-drop cascades to table part_21_30_cd
-drop cascades to table part_40_inf
-drop cascades to table part_40_inf_ab
-drop cascades to table part_40_inf_cd
-drop cascades to table part_40_inf_null
+drop table list_parted;
+drop table range_list_parted;
diff --git a/src/test/regress/expected/insert.out b/src/test/regress/expected/insert.out
index 81af3ef497..31cfa4e76e 100644
--- a/src/test/regress/expected/insert.out
+++ b/src/test/regress/expected/insert.out
@@ -314,10 +314,7 @@ select tableoid::regclass::text, a, min(b) as min_b, max(b) as max_b from list_p
(9 rows)
-- cleanup
-drop table part1, part2, part3, part4, range_parted;
-drop table part_ee_ff3_1, part_ee_ff3_2, part_ee_ff1, part_ee_ff2, part_ee_ff3;
-drop table part_ee_ff, part_gg2_2, part_gg2_1, part_gg2, part_gg1, part_gg;
-drop table part_aa_bb, part_cc_dd, part_null, list_parted;
+drop table range_parted, list_parted;
-- more tests for certain multi-level partitioning scenarios
create table p (a int, b int) partition by range (a, b);
create table p1 (b int not null, a int not null) partition by range ((b+0));
@@ -387,4 +384,4 @@ with ins (a, b, c) as
(5 rows)
-- cleanup
-drop table p, p1, p11, p12, p2, p3, p4;
+drop table p;
diff --git a/src/test/regress/expected/update.out b/src/test/regress/expected/update.out
index a1e9255450..9366f04255 100644
--- a/src/test/regress/expected/update.out
+++ b/src/test/regress/expected/update.out
@@ -219,9 +219,4 @@ DETAIL: Failing row contains (b, 9).
-- ok
update range_parted set b = b + 1 where b = 10;
-- cleanup
-drop table range_parted cascade;
-NOTICE: drop cascades to 4 other objects
-DETAIL: drop cascades to table part_a_1_a_10
-drop cascades to table part_a_10_a_20
-drop cascades to table part_b_1_b_10
-drop cascades to table part_b_10_b_20
+drop table range_parted;
diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql
index a403fd8cb4..fbcc739f41 100644
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -2199,10 +2199,8 @@ ALTER TABLE part_2 INHERIT inh_test;
ALTER TABLE list_parted2 DROP COLUMN b;
ALTER TABLE list_parted2 ALTER COLUMN b TYPE text;
--- cleanup: avoid using CASCADE
-DROP TABLE list_parted, part_1;
-DROP TABLE list_parted2, part_2, part_5, part_5_a;
-DROP TABLE range_parted, part1, part2;
+-- cleanup
+DROP TABLE list_parted, list_parted2, range_parted;
-- more tests for certain multi-level partitioning scenarios
create table p (a int, b int) partition by range (a, b);
@@ -2227,5 +2225,5 @@ insert into p1 (a, b) values (2, 3);
-- check that partition validation scan correctly detects violating rows
alter table p attach partition p1 for values from (1, 2) to (1, 10);
--- cleanup: avoid using CASCADE
-drop table p, p1, p11;
+-- cleanup
+drop table p;
diff --git a/src/test/regress/sql/create_table.sql b/src/test/regress/sql/create_table.sql
index f41dd71475..1f0fa8e16d 100644
--- a/src/test/regress/sql/create_table.sql
+++ b/src/test/regress/sql/create_table.sql
@@ -595,10 +595,5 @@ CREATE TABLE part_c_1_10 PARTITION OF part_c FOR VALUES FROM (1) TO (10);
-- returned.
\d parted
--- cleanup: avoid using CASCADE
-DROP TABLE parted, part_a, part_b, part_c, part_c_1_10;
-DROP TABLE list_parted, part_1, part_2, part_null;
-DROP TABLE range_parted;
-DROP TABLE list_parted2, part_ab, part_null_z;
-DROP TABLE range_parted2, part0, part1, part2, part3;
-DROP TABLE range_parted3, part00, part10, part11, part12;
+-- cleanup
+DROP TABLE parted, list_parted, range_parted, list_parted2, range_parted2, range_parted3;
diff --git a/src/test/regress/sql/inherit.sql b/src/test/regress/sql/inherit.sql
index a8b7eb1c8d..836ec22c20 100644
--- a/src/test/regress/sql/inherit.sql
+++ b/src/test/regress/sql/inherit.sql
@@ -612,5 +612,5 @@ explain (costs off) select * from range_list_parted where b is null;
explain (costs off) select * from range_list_parted where a is not null and a < 67;
explain (costs off) select * from range_list_parted where a >= 30;
-drop table list_parted cascade;
-drop table range_list_parted cascade;
+drop table list_parted;
+drop table range_list_parted;
diff --git a/src/test/regress/sql/insert.sql b/src/test/regress/sql/insert.sql
index 454e1ce2e7..dfdc24eba8 100644
--- a/src/test/regress/sql/insert.sql
+++ b/src/test/regress/sql/insert.sql
@@ -186,10 +186,7 @@ insert into list_parted (b) values (1);
select tableoid::regclass::text, a, min(b) as min_b, max(b) as max_b from list_parted group by 1, 2 order by 1;
-- cleanup
-drop table part1, part2, part3, part4, range_parted;
-drop table part_ee_ff3_1, part_ee_ff3_2, part_ee_ff1, part_ee_ff2, part_ee_ff3;
-drop table part_ee_ff, part_gg2_2, part_gg2_1, part_gg2, part_gg1, part_gg;
-drop table part_aa_bb, part_cc_dd, part_null, list_parted;
+drop table range_parted, list_parted;
-- more tests for certain multi-level partitioning scenarios
create table p (a int, b int) partition by range (a, b);
@@ -241,4 +238,4 @@ with ins (a, b, c) as
select a, b, min(c), max(c) from ins group by a, b order by 1;
-- cleanup
-drop table p, p1, p11, p12, p2, p3, p4;
+drop table p;
diff --git a/src/test/regress/sql/update.sql b/src/test/regress/sql/update.sql
index d7721ed376..663711997b 100644
--- a/src/test/regress/sql/update.sql
+++ b/src/test/regress/sql/update.sql
@@ -126,4 +126,4 @@ update range_parted set b = b - 1 where b = 10;
update range_parted set b = b + 1 where b = 10;
-- cleanup
-drop table range_parted cascade;
+drop table range_parted;
--
2.11.0
0002-Add-regression-tests-foreign-partition-DDL.patchtext/x-diff; name=0002-Add-regression-tests-foreign-partition-DDL.patchDownload
From 6eb41841b9d72720807bfdb60517b0d7a7a71724 Mon Sep 17 00:00:00 2001
From: amit <amitlangote09@gmail.com>
Date: Tue, 21 Feb 2017 15:06:04 +0900
Subject: [PATCH 2/2] Add regression tests foreign partition DDL
Commands like CREATE FOREIGN TABLE .. PARTITION OF, ATTACH PARTITION,
DETACH PARTITION foreign_table didn't get any tests so far. Per
suggestion from Ashutosh Bapat.
---
src/test/regress/expected/foreign_data.out | 194 +++++++++++++++++++++++++++++
src/test/regress/sql/foreign_data.sql | 70 +++++++++++
2 files changed, 264 insertions(+)
diff --git a/src/test/regress/expected/foreign_data.out b/src/test/regress/expected/foreign_data.out
index 3a9fb8f558..541f2eefe1 100644
--- a/src/test/regress/expected/foreign_data.out
+++ b/src/test/regress/expected/foreign_data.out
@@ -1751,6 +1751,200 @@ DETAIL: user mapping for regress_test_role on server s5 depends on server s5
HINT: Use DROP ... CASCADE to drop the dependent objects too.
DROP OWNED BY regress_test_role2 CASCADE;
NOTICE: drop cascades to user mapping for regress_test_role on server s5
+-- Foreign partition DDL stuff
+CREATE TABLE pt2 (
+ c1 integer NOT NULL,
+ c2 text,
+ c3 date
+) PARTITION BY LIST (c1);
+CREATE FOREIGN TABLE pt2_1 PARTITION OF pt2 FOR VALUES IN (1)
+ SERVER s0 OPTIONS (delimiter ',', quote '"', "be quoted" 'value');
+\d+ pt2
+ Table "public.pt2"
+ Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
+--------+---------+-----------+----------+---------+----------+--------------+-------------
+ c1 | integer | | not null | | plain | |
+ c2 | text | | | | extended | |
+ c3 | date | | | | plain | |
+Partition key: LIST (c1)
+Partitions: pt2_1 FOR VALUES IN (1)
+
+\d+ pt2_1
+ Foreign table "public.pt2_1"
+ Column | Type | Collation | Nullable | Default | FDW Options | Storage | Stats target | Description
+--------+---------+-----------+----------+---------+-------------+----------+--------------+-------------
+ c1 | integer | | not null | | | plain | |
+ c2 | text | | | | | extended | |
+ c3 | date | | | | | plain | |
+Partition of: pt2 FOR VALUES IN (1)
+Server: s0
+FDW Options: (delimiter ',', quote '"', "be quoted" 'value')
+
+-- partition cannot have additional columns
+DROP FOREIGN TABLE pt2_1;
+CREATE FOREIGN TABLE pt2_1 (
+ c1 integer NOT NULL,
+ c2 text,
+ c3 date,
+ c4 char
+) SERVER s0 OPTIONS (delimiter ',', quote '"', "be quoted" 'value');
+\d+ pt2_1
+ Foreign table "public.pt2_1"
+ Column | Type | Collation | Nullable | Default | FDW Options | Storage | Stats target | Description
+--------+--------------+-----------+----------+---------+-------------+----------+--------------+-------------
+ c1 | integer | | not null | | | plain | |
+ c2 | text | | | | | extended | |
+ c3 | date | | | | | plain | |
+ c4 | character(1) | | | | | extended | |
+Server: s0
+FDW Options: (delimiter ',', quote '"', "be quoted" 'value')
+
+ALTER TABLE pt2 ATTACH PARTITION pt2_1 FOR VALUES IN (1); -- ERROR
+ERROR: table "pt2_1" contains column "c4" not found in parent "pt2"
+DETAIL: New partition should contain only the columns present in parent.
+DROP FOREIGN TABLE pt2_1;
+\d+ pt2
+ Table "public.pt2"
+ Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
+--------+---------+-----------+----------+---------+----------+--------------+-------------
+ c1 | integer | | not null | | plain | |
+ c2 | text | | | | extended | |
+ c3 | date | | | | plain | |
+Partition key: LIST (c1)
+
+CREATE FOREIGN TABLE pt2_1 (
+ c1 integer NOT NULL,
+ c2 text,
+ c3 date
+) SERVER s0 OPTIONS (delimiter ',', quote '"', "be quoted" 'value');
+\d+ pt2_1
+ Foreign table "public.pt2_1"
+ Column | Type | Collation | Nullable | Default | FDW Options | Storage | Stats target | Description
+--------+---------+-----------+----------+---------+-------------+----------+--------------+-------------
+ c1 | integer | | not null | | | plain | |
+ c2 | text | | | | | extended | |
+ c3 | date | | | | | plain | |
+Server: s0
+FDW Options: (delimiter ',', quote '"', "be quoted" 'value')
+
+-- no attach partition validation occurs for foreign tables
+ALTER TABLE pt2 ATTACH PARTITION pt2_1 FOR VALUES IN (1);
+\d+ pt2
+ Table "public.pt2"
+ Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
+--------+---------+-----------+----------+---------+----------+--------------+-------------
+ c1 | integer | | not null | | plain | |
+ c2 | text | | | | extended | |
+ c3 | date | | | | plain | |
+Partition key: LIST (c1)
+Partitions: pt2_1 FOR VALUES IN (1)
+
+\d+ pt2_1
+ Foreign table "public.pt2_1"
+ Column | Type | Collation | Nullable | Default | FDW Options | Storage | Stats target | Description
+--------+---------+-----------+----------+---------+-------------+----------+--------------+-------------
+ c1 | integer | | not null | | | plain | |
+ c2 | text | | | | | extended | |
+ c3 | date | | | | | plain | |
+Partition of: pt2 FOR VALUES IN (1)
+Server: s0
+FDW Options: (delimiter ',', quote '"', "be quoted" 'value')
+
+-- cannot add column to a partition
+ALTER TABLE pt2_1 ADD c4 char;
+ERROR: cannot add column to a partition
+-- ok to have a partition's own constraints though
+ALTER TABLE pt2_1 ALTER c3 SET NOT NULL;
+ALTER TABLE pt2_1 ADD CONSTRAINT p21chk CHECK (c2 <> '');
+\d+ pt2
+ Table "public.pt2"
+ Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
+--------+---------+-----------+----------+---------+----------+--------------+-------------
+ c1 | integer | | not null | | plain | |
+ c2 | text | | | | extended | |
+ c3 | date | | | | plain | |
+Partition key: LIST (c1)
+Partitions: pt2_1 FOR VALUES IN (1)
+
+\d+ pt2_1
+ Foreign table "public.pt2_1"
+ Column | Type | Collation | Nullable | Default | FDW Options | Storage | Stats target | Description
+--------+---------+-----------+----------+---------+-------------+----------+--------------+-------------
+ c1 | integer | | not null | | | plain | |
+ c2 | text | | | | | extended | |
+ c3 | date | | not null | | | plain | |
+Partition of: pt2 FOR VALUES IN (1)
+Check constraints:
+ "p21chk" CHECK (c2 <> ''::text)
+Server: s0
+FDW Options: (delimiter ',', quote '"', "be quoted" 'value')
+
+-- cannot drop inherited NOT NULL constraint from a partition
+ALTER TABLE pt2_1 ALTER c1 DROP NOT NULL;
+ERROR: column "c1" is marked NOT NULL in parent table
+-- partition must have parent's constraints
+ALTER TABLE pt2 DETACH PARTITION pt2_1;
+ALTER TABLE pt2 ALTER c2 SET NOT NULL;
+\d+ pt2
+ Table "public.pt2"
+ Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
+--------+---------+-----------+----------+---------+----------+--------------+-------------
+ c1 | integer | | not null | | plain | |
+ c2 | text | | not null | | extended | |
+ c3 | date | | | | plain | |
+Partition key: LIST (c1)
+
+\d+ pt2_1
+ Foreign table "public.pt2_1"
+ Column | Type | Collation | Nullable | Default | FDW Options | Storage | Stats target | Description
+--------+---------+-----------+----------+---------+-------------+----------+--------------+-------------
+ c1 | integer | | not null | | | plain | |
+ c2 | text | | | | | extended | |
+ c3 | date | | not null | | | plain | |
+Check constraints:
+ "p21chk" CHECK (c2 <> ''::text)
+Server: s0
+FDW Options: (delimiter ',', quote '"', "be quoted" 'value')
+
+ALTER TABLE pt2 ATTACH PARTITION pt2_1 FOR VALUES IN (1); -- ERROR
+ERROR: column "c2" in child table must be marked NOT NULL
+ALTER FOREIGN TABLE pt2_1 ALTER c2 SET NOT NULL;
+ALTER TABLE pt2 ATTACH PARTITION pt2_1 FOR VALUES IN (1);
+ALTER TABLE pt2 DETACH PARTITION pt2_1;
+ALTER TABLE pt2 ADD CONSTRAINT pt2chk1 CHECK (c1 > 0);
+\d+ pt2
+ Table "public.pt2"
+ Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
+--------+---------+-----------+----------+---------+----------+--------------+-------------
+ c1 | integer | | not null | | plain | |
+ c2 | text | | not null | | extended | |
+ c3 | date | | | | plain | |
+Partition key: LIST (c1)
+Check constraints:
+ "pt2chk1" CHECK (c1 > 0)
+
+\d+ pt2_1
+ Foreign table "public.pt2_1"
+ Column | Type | Collation | Nullable | Default | FDW Options | Storage | Stats target | Description
+--------+---------+-----------+----------+---------+-------------+----------+--------------+-------------
+ c1 | integer | | not null | | | plain | |
+ c2 | text | | not null | | | extended | |
+ c3 | date | | not null | | | plain | |
+Check constraints:
+ "p21chk" CHECK (c2 <> ''::text)
+Server: s0
+FDW Options: (delimiter ',', quote '"', "be quoted" 'value')
+
+ALTER TABLE pt2 ATTACH PARTITION pt2_1 FOR VALUES IN (1); -- ERROR
+ERROR: child table is missing constraint "pt2chk1"
+ALTER FOREIGN TABLE pt2_1 ADD CONSTRAINT pt2chk1 CHECK (c1 > 0);
+ALTER TABLE pt2 ATTACH PARTITION pt2_1 FOR VALUES IN (1);
+-- TRUNCATE doesn't work on foreign tables, either directly or recursively
+TRUNCATE pt2_1; -- ERROR
+ERROR: "pt2_1" is not a table
+TRUNCATE pt2; -- ERROR
+ERROR: "pt2_1" is not a table
+DROP TABLE pt2;
-- Cleanup
DROP SCHEMA foreign_schema CASCADE;
DROP ROLE regress_test_role; -- ERROR
diff --git a/src/test/regress/sql/foreign_data.sql b/src/test/regress/sql/foreign_data.sql
index 38e1d41a5f..57e38515db 100644
--- a/src/test/regress/sql/foreign_data.sql
+++ b/src/test/regress/sql/foreign_data.sql
@@ -684,6 +684,76 @@ REASSIGN OWNED BY regress_test_role TO regress_test_role2;
DROP OWNED BY regress_test_role2;
DROP OWNED BY regress_test_role2 CASCADE;
+-- Foreign partition DDL stuff
+CREATE TABLE pt2 (
+ c1 integer NOT NULL,
+ c2 text,
+ c3 date
+) PARTITION BY LIST (c1);
+CREATE FOREIGN TABLE pt2_1 PARTITION OF pt2 FOR VALUES IN (1)
+ SERVER s0 OPTIONS (delimiter ',', quote '"', "be quoted" 'value');
+\d+ pt2
+\d+ pt2_1
+
+-- partition cannot have additional columns
+DROP FOREIGN TABLE pt2_1;
+CREATE FOREIGN TABLE pt2_1 (
+ c1 integer NOT NULL,
+ c2 text,
+ c3 date,
+ c4 char
+) SERVER s0 OPTIONS (delimiter ',', quote '"', "be quoted" 'value');
+\d+ pt2_1
+ALTER TABLE pt2 ATTACH PARTITION pt2_1 FOR VALUES IN (1); -- ERROR
+
+DROP FOREIGN TABLE pt2_1;
+\d+ pt2
+CREATE FOREIGN TABLE pt2_1 (
+ c1 integer NOT NULL,
+ c2 text,
+ c3 date
+) SERVER s0 OPTIONS (delimiter ',', quote '"', "be quoted" 'value');
+\d+ pt2_1
+-- no attach partition validation occurs for foreign tables
+ALTER TABLE pt2 ATTACH PARTITION pt2_1 FOR VALUES IN (1);
+\d+ pt2
+\d+ pt2_1
+
+-- cannot add column to a partition
+ALTER TABLE pt2_1 ADD c4 char;
+
+-- ok to have a partition's own constraints though
+ALTER TABLE pt2_1 ALTER c3 SET NOT NULL;
+ALTER TABLE pt2_1 ADD CONSTRAINT p21chk CHECK (c2 <> '');
+\d+ pt2
+\d+ pt2_1
+
+-- cannot drop inherited NOT NULL constraint from a partition
+ALTER TABLE pt2_1 ALTER c1 DROP NOT NULL;
+
+-- partition must have parent's constraints
+ALTER TABLE pt2 DETACH PARTITION pt2_1;
+ALTER TABLE pt2 ALTER c2 SET NOT NULL;
+\d+ pt2
+\d+ pt2_1
+ALTER TABLE pt2 ATTACH PARTITION pt2_1 FOR VALUES IN (1); -- ERROR
+ALTER FOREIGN TABLE pt2_1 ALTER c2 SET NOT NULL;
+ALTER TABLE pt2 ATTACH PARTITION pt2_1 FOR VALUES IN (1);
+
+ALTER TABLE pt2 DETACH PARTITION pt2_1;
+ALTER TABLE pt2 ADD CONSTRAINT pt2chk1 CHECK (c1 > 0);
+\d+ pt2
+\d+ pt2_1
+ALTER TABLE pt2 ATTACH PARTITION pt2_1 FOR VALUES IN (1); -- ERROR
+ALTER FOREIGN TABLE pt2_1 ADD CONSTRAINT pt2chk1 CHECK (c1 > 0);
+ALTER TABLE pt2 ATTACH PARTITION pt2_1 FOR VALUES IN (1);
+
+-- TRUNCATE doesn't work on foreign tables, either directly or recursively
+TRUNCATE pt2_1; -- ERROR
+TRUNCATE pt2; -- ERROR
+
+DROP TABLE pt2;
+
-- Cleanup
DROP SCHEMA foreign_schema CASCADE;
DROP ROLE regress_test_role; -- ERROR
--
2.11.0
On Tue, Feb 21, 2017 at 12:05 PM, Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:
Hi Ashutosh,
Thanks for taking a look at the patch.
On 2017/02/20 21:49, Ashutosh Bapat wrote:
Thanks for working on all the follow on work for partitioning feature.
May be you should add all those patches in the next commitfest, so
that we don't forget those.I think adding these as one of the PostgreSQL 10 Open Items [0] might be
better. I've done that.On Mon, Feb 20, 2017 at 7:46 AM, Amit Langote wrote:
So I count more than a few votes saying that we should be able to DROP
partitioned tables without specifying CASCADE.I tried to implement that using the attached patch by having
StoreCatalogInheritance1() create DEPENDENCY_AUTO dependency between
parent and child if the child is a partition, instead of DEPENDENCY_NORMAL
that would otherwise be created. Now it seems that that is one way of
making sure that partitions are dropped when the root partitioned table is
dropped, not sure if the best; why create the pg_depend entries at all one
might ask. I chose it for now because that's the one with fewest lines of
change. Adjusted regression tests as well, since we recently tweaked
tests [1] to work around the irregularities of test output when using CASCADE.The patch applies cleanly and regression does not show any failures.
Here are some comments
For the sake of readability you may want reverse the if and else order. - recordDependencyOn(&childobject, &parentobject, DEPENDENCY_NORMAL); + if (!child_is_partition) + recordDependencyOn(&childobject, &parentobject, DEPENDENCY_NORMAL); + else + recordDependencyOn(&childobject, &parentobject, DEPENDENCY_AUTO); like + if (child_is_partition) + recordDependencyOn(&childobject, &parentobject, DEPENDENCY_AUTO); + else + recordDependencyOn(&childobject, &parentobject, DEPENDENCY_NORMAL);Sure, done.
I still see
- recordDependencyOn(&childobject, &parentobject, DEPENDENCY_NORMAL);
+ if (!child_is_partition)
+ recordDependencyOn(&childobject, &parentobject, DEPENDENCY_NORMAL);
+ else
+ recordDependencyOn(&childobject, &parentobject, DEPENDENCY_AUTO);Are you sure you have attached the right patch?
To avoid duplication you could actually write
recordDependencyOn(&childobject, &parentobject,
child_is_partition ?
DEPENDENCY_AUTO : DEPENDENCY_NORMAL);
It's weird that somebody can perform DROP TABLE on the partition without
referring to its parent. That may be a useful feature as it allows one to
detach the partition as well as remove the table in one command. But it looks
wierd for someone familiar with partitioning features of other DBMSes. But then
our partition creation command is CREATE TABLE .... So may be this is expected
difference.There is a line on the CREATE TABLE page in the description of PARTITION
OF clause:"Note that dropping a partition with DROP TABLE requires taking an ACCESS
EXCLUSIVE lock on the parent table."In earlier proposals I had included the ALTER TABLE parent ADD/DROP
PARTITION commands, but CRAETE TABLE PARTITION OF / DROP TABLE prevailed.
Ok.
--- cleanup: avoid using CASCADE -DROP TABLE list_parted, part_1; -DROP TABLE list_parted2, part_2, part_5, part_5_a; -DROP TABLE range_parted, part1, part2; +-- cleanup +DROP TABLE list_parted, list_parted2, range_parted; Testcases usually drop one table at a time, I guess, to reduce the differences when we add or remove tables from testcases. All such blocks should probably follow same policy.Hmm, I see this in src/test/regress/sql/inherit.sql:141
DROP TABLE firstparent, secondparent, jointchild, thirdparent, otherchild;
Hmm, I can spot some more such usages. Let's keep this for the
committer to decide.
drop table list_parted cascade;
-NOTICE: drop cascades to 3 other objects
-DETAIL: drop cascades to table part_ab_cd
probably we should remove cascade from there, unless you are testing CASCADE
functionality. Similarly for other blocks like
drop table range_parted cascade;BTW, I noticed that although we are allowing foreign tables to be
partitions, there are no tests in foreign_data.sql for testing it. If
there would have been we would tests DROP TABLE on a partitioned table
with foreign partitions as well. That file has testcases for testing
foreign table inheritance, and should have tests for foreign table
partitions.That makes sense. Patch 0002 is for that (I'm afraid this should be
posted separately though). I didn't add/repeat all the tests that were
added by the foreign table inheritance patch again for foreign partitions
(common inheritance rules apply to both cases), only added those for the
new partitioning commands and certain new rules.
Thanks. Yes, a separate thread would do. I will review it there. May
be you want to add it to the commitfest too.
--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 2017/02/21 20:17, Ashutosh Bapat wrote:
On Tue, Feb 21, 2017 at 12:05 PM, Amit Langote wrote:
On 2017/02/20 21:49, Ashutosh Bapat wrote:
Here are some comments
For the sake of readability you may want reverse the if and else order. - recordDependencyOn(&childobject, &parentobject, DEPENDENCY_NORMAL); + if (!child_is_partition) + recordDependencyOn(&childobject, &parentobject, DEPENDENCY_NORMAL); + else + recordDependencyOn(&childobject, &parentobject, DEPENDENCY_AUTO); like + if (child_is_partition) + recordDependencyOn(&childobject, &parentobject, DEPENDENCY_AUTO); + else + recordDependencyOn(&childobject, &parentobject, DEPENDENCY_NORMAL);Sure, done.
I still see - recordDependencyOn(&childobject, &parentobject, DEPENDENCY_NORMAL); + if (!child_is_partition) + recordDependencyOn(&childobject, &parentobject, DEPENDENCY_NORMAL); + else + recordDependencyOn(&childobject, &parentobject, DEPENDENCY_AUTO);Are you sure you have attached the right patch?
Oops, really fixed this time.
--- cleanup: avoid using CASCADE -DROP TABLE list_parted, part_1; -DROP TABLE list_parted2, part_2, part_5, part_5_a; -DROP TABLE range_parted, part1, part2; +-- cleanup +DROP TABLE list_parted, list_parted2, range_parted; Testcases usually drop one table at a time, I guess, to reduce the differences when we add or remove tables from testcases. All such blocks should probably follow same policy.Hmm, I see this in src/test/regress/sql/inherit.sql:141
DROP TABLE firstparent, secondparent, jointchild, thirdparent, otherchild;
Hmm, I can spot some more such usages. Let's keep this for the
committer to decide.
Sure.
BTW, I noticed that although we are allowing foreign tables to be
partitions, there are no tests in foreign_data.sql for testing it. If
there would have been we would tests DROP TABLE on a partitioned table
with foreign partitions as well. That file has testcases for testing
foreign table inheritance, and should have tests for foreign table
partitions.That makes sense. Patch 0002 is for that (I'm afraid this should be
posted separately though). I didn't add/repeat all the tests that were
added by the foreign table inheritance patch again for foreign partitions
(common inheritance rules apply to both cases), only added those for the
new partitioning commands and certain new rules.Thanks. Yes, a separate thread would do. I will review it there. May
be you want to add it to the commitfest too.
Posted in a new thread titled "foreign partition DDL regression tests".
Thanks,
Amit
Attachments:
0001-Allow-dropping-partitioned-table-without-CASCADE.patchtext/x-diff; name=0001-Allow-dropping-partitioned-table-without-CASCADE.patchDownload
From 525da2525b69e5191abac68f074d1d36c78bef8c Mon Sep 17 00:00:00 2001
From: amit <amitlangote09@gmail.com>
Date: Thu, 16 Feb 2017 15:56:44 +0900
Subject: [PATCH] Allow dropping partitioned table without CASCADE
Currently, a normal dependency is created between a inheritance
parent and child when creating the child. That means one must
specify CASCADE to drop the parent table if a child table exists.
When creating partitions as inheritance children, create auto
dependency instead, so that partitions are dropped automatically
when the parent is dropped i.e., without specifying CASCADE.
---
src/backend/commands/tablecmds.c | 26 ++++++++++++++++++--------
src/test/regress/expected/alter_table.out | 10 ++++------
src/test/regress/expected/create_table.out | 9 ++-------
src/test/regress/expected/inherit.out | 22 ++--------------------
src/test/regress/expected/insert.out | 7 ++-----
src/test/regress/expected/update.out | 7 +------
src/test/regress/sql/alter_table.sql | 10 ++++------
src/test/regress/sql/create_table.sql | 9 ++-------
src/test/regress/sql/inherit.sql | 4 ++--
src/test/regress/sql/insert.sql | 7 ++-----
src/test/regress/sql/update.sql | 2 +-
11 files changed, 40 insertions(+), 73 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 3cea220421..31b50ad77f 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -289,9 +289,11 @@ static List *MergeAttributes(List *schema, List *supers, char relpersistence,
static bool MergeCheckConstraint(List *constraints, char *name, Node *expr);
static void MergeAttributesIntoExisting(Relation child_rel, Relation parent_rel);
static void MergeConstraintsIntoExisting(Relation child_rel, Relation parent_rel);
-static void StoreCatalogInheritance(Oid relationId, List *supers);
+static void StoreCatalogInheritance(Oid relationId, List *supers,
+ bool child_is_partition);
static void StoreCatalogInheritance1(Oid relationId, Oid parentOid,
- int16 seqNumber, Relation inhRelation);
+ int16 seqNumber, Relation inhRelation,
+ bool child_is_partition);
static int findAttrByName(const char *attributeName, List *schema);
static void AlterIndexNamespaces(Relation classRel, Relation rel,
Oid oldNspOid, Oid newNspOid, ObjectAddresses *objsMoved);
@@ -725,7 +727,7 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
typaddress);
/* Store inheritance information for new rel. */
- StoreCatalogInheritance(relationId, inheritOids);
+ StoreCatalogInheritance(relationId, inheritOids, stmt->partbound != NULL);
/*
* We must bump the command counter to make the newly-created relation
@@ -2240,7 +2242,8 @@ MergeCheckConstraint(List *constraints, char *name, Node *expr)
* supers is a list of the OIDs of the new relation's direct ancestors.
*/
static void
-StoreCatalogInheritance(Oid relationId, List *supers)
+StoreCatalogInheritance(Oid relationId, List *supers,
+ bool child_is_partition)
{
Relation relation;
int16 seqNumber;
@@ -2270,7 +2273,8 @@ StoreCatalogInheritance(Oid relationId, List *supers)
{
Oid parentOid = lfirst_oid(entry);
- StoreCatalogInheritance1(relationId, parentOid, seqNumber, relation);
+ StoreCatalogInheritance1(relationId, parentOid, seqNumber, relation,
+ child_is_partition);
seqNumber++;
}
@@ -2283,7 +2287,8 @@ StoreCatalogInheritance(Oid relationId, List *supers)
*/
static void
StoreCatalogInheritance1(Oid relationId, Oid parentOid,
- int16 seqNumber, Relation inhRelation)
+ int16 seqNumber, Relation inhRelation,
+ bool child_is_partition)
{
TupleDesc desc = RelationGetDescr(inhRelation);
Datum values[Natts_pg_inherits];
@@ -2317,7 +2322,10 @@ StoreCatalogInheritance1(Oid relationId, Oid parentOid,
childobject.objectId = relationId;
childobject.objectSubId = 0;
- recordDependencyOn(&childobject, &parentobject, DEPENDENCY_NORMAL);
+ if (!child_is_partition)
+ recordDependencyOn(&childobject, &parentobject, DEPENDENCY_NORMAL);
+ else
+ recordDependencyOn(&childobject, &parentobject, DEPENDENCY_AUTO);
/*
* Post creation hook of this inheritance. Since object_access_hook
@@ -10744,7 +10752,9 @@ CreateInheritance(Relation child_rel, Relation parent_rel)
StoreCatalogInheritance1(RelationGetRelid(child_rel),
RelationGetRelid(parent_rel),
inhseqno + 1,
- catalogRelation);
+ catalogRelation,
+ parent_rel->rd_rel->relkind ==
+ RELKIND_PARTITIONED_TABLE);
/* Now we're done with pg_inherits */
heap_close(catalogRelation, RowExclusiveLock);
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index 9885fcba89..c15bbdcbd1 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -3339,10 +3339,8 @@ ALTER TABLE list_parted2 DROP COLUMN b;
ERROR: cannot drop column named in partition key
ALTER TABLE list_parted2 ALTER COLUMN b TYPE text;
ERROR: cannot alter type of column named in partition key
--- cleanup: avoid using CASCADE
-DROP TABLE list_parted, part_1;
-DROP TABLE list_parted2, part_2, part_5, part_5_a;
-DROP TABLE range_parted, part1, part2;
+-- cleanup
+DROP TABLE list_parted, list_parted2, range_parted;
-- more tests for certain multi-level partitioning scenarios
create table p (a int, b int) partition by range (a, b);
create table p1 (b int, a int not null) partition by range (b);
@@ -3371,5 +3369,5 @@ insert into p1 (a, b) values (2, 3);
-- check that partition validation scan correctly detects violating rows
alter table p attach partition p1 for values from (1, 2) to (1, 10);
ERROR: partition constraint is violated by some row
--- cleanup: avoid using CASCADE
-drop table p, p1, p11;
+-- cleanup
+drop table p;
diff --git a/src/test/regress/expected/create_table.out b/src/test/regress/expected/create_table.out
index 20eb3d35f9..c07a474b3d 100644
--- a/src/test/regress/expected/create_table.out
+++ b/src/test/regress/expected/create_table.out
@@ -667,10 +667,5 @@ Check constraints:
"check_a" CHECK (length(a) > 0)
Number of partitions: 3 (Use \d+ to list them.)
--- cleanup: avoid using CASCADE
-DROP TABLE parted, part_a, part_b, part_c, part_c_1_10;
-DROP TABLE list_parted, part_1, part_2, part_null;
-DROP TABLE range_parted;
-DROP TABLE list_parted2, part_ab, part_null_z;
-DROP TABLE range_parted2, part0, part1, part2, part3;
-DROP TABLE range_parted3, part00, part10, part11, part12;
+-- cleanup
+DROP TABLE parted, list_parted, range_parted, list_parted2, range_parted2, range_parted3;
diff --git a/src/test/regress/expected/inherit.out b/src/test/regress/expected/inherit.out
index a8c8b28a75..795d9f575c 100644
--- a/src/test/regress/expected/inherit.out
+++ b/src/test/regress/expected/inherit.out
@@ -1843,23 +1843,5 @@ explain (costs off) select * from range_list_parted where a >= 30;
Filter: (a >= 30)
(11 rows)
-drop table list_parted cascade;
-NOTICE: drop cascades to 3 other objects
-DETAIL: drop cascades to table part_ab_cd
-drop cascades to table part_ef_gh
-drop cascades to table part_null_xy
-drop table range_list_parted cascade;
-NOTICE: drop cascades to 13 other objects
-DETAIL: drop cascades to table part_1_10
-drop cascades to table part_1_10_ab
-drop cascades to table part_1_10_cd
-drop cascades to table part_10_20
-drop cascades to table part_10_20_ab
-drop cascades to table part_10_20_cd
-drop cascades to table part_21_30
-drop cascades to table part_21_30_ab
-drop cascades to table part_21_30_cd
-drop cascades to table part_40_inf
-drop cascades to table part_40_inf_ab
-drop cascades to table part_40_inf_cd
-drop cascades to table part_40_inf_null
+drop table list_parted;
+drop table range_list_parted;
diff --git a/src/test/regress/expected/insert.out b/src/test/regress/expected/insert.out
index 81af3ef497..31cfa4e76e 100644
--- a/src/test/regress/expected/insert.out
+++ b/src/test/regress/expected/insert.out
@@ -314,10 +314,7 @@ select tableoid::regclass::text, a, min(b) as min_b, max(b) as max_b from list_p
(9 rows)
-- cleanup
-drop table part1, part2, part3, part4, range_parted;
-drop table part_ee_ff3_1, part_ee_ff3_2, part_ee_ff1, part_ee_ff2, part_ee_ff3;
-drop table part_ee_ff, part_gg2_2, part_gg2_1, part_gg2, part_gg1, part_gg;
-drop table part_aa_bb, part_cc_dd, part_null, list_parted;
+drop table range_parted, list_parted;
-- more tests for certain multi-level partitioning scenarios
create table p (a int, b int) partition by range (a, b);
create table p1 (b int not null, a int not null) partition by range ((b+0));
@@ -387,4 +384,4 @@ with ins (a, b, c) as
(5 rows)
-- cleanup
-drop table p, p1, p11, p12, p2, p3, p4;
+drop table p;
diff --git a/src/test/regress/expected/update.out b/src/test/regress/expected/update.out
index a1e9255450..9366f04255 100644
--- a/src/test/regress/expected/update.out
+++ b/src/test/regress/expected/update.out
@@ -219,9 +219,4 @@ DETAIL: Failing row contains (b, 9).
-- ok
update range_parted set b = b + 1 where b = 10;
-- cleanup
-drop table range_parted cascade;
-NOTICE: drop cascades to 4 other objects
-DETAIL: drop cascades to table part_a_1_a_10
-drop cascades to table part_a_10_a_20
-drop cascades to table part_b_1_b_10
-drop cascades to table part_b_10_b_20
+drop table range_parted;
diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql
index f7b754f0be..37f327bf6d 100644
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -2199,10 +2199,8 @@ ALTER TABLE part_2 INHERIT inh_test;
ALTER TABLE list_parted2 DROP COLUMN b;
ALTER TABLE list_parted2 ALTER COLUMN b TYPE text;
--- cleanup: avoid using CASCADE
-DROP TABLE list_parted, part_1;
-DROP TABLE list_parted2, part_2, part_5, part_5_a;
-DROP TABLE range_parted, part1, part2;
+-- cleanup
+DROP TABLE list_parted, list_parted2, range_parted;
-- more tests for certain multi-level partitioning scenarios
create table p (a int, b int) partition by range (a, b);
@@ -2227,5 +2225,5 @@ insert into p1 (a, b) values (2, 3);
-- check that partition validation scan correctly detects violating rows
alter table p attach partition p1 for values from (1, 2) to (1, 10);
--- cleanup: avoid using CASCADE
-drop table p, p1, p11;
+-- cleanup
+drop table p;
diff --git a/src/test/regress/sql/create_table.sql b/src/test/regress/sql/create_table.sql
index f41dd71475..1f0fa8e16d 100644
--- a/src/test/regress/sql/create_table.sql
+++ b/src/test/regress/sql/create_table.sql
@@ -595,10 +595,5 @@ CREATE TABLE part_c_1_10 PARTITION OF part_c FOR VALUES FROM (1) TO (10);
-- returned.
\d parted
--- cleanup: avoid using CASCADE
-DROP TABLE parted, part_a, part_b, part_c, part_c_1_10;
-DROP TABLE list_parted, part_1, part_2, part_null;
-DROP TABLE range_parted;
-DROP TABLE list_parted2, part_ab, part_null_z;
-DROP TABLE range_parted2, part0, part1, part2, part3;
-DROP TABLE range_parted3, part00, part10, part11, part12;
+-- cleanup
+DROP TABLE parted, list_parted, range_parted, list_parted2, range_parted2, range_parted3;
diff --git a/src/test/regress/sql/inherit.sql b/src/test/regress/sql/inherit.sql
index a8b7eb1c8d..836ec22c20 100644
--- a/src/test/regress/sql/inherit.sql
+++ b/src/test/regress/sql/inherit.sql
@@ -612,5 +612,5 @@ explain (costs off) select * from range_list_parted where b is null;
explain (costs off) select * from range_list_parted where a is not null and a < 67;
explain (costs off) select * from range_list_parted where a >= 30;
-drop table list_parted cascade;
-drop table range_list_parted cascade;
+drop table list_parted;
+drop table range_list_parted;
diff --git a/src/test/regress/sql/insert.sql b/src/test/regress/sql/insert.sql
index 454e1ce2e7..dfdc24eba8 100644
--- a/src/test/regress/sql/insert.sql
+++ b/src/test/regress/sql/insert.sql
@@ -186,10 +186,7 @@ insert into list_parted (b) values (1);
select tableoid::regclass::text, a, min(b) as min_b, max(b) as max_b from list_parted group by 1, 2 order by 1;
-- cleanup
-drop table part1, part2, part3, part4, range_parted;
-drop table part_ee_ff3_1, part_ee_ff3_2, part_ee_ff1, part_ee_ff2, part_ee_ff3;
-drop table part_ee_ff, part_gg2_2, part_gg2_1, part_gg2, part_gg1, part_gg;
-drop table part_aa_bb, part_cc_dd, part_null, list_parted;
+drop table range_parted, list_parted;
-- more tests for certain multi-level partitioning scenarios
create table p (a int, b int) partition by range (a, b);
@@ -241,4 +238,4 @@ with ins (a, b, c) as
select a, b, min(c), max(c) from ins group by a, b order by 1;
-- cleanup
-drop table p, p1, p11, p12, p2, p3, p4;
+drop table p;
diff --git a/src/test/regress/sql/update.sql b/src/test/regress/sql/update.sql
index d7721ed376..663711997b 100644
--- a/src/test/regress/sql/update.sql
+++ b/src/test/regress/sql/update.sql
@@ -126,4 +126,4 @@ update range_parted set b = b - 1 where b = 10;
update range_parted set b = b + 1 where b = 10;
-- cleanup
-drop table range_parted cascade;
+drop table range_parted;
--
2.11.0
On 2017/02/22 10:49, Amit Langote wrote:
On 2017/02/21 20:17, Ashutosh Bapat wrote:
Are you sure you have attached the right patch?
Oops, really fixed this time.
Sorry again, 3rd time's a charm. I copy-paste the hunk below from the
patch file before I attach it to make sure:
- recordDependencyOn(&childobject, &parentobject, DEPENDENCY_NORMAL);
+ if (child_is_partition)
+ recordDependencyOn(&childobject, &parentobject, DEPENDENCY_AUTO);
+ else
+ recordDependencyOn(&childobject, &parentobject, DEPENDENCY_NORMAL);Thanks,
Amit
Attachments:
0001-Allow-dropping-partitioned-table-without-CASCADE.patchtext/x-diff; name=0001-Allow-dropping-partitioned-table-without-CASCADE.patchDownload
From 682624f4562087bb05b2ff9f282080bcfcfb5233 Mon Sep 17 00:00:00 2001
From: amit <amitlangote09@gmail.com>
Date: Thu, 16 Feb 2017 15:56:44 +0900
Subject: [PATCH] Allow dropping partitioned table without CASCADE
Currently, a normal dependency is created between a inheritance
parent and child when creating the child. That means one must
specify CASCADE to drop the parent table if a child table exists.
When creating partitions as inheritance children, create auto
dependency instead, so that partitions are dropped automatically
when the parent is dropped i.e., without specifying CASCADE.
---
src/backend/commands/tablecmds.c | 26 ++++++++++++++++++--------
src/test/regress/expected/alter_table.out | 10 ++++------
src/test/regress/expected/create_table.out | 9 ++-------
src/test/regress/expected/inherit.out | 22 ++--------------------
src/test/regress/expected/insert.out | 7 ++-----
src/test/regress/expected/update.out | 7 +------
src/test/regress/sql/alter_table.sql | 10 ++++------
src/test/regress/sql/create_table.sql | 9 ++-------
src/test/regress/sql/inherit.sql | 4 ++--
src/test/regress/sql/insert.sql | 7 ++-----
src/test/regress/sql/update.sql | 2 +-
11 files changed, 40 insertions(+), 73 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 3cea220421..cf566f974b 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -289,9 +289,11 @@ static List *MergeAttributes(List *schema, List *supers, char relpersistence,
static bool MergeCheckConstraint(List *constraints, char *name, Node *expr);
static void MergeAttributesIntoExisting(Relation child_rel, Relation parent_rel);
static void MergeConstraintsIntoExisting(Relation child_rel, Relation parent_rel);
-static void StoreCatalogInheritance(Oid relationId, List *supers);
+static void StoreCatalogInheritance(Oid relationId, List *supers,
+ bool child_is_partition);
static void StoreCatalogInheritance1(Oid relationId, Oid parentOid,
- int16 seqNumber, Relation inhRelation);
+ int16 seqNumber, Relation inhRelation,
+ bool child_is_partition);
static int findAttrByName(const char *attributeName, List *schema);
static void AlterIndexNamespaces(Relation classRel, Relation rel,
Oid oldNspOid, Oid newNspOid, ObjectAddresses *objsMoved);
@@ -725,7 +727,7 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
typaddress);
/* Store inheritance information for new rel. */
- StoreCatalogInheritance(relationId, inheritOids);
+ StoreCatalogInheritance(relationId, inheritOids, stmt->partbound != NULL);
/*
* We must bump the command counter to make the newly-created relation
@@ -2240,7 +2242,8 @@ MergeCheckConstraint(List *constraints, char *name, Node *expr)
* supers is a list of the OIDs of the new relation's direct ancestors.
*/
static void
-StoreCatalogInheritance(Oid relationId, List *supers)
+StoreCatalogInheritance(Oid relationId, List *supers,
+ bool child_is_partition)
{
Relation relation;
int16 seqNumber;
@@ -2270,7 +2273,8 @@ StoreCatalogInheritance(Oid relationId, List *supers)
{
Oid parentOid = lfirst_oid(entry);
- StoreCatalogInheritance1(relationId, parentOid, seqNumber, relation);
+ StoreCatalogInheritance1(relationId, parentOid, seqNumber, relation,
+ child_is_partition);
seqNumber++;
}
@@ -2283,7 +2287,8 @@ StoreCatalogInheritance(Oid relationId, List *supers)
*/
static void
StoreCatalogInheritance1(Oid relationId, Oid parentOid,
- int16 seqNumber, Relation inhRelation)
+ int16 seqNumber, Relation inhRelation,
+ bool child_is_partition)
{
TupleDesc desc = RelationGetDescr(inhRelation);
Datum values[Natts_pg_inherits];
@@ -2317,7 +2322,10 @@ StoreCatalogInheritance1(Oid relationId, Oid parentOid,
childobject.objectId = relationId;
childobject.objectSubId = 0;
- recordDependencyOn(&childobject, &parentobject, DEPENDENCY_NORMAL);
+ if (child_is_partition)
+ recordDependencyOn(&childobject, &parentobject, DEPENDENCY_AUTO);
+ else
+ recordDependencyOn(&childobject, &parentobject, DEPENDENCY_NORMAL);
/*
* Post creation hook of this inheritance. Since object_access_hook
@@ -10744,7 +10752,9 @@ CreateInheritance(Relation child_rel, Relation parent_rel)
StoreCatalogInheritance1(RelationGetRelid(child_rel),
RelationGetRelid(parent_rel),
inhseqno + 1,
- catalogRelation);
+ catalogRelation,
+ parent_rel->rd_rel->relkind ==
+ RELKIND_PARTITIONED_TABLE);
/* Now we're done with pg_inherits */
heap_close(catalogRelation, RowExclusiveLock);
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index 9885fcba89..c15bbdcbd1 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -3339,10 +3339,8 @@ ALTER TABLE list_parted2 DROP COLUMN b;
ERROR: cannot drop column named in partition key
ALTER TABLE list_parted2 ALTER COLUMN b TYPE text;
ERROR: cannot alter type of column named in partition key
--- cleanup: avoid using CASCADE
-DROP TABLE list_parted, part_1;
-DROP TABLE list_parted2, part_2, part_5, part_5_a;
-DROP TABLE range_parted, part1, part2;
+-- cleanup
+DROP TABLE list_parted, list_parted2, range_parted;
-- more tests for certain multi-level partitioning scenarios
create table p (a int, b int) partition by range (a, b);
create table p1 (b int, a int not null) partition by range (b);
@@ -3371,5 +3369,5 @@ insert into p1 (a, b) values (2, 3);
-- check that partition validation scan correctly detects violating rows
alter table p attach partition p1 for values from (1, 2) to (1, 10);
ERROR: partition constraint is violated by some row
--- cleanup: avoid using CASCADE
-drop table p, p1, p11;
+-- cleanup
+drop table p;
diff --git a/src/test/regress/expected/create_table.out b/src/test/regress/expected/create_table.out
index 20eb3d35f9..c07a474b3d 100644
--- a/src/test/regress/expected/create_table.out
+++ b/src/test/regress/expected/create_table.out
@@ -667,10 +667,5 @@ Check constraints:
"check_a" CHECK (length(a) > 0)
Number of partitions: 3 (Use \d+ to list them.)
--- cleanup: avoid using CASCADE
-DROP TABLE parted, part_a, part_b, part_c, part_c_1_10;
-DROP TABLE list_parted, part_1, part_2, part_null;
-DROP TABLE range_parted;
-DROP TABLE list_parted2, part_ab, part_null_z;
-DROP TABLE range_parted2, part0, part1, part2, part3;
-DROP TABLE range_parted3, part00, part10, part11, part12;
+-- cleanup
+DROP TABLE parted, list_parted, range_parted, list_parted2, range_parted2, range_parted3;
diff --git a/src/test/regress/expected/inherit.out b/src/test/regress/expected/inherit.out
index a8c8b28a75..795d9f575c 100644
--- a/src/test/regress/expected/inherit.out
+++ b/src/test/regress/expected/inherit.out
@@ -1843,23 +1843,5 @@ explain (costs off) select * from range_list_parted where a >= 30;
Filter: (a >= 30)
(11 rows)
-drop table list_parted cascade;
-NOTICE: drop cascades to 3 other objects
-DETAIL: drop cascades to table part_ab_cd
-drop cascades to table part_ef_gh
-drop cascades to table part_null_xy
-drop table range_list_parted cascade;
-NOTICE: drop cascades to 13 other objects
-DETAIL: drop cascades to table part_1_10
-drop cascades to table part_1_10_ab
-drop cascades to table part_1_10_cd
-drop cascades to table part_10_20
-drop cascades to table part_10_20_ab
-drop cascades to table part_10_20_cd
-drop cascades to table part_21_30
-drop cascades to table part_21_30_ab
-drop cascades to table part_21_30_cd
-drop cascades to table part_40_inf
-drop cascades to table part_40_inf_ab
-drop cascades to table part_40_inf_cd
-drop cascades to table part_40_inf_null
+drop table list_parted;
+drop table range_list_parted;
diff --git a/src/test/regress/expected/insert.out b/src/test/regress/expected/insert.out
index 81af3ef497..31cfa4e76e 100644
--- a/src/test/regress/expected/insert.out
+++ b/src/test/regress/expected/insert.out
@@ -314,10 +314,7 @@ select tableoid::regclass::text, a, min(b) as min_b, max(b) as max_b from list_p
(9 rows)
-- cleanup
-drop table part1, part2, part3, part4, range_parted;
-drop table part_ee_ff3_1, part_ee_ff3_2, part_ee_ff1, part_ee_ff2, part_ee_ff3;
-drop table part_ee_ff, part_gg2_2, part_gg2_1, part_gg2, part_gg1, part_gg;
-drop table part_aa_bb, part_cc_dd, part_null, list_parted;
+drop table range_parted, list_parted;
-- more tests for certain multi-level partitioning scenarios
create table p (a int, b int) partition by range (a, b);
create table p1 (b int not null, a int not null) partition by range ((b+0));
@@ -387,4 +384,4 @@ with ins (a, b, c) as
(5 rows)
-- cleanup
-drop table p, p1, p11, p12, p2, p3, p4;
+drop table p;
diff --git a/src/test/regress/expected/update.out b/src/test/regress/expected/update.out
index a1e9255450..9366f04255 100644
--- a/src/test/regress/expected/update.out
+++ b/src/test/regress/expected/update.out
@@ -219,9 +219,4 @@ DETAIL: Failing row contains (b, 9).
-- ok
update range_parted set b = b + 1 where b = 10;
-- cleanup
-drop table range_parted cascade;
-NOTICE: drop cascades to 4 other objects
-DETAIL: drop cascades to table part_a_1_a_10
-drop cascades to table part_a_10_a_20
-drop cascades to table part_b_1_b_10
-drop cascades to table part_b_10_b_20
+drop table range_parted;
diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql
index f7b754f0be..37f327bf6d 100644
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -2199,10 +2199,8 @@ ALTER TABLE part_2 INHERIT inh_test;
ALTER TABLE list_parted2 DROP COLUMN b;
ALTER TABLE list_parted2 ALTER COLUMN b TYPE text;
--- cleanup: avoid using CASCADE
-DROP TABLE list_parted, part_1;
-DROP TABLE list_parted2, part_2, part_5, part_5_a;
-DROP TABLE range_parted, part1, part2;
+-- cleanup
+DROP TABLE list_parted, list_parted2, range_parted;
-- more tests for certain multi-level partitioning scenarios
create table p (a int, b int) partition by range (a, b);
@@ -2227,5 +2225,5 @@ insert into p1 (a, b) values (2, 3);
-- check that partition validation scan correctly detects violating rows
alter table p attach partition p1 for values from (1, 2) to (1, 10);
--- cleanup: avoid using CASCADE
-drop table p, p1, p11;
+-- cleanup
+drop table p;
diff --git a/src/test/regress/sql/create_table.sql b/src/test/regress/sql/create_table.sql
index f41dd71475..1f0fa8e16d 100644
--- a/src/test/regress/sql/create_table.sql
+++ b/src/test/regress/sql/create_table.sql
@@ -595,10 +595,5 @@ CREATE TABLE part_c_1_10 PARTITION OF part_c FOR VALUES FROM (1) TO (10);
-- returned.
\d parted
--- cleanup: avoid using CASCADE
-DROP TABLE parted, part_a, part_b, part_c, part_c_1_10;
-DROP TABLE list_parted, part_1, part_2, part_null;
-DROP TABLE range_parted;
-DROP TABLE list_parted2, part_ab, part_null_z;
-DROP TABLE range_parted2, part0, part1, part2, part3;
-DROP TABLE range_parted3, part00, part10, part11, part12;
+-- cleanup
+DROP TABLE parted, list_parted, range_parted, list_parted2, range_parted2, range_parted3;
diff --git a/src/test/regress/sql/inherit.sql b/src/test/regress/sql/inherit.sql
index a8b7eb1c8d..836ec22c20 100644
--- a/src/test/regress/sql/inherit.sql
+++ b/src/test/regress/sql/inherit.sql
@@ -612,5 +612,5 @@ explain (costs off) select * from range_list_parted where b is null;
explain (costs off) select * from range_list_parted where a is not null and a < 67;
explain (costs off) select * from range_list_parted where a >= 30;
-drop table list_parted cascade;
-drop table range_list_parted cascade;
+drop table list_parted;
+drop table range_list_parted;
diff --git a/src/test/regress/sql/insert.sql b/src/test/regress/sql/insert.sql
index 454e1ce2e7..dfdc24eba8 100644
--- a/src/test/regress/sql/insert.sql
+++ b/src/test/regress/sql/insert.sql
@@ -186,10 +186,7 @@ insert into list_parted (b) values (1);
select tableoid::regclass::text, a, min(b) as min_b, max(b) as max_b from list_parted group by 1, 2 order by 1;
-- cleanup
-drop table part1, part2, part3, part4, range_parted;
-drop table part_ee_ff3_1, part_ee_ff3_2, part_ee_ff1, part_ee_ff2, part_ee_ff3;
-drop table part_ee_ff, part_gg2_2, part_gg2_1, part_gg2, part_gg1, part_gg;
-drop table part_aa_bb, part_cc_dd, part_null, list_parted;
+drop table range_parted, list_parted;
-- more tests for certain multi-level partitioning scenarios
create table p (a int, b int) partition by range (a, b);
@@ -241,4 +238,4 @@ with ins (a, b, c) as
select a, b, min(c), max(c) from ins group by a, b order by 1;
-- cleanup
-drop table p, p1, p11, p12, p2, p3, p4;
+drop table p;
diff --git a/src/test/regress/sql/update.sql b/src/test/regress/sql/update.sql
index d7721ed376..663711997b 100644
--- a/src/test/regress/sql/update.sql
+++ b/src/test/regress/sql/update.sql
@@ -126,4 +126,4 @@ update range_parted set b = b - 1 where b = 10;
update range_parted set b = b + 1 where b = 10;
-- cleanup
-drop table range_parted cascade;
+drop table range_parted;
--
2.11.0
Looks good to me. In the attached patch I have added a comment
explaining the reason to make partition tables "Auto" dependent upon
the corresponding partitioned tables.
In the tests we are firing commands to drop partitioned table, but are
not checking whether those tables or the partitions are getting
dropped or not. Except for drop_if_exists.sql, I did not find that we
really check this. Should we try a query on pg_class to ensure that
the tables get really dropped?
On Wed, Feb 22, 2017 at 7:26 AM, Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:
On 2017/02/22 10:49, Amit Langote wrote:
On 2017/02/21 20:17, Ashutosh Bapat wrote:
Are you sure you have attached the right patch?
Oops, really fixed this time.
Sorry again, 3rd time's a charm. I copy-paste the hunk below from the
patch file before I attach it to make sure:- recordDependencyOn(&childobject, &parentobject, DEPENDENCY_NORMAL); + if (child_is_partition) + recordDependencyOn(&childobject, &parentobject, DEPENDENCY_AUTO); + else + recordDependencyOn(&childobject, &parentobject, DEPENDENCY_NORMAL);Thanks,
Amit
--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company
Attachments:
0001-Allow-dropping-partitioned-table-without-CASCADE_v2.patchapplication/octet-stream; name=0001-Allow-dropping-partitioned-table-without-CASCADE_v2.patchDownload
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index f33aa70..715c1f0 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -289,9 +289,11 @@ static List *MergeAttributes(List *schema, List *supers, char relpersistence,
static bool MergeCheckConstraint(List *constraints, char *name, Node *expr);
static void MergeAttributesIntoExisting(Relation child_rel, Relation parent_rel);
static void MergeConstraintsIntoExisting(Relation child_rel, Relation parent_rel);
-static void StoreCatalogInheritance(Oid relationId, List *supers);
+static void StoreCatalogInheritance(Oid relationId, List *supers,
+ bool child_is_partition);
static void StoreCatalogInheritance1(Oid relationId, Oid parentOid,
- int16 seqNumber, Relation inhRelation);
+ int16 seqNumber, Relation inhRelation,
+ bool child_is_partition);
static int findAttrByName(const char *attributeName, List *schema);
static void AlterIndexNamespaces(Relation classRel, Relation rel,
Oid oldNspOid, Oid newNspOid, ObjectAddresses *objsMoved);
@@ -730,7 +732,7 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
typaddress);
/* Store inheritance information for new rel. */
- StoreCatalogInheritance(relationId, inheritOids);
+ StoreCatalogInheritance(relationId, inheritOids, stmt->partbound != NULL);
/*
* We must bump the command counter to make the newly-created relation
@@ -2245,7 +2247,8 @@ MergeCheckConstraint(List *constraints, char *name, Node *expr)
* supers is a list of the OIDs of the new relation's direct ancestors.
*/
static void
-StoreCatalogInheritance(Oid relationId, List *supers)
+StoreCatalogInheritance(Oid relationId, List *supers,
+ bool child_is_partition)
{
Relation relation;
int16 seqNumber;
@@ -2275,7 +2278,8 @@ StoreCatalogInheritance(Oid relationId, List *supers)
{
Oid parentOid = lfirst_oid(entry);
- StoreCatalogInheritance1(relationId, parentOid, seqNumber, relation);
+ StoreCatalogInheritance1(relationId, parentOid, seqNumber, relation,
+ child_is_partition);
seqNumber++;
}
@@ -2288,7 +2292,8 @@ StoreCatalogInheritance(Oid relationId, List *supers)
*/
static void
StoreCatalogInheritance1(Oid relationId, Oid parentOid,
- int16 seqNumber, Relation inhRelation)
+ int16 seqNumber, Relation inhRelation,
+ bool child_is_partition)
{
TupleDesc desc = RelationGetDescr(inhRelation);
Datum values[Natts_pg_inherits];
@@ -2322,7 +2327,14 @@ StoreCatalogInheritance1(Oid relationId, Oid parentOid,
childobject.objectId = relationId;
childobject.objectSubId = 0;
- recordDependencyOn(&childobject, &parentobject, DEPENDENCY_NORMAL);
+ /*
+ * Unlike inheritance children, partition tables are expected to be dropped
+ * when the parent partitioned table gets dropped.
+ */
+ if (child_is_partition)
+ recordDependencyOn(&childobject, &parentobject, DEPENDENCY_AUTO);
+ else
+ recordDependencyOn(&childobject, &parentobject, DEPENDENCY_NORMAL);
/*
* Post creation hook of this inheritance. Since object_access_hook
@@ -10749,7 +10761,9 @@ CreateInheritance(Relation child_rel, Relation parent_rel)
StoreCatalogInheritance1(RelationGetRelid(child_rel),
RelationGetRelid(parent_rel),
inhseqno + 1,
- catalogRelation);
+ catalogRelation,
+ parent_rel->rd_rel->relkind ==
+ RELKIND_PARTITIONED_TABLE);
/* Now we're done with pg_inherits */
heap_close(catalogRelation, RowExclusiveLock);
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index b0e80a7..0d1554a 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -3334,10 +3334,8 @@ ALTER TABLE list_parted2 DROP COLUMN b;
ERROR: cannot drop column named in partition key
ALTER TABLE list_parted2 ALTER COLUMN b TYPE text;
ERROR: cannot alter type of column named in partition key
--- cleanup: avoid using CASCADE
-DROP TABLE list_parted, part_1;
-DROP TABLE list_parted2, part_2, part_5, part_5_a;
-DROP TABLE range_parted, part1, part2;
+-- cleanup
+DROP TABLE list_parted, list_parted2, range_parted;
-- more tests for certain multi-level partitioning scenarios
create table p (a int, b int) partition by range (a, b);
create table p1 (b int, a int not null) partition by range (b);
@@ -3366,5 +3364,5 @@ insert into p1 (a, b) values (2, 3);
-- check that partition validation scan correctly detects violating rows
alter table p attach partition p1 for values from (1, 2) to (1, 10);
ERROR: partition constraint is violated by some row
--- cleanup: avoid using CASCADE
-drop table p, p1, p11;
+-- cleanup
+drop table p;
diff --git a/src/test/regress/expected/create_table.out b/src/test/regress/expected/create_table.out
index fc92cd9..bfad755 100644
--- a/src/test/regress/expected/create_table.out
+++ b/src/test/regress/expected/create_table.out
@@ -659,10 +659,5 @@ Check constraints:
"check_a" CHECK (length(a) > 0)
Number of partitions: 3 (Use \d+ to list them.)
--- cleanup: avoid using CASCADE
-DROP TABLE parted, part_a, part_b, part_c, part_c_1_10;
-DROP TABLE list_parted, part_1, part_2, part_null;
-DROP TABLE range_parted;
-DROP TABLE list_parted2, part_ab, part_null_z;
-DROP TABLE range_parted2, part0, part1, part2, part3;
-DROP TABLE range_parted3, part00, part10, part11, part12;
+-- cleanup
+DROP TABLE parted, list_parted, range_parted, list_parted2, range_parted2, range_parted3;
diff --git a/src/test/regress/expected/inherit.out b/src/test/regress/expected/inherit.out
index a8c8b28..795d9f5 100644
--- a/src/test/regress/expected/inherit.out
+++ b/src/test/regress/expected/inherit.out
@@ -1843,23 +1843,5 @@ explain (costs off) select * from range_list_parted where a >= 30;
Filter: (a >= 30)
(11 rows)
-drop table list_parted cascade;
-NOTICE: drop cascades to 3 other objects
-DETAIL: drop cascades to table part_ab_cd
-drop cascades to table part_ef_gh
-drop cascades to table part_null_xy
-drop table range_list_parted cascade;
-NOTICE: drop cascades to 13 other objects
-DETAIL: drop cascades to table part_1_10
-drop cascades to table part_1_10_ab
-drop cascades to table part_1_10_cd
-drop cascades to table part_10_20
-drop cascades to table part_10_20_ab
-drop cascades to table part_10_20_cd
-drop cascades to table part_21_30
-drop cascades to table part_21_30_ab
-drop cascades to table part_21_30_cd
-drop cascades to table part_40_inf
-drop cascades to table part_40_inf_ab
-drop cascades to table part_40_inf_cd
-drop cascades to table part_40_inf_null
+drop table list_parted;
+drop table range_list_parted;
diff --git a/src/test/regress/expected/insert.out b/src/test/regress/expected/insert.out
index 81af3ef..31cfa4e 100644
--- a/src/test/regress/expected/insert.out
+++ b/src/test/regress/expected/insert.out
@@ -314,10 +314,7 @@ select tableoid::regclass::text, a, min(b) as min_b, max(b) as max_b from list_p
(9 rows)
-- cleanup
-drop table part1, part2, part3, part4, range_parted;
-drop table part_ee_ff3_1, part_ee_ff3_2, part_ee_ff1, part_ee_ff2, part_ee_ff3;
-drop table part_ee_ff, part_gg2_2, part_gg2_1, part_gg2, part_gg1, part_gg;
-drop table part_aa_bb, part_cc_dd, part_null, list_parted;
+drop table range_parted, list_parted;
-- more tests for certain multi-level partitioning scenarios
create table p (a int, b int) partition by range (a, b);
create table p1 (b int not null, a int not null) partition by range ((b+0));
@@ -387,4 +384,4 @@ with ins (a, b, c) as
(5 rows)
-- cleanup
-drop table p, p1, p11, p12, p2, p3, p4;
+drop table p;
diff --git a/src/test/regress/expected/update.out b/src/test/regress/expected/update.out
index a1e9255..9366f04 100644
--- a/src/test/regress/expected/update.out
+++ b/src/test/regress/expected/update.out
@@ -219,9 +219,4 @@ DETAIL: Failing row contains (b, 9).
-- ok
update range_parted set b = b + 1 where b = 10;
-- cleanup
-drop table range_parted cascade;
-NOTICE: drop cascades to 4 other objects
-DETAIL: drop cascades to table part_a_1_a_10
-drop cascades to table part_a_10_a_20
-drop cascades to table part_b_1_b_10
-drop cascades to table part_b_10_b_20
+drop table range_parted;
diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql
index 7513769..0787e78 100644
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -2194,10 +2194,8 @@ ALTER TABLE part_2 INHERIT inh_test;
ALTER TABLE list_parted2 DROP COLUMN b;
ALTER TABLE list_parted2 ALTER COLUMN b TYPE text;
--- cleanup: avoid using CASCADE
-DROP TABLE list_parted, part_1;
-DROP TABLE list_parted2, part_2, part_5, part_5_a;
-DROP TABLE range_parted, part1, part2;
+-- cleanup
+DROP TABLE list_parted, list_parted2, range_parted;
-- more tests for certain multi-level partitioning scenarios
create table p (a int, b int) partition by range (a, b);
@@ -2222,5 +2220,5 @@ insert into p1 (a, b) values (2, 3);
-- check that partition validation scan correctly detects violating rows
alter table p attach partition p1 for values from (1, 2) to (1, 10);
--- cleanup: avoid using CASCADE
-drop table p, p1, p11;
+-- cleanup
+drop table p;
diff --git a/src/test/regress/sql/create_table.sql b/src/test/regress/sql/create_table.sql
index 5f25c43..553c084 100644
--- a/src/test/regress/sql/create_table.sql
+++ b/src/test/regress/sql/create_table.sql
@@ -593,10 +593,5 @@ CREATE TABLE part_c_1_10 PARTITION OF part_c FOR VALUES FROM (1) TO (10);
-- returned.
\d parted
--- cleanup: avoid using CASCADE
-DROP TABLE parted, part_a, part_b, part_c, part_c_1_10;
-DROP TABLE list_parted, part_1, part_2, part_null;
-DROP TABLE range_parted;
-DROP TABLE list_parted2, part_ab, part_null_z;
-DROP TABLE range_parted2, part0, part1, part2, part3;
-DROP TABLE range_parted3, part00, part10, part11, part12;
+-- cleanup
+DROP TABLE parted, list_parted, range_parted, list_parted2, range_parted2, range_parted3;
diff --git a/src/test/regress/sql/inherit.sql b/src/test/regress/sql/inherit.sql
index a8b7eb1..836ec22 100644
--- a/src/test/regress/sql/inherit.sql
+++ b/src/test/regress/sql/inherit.sql
@@ -612,5 +612,5 @@ explain (costs off) select * from range_list_parted where b is null;
explain (costs off) select * from range_list_parted where a is not null and a < 67;
explain (costs off) select * from range_list_parted where a >= 30;
-drop table list_parted cascade;
-drop table range_list_parted cascade;
+drop table list_parted;
+drop table range_list_parted;
diff --git a/src/test/regress/sql/insert.sql b/src/test/regress/sql/insert.sql
index 454e1ce..dfdc24e 100644
--- a/src/test/regress/sql/insert.sql
+++ b/src/test/regress/sql/insert.sql
@@ -186,10 +186,7 @@ insert into list_parted (b) values (1);
select tableoid::regclass::text, a, min(b) as min_b, max(b) as max_b from list_parted group by 1, 2 order by 1;
-- cleanup
-drop table part1, part2, part3, part4, range_parted;
-drop table part_ee_ff3_1, part_ee_ff3_2, part_ee_ff1, part_ee_ff2, part_ee_ff3;
-drop table part_ee_ff, part_gg2_2, part_gg2_1, part_gg2, part_gg1, part_gg;
-drop table part_aa_bb, part_cc_dd, part_null, list_parted;
+drop table range_parted, list_parted;
-- more tests for certain multi-level partitioning scenarios
create table p (a int, b int) partition by range (a, b);
@@ -241,4 +238,4 @@ with ins (a, b, c) as
select a, b, min(c), max(c) from ins group by a, b order by 1;
-- cleanup
-drop table p, p1, p11, p12, p2, p3, p4;
+drop table p;
diff --git a/src/test/regress/sql/update.sql b/src/test/regress/sql/update.sql
index d7721ed..6637119 100644
--- a/src/test/regress/sql/update.sql
+++ b/src/test/regress/sql/update.sql
@@ -126,4 +126,4 @@ update range_parted set b = b - 1 where b = 10;
update range_parted set b = b + 1 where b = 10;
-- cleanup
-drop table range_parted cascade;
+drop table range_parted;
On 2017/02/22 13:46, Ashutosh Bapat wrote:
Looks good to me. In the attached patch I have added a comment
explaining the reason to make partition tables "Auto" dependent upon
the corresponding partitioned tables.
Good call.
+ /*
+ * Unlike inheritance children, partition tables are expected to be dropped
+ * when the parent partitioned table gets dropped.
+ */Hmm. Partitions *are* inheritance children, so we perhaps don't need the
part before the comma. Also, adding "automatically" somewhere in there
would be nice.
Or, one could just write: /* add an auto dependency for partitions */
In the tests we are firing commands to drop partitioned table, but are
not checking whether those tables or the partitions are getting
dropped or not. Except for drop_if_exists.sql, I did not find that we
really check this. Should we try a query on pg_class to ensure that
the tables get really dropped?
I don't see why this patch should do it, if dependency.sql itself does
not? I mean dropping AUTO dependent objects is one of the contracts of
dependency.c, so perhaps it would make sense to query pg_class in
dependency.sql to check if AUTO dependencies work correctly.
Thanks,
Amit
Attachments:
0001-Allow-dropping-partitioned-table-without-CASCADE.patchtext/x-diff; name=0001-Allow-dropping-partitioned-table-without-CASCADE.patchDownload
From 682624f4562087bb05b2ff9f282080bcfcfb5233 Mon Sep 17 00:00:00 2001
From: amit <amitlangote09@gmail.com>
Date: Thu, 16 Feb 2017 15:56:44 +0900
Subject: [PATCH] Allow dropping partitioned table without CASCADE
Currently, a normal dependency is created between a inheritance
parent and child when creating the child. That means one must
specify CASCADE to drop the parent table if a child table exists.
When creating partitions as inheritance children, create auto
dependency instead, so that partitions are dropped automatically
when the parent is dropped i.e., without specifying CASCADE.
---
src/backend/commands/tablecmds.c | 26 ++++++++++++++++++--------
src/test/regress/expected/alter_table.out | 10 ++++------
src/test/regress/expected/create_table.out | 9 ++-------
src/test/regress/expected/inherit.out | 22 ++--------------------
src/test/regress/expected/insert.out | 7 ++-----
src/test/regress/expected/update.out | 7 +------
src/test/regress/sql/alter_table.sql | 10 ++++------
src/test/regress/sql/create_table.sql | 9 ++-------
src/test/regress/sql/inherit.sql | 4 ++--
src/test/regress/sql/insert.sql | 7 ++-----
src/test/regress/sql/update.sql | 2 +-
11 files changed, 40 insertions(+), 73 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 3cea220421..cf566f974b 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -289,9 +289,11 @@ static List *MergeAttributes(List *schema, List *supers, char relpersistence,
static bool MergeCheckConstraint(List *constraints, char *name, Node *expr);
static void MergeAttributesIntoExisting(Relation child_rel, Relation parent_rel);
static void MergeConstraintsIntoExisting(Relation child_rel, Relation parent_rel);
-static void StoreCatalogInheritance(Oid relationId, List *supers);
+static void StoreCatalogInheritance(Oid relationId, List *supers,
+ bool child_is_partition);
static void StoreCatalogInheritance1(Oid relationId, Oid parentOid,
- int16 seqNumber, Relation inhRelation);
+ int16 seqNumber, Relation inhRelation,
+ bool child_is_partition);
static int findAttrByName(const char *attributeName, List *schema);
static void AlterIndexNamespaces(Relation classRel, Relation rel,
Oid oldNspOid, Oid newNspOid, ObjectAddresses *objsMoved);
@@ -725,7 +727,7 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
typaddress);
/* Store inheritance information for new rel. */
- StoreCatalogInheritance(relationId, inheritOids);
+ StoreCatalogInheritance(relationId, inheritOids, stmt->partbound != NULL);
/*
* We must bump the command counter to make the newly-created relation
@@ -2240,7 +2242,8 @@ MergeCheckConstraint(List *constraints, char *name, Node *expr)
* supers is a list of the OIDs of the new relation's direct ancestors.
*/
static void
-StoreCatalogInheritance(Oid relationId, List *supers)
+StoreCatalogInheritance(Oid relationId, List *supers,
+ bool child_is_partition)
{
Relation relation;
int16 seqNumber;
@@ -2270,7 +2273,8 @@ StoreCatalogInheritance(Oid relationId, List *supers)
{
Oid parentOid = lfirst_oid(entry);
- StoreCatalogInheritance1(relationId, parentOid, seqNumber, relation);
+ StoreCatalogInheritance1(relationId, parentOid, seqNumber, relation,
+ child_is_partition);
seqNumber++;
}
@@ -2283,7 +2287,8 @@ StoreCatalogInheritance(Oid relationId, List *supers)
*/
static void
StoreCatalogInheritance1(Oid relationId, Oid parentOid,
- int16 seqNumber, Relation inhRelation)
+ int16 seqNumber, Relation inhRelation,
+ bool child_is_partition)
{
TupleDesc desc = RelationGetDescr(inhRelation);
Datum values[Natts_pg_inherits];
@@ -2317,7 +2322,10 @@ StoreCatalogInheritance1(Oid relationId, Oid parentOid,
childobject.objectId = relationId;
childobject.objectSubId = 0;
- recordDependencyOn(&childobject, &parentobject, DEPENDENCY_NORMAL);
+ if (child_is_partition)
+ recordDependencyOn(&childobject, &parentobject, DEPENDENCY_AUTO);
+ else
+ recordDependencyOn(&childobject, &parentobject, DEPENDENCY_NORMAL);
/*
* Post creation hook of this inheritance. Since object_access_hook
@@ -10744,7 +10752,9 @@ CreateInheritance(Relation child_rel, Relation parent_rel)
StoreCatalogInheritance1(RelationGetRelid(child_rel),
RelationGetRelid(parent_rel),
inhseqno + 1,
- catalogRelation);
+ catalogRelation,
+ parent_rel->rd_rel->relkind ==
+ RELKIND_PARTITIONED_TABLE);
/* Now we're done with pg_inherits */
heap_close(catalogRelation, RowExclusiveLock);
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index 9885fcba89..c15bbdcbd1 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -3339,10 +3339,8 @@ ALTER TABLE list_parted2 DROP COLUMN b;
ERROR: cannot drop column named in partition key
ALTER TABLE list_parted2 ALTER COLUMN b TYPE text;
ERROR: cannot alter type of column named in partition key
--- cleanup: avoid using CASCADE
-DROP TABLE list_parted, part_1;
-DROP TABLE list_parted2, part_2, part_5, part_5_a;
-DROP TABLE range_parted, part1, part2;
+-- cleanup
+DROP TABLE list_parted, list_parted2, range_parted;
-- more tests for certain multi-level partitioning scenarios
create table p (a int, b int) partition by range (a, b);
create table p1 (b int, a int not null) partition by range (b);
@@ -3371,5 +3369,5 @@ insert into p1 (a, b) values (2, 3);
-- check that partition validation scan correctly detects violating rows
alter table p attach partition p1 for values from (1, 2) to (1, 10);
ERROR: partition constraint is violated by some row
--- cleanup: avoid using CASCADE
-drop table p, p1, p11;
+-- cleanup
+drop table p;
diff --git a/src/test/regress/expected/create_table.out b/src/test/regress/expected/create_table.out
index 20eb3d35f9..c07a474b3d 100644
--- a/src/test/regress/expected/create_table.out
+++ b/src/test/regress/expected/create_table.out
@@ -667,10 +667,5 @@ Check constraints:
"check_a" CHECK (length(a) > 0)
Number of partitions: 3 (Use \d+ to list them.)
--- cleanup: avoid using CASCADE
-DROP TABLE parted, part_a, part_b, part_c, part_c_1_10;
-DROP TABLE list_parted, part_1, part_2, part_null;
-DROP TABLE range_parted;
-DROP TABLE list_parted2, part_ab, part_null_z;
-DROP TABLE range_parted2, part0, part1, part2, part3;
-DROP TABLE range_parted3, part00, part10, part11, part12;
+-- cleanup
+DROP TABLE parted, list_parted, range_parted, list_parted2, range_parted2, range_parted3;
diff --git a/src/test/regress/expected/inherit.out b/src/test/regress/expected/inherit.out
index a8c8b28a75..795d9f575c 100644
--- a/src/test/regress/expected/inherit.out
+++ b/src/test/regress/expected/inherit.out
@@ -1843,23 +1843,5 @@ explain (costs off) select * from range_list_parted where a >= 30;
Filter: (a >= 30)
(11 rows)
-drop table list_parted cascade;
-NOTICE: drop cascades to 3 other objects
-DETAIL: drop cascades to table part_ab_cd
-drop cascades to table part_ef_gh
-drop cascades to table part_null_xy
-drop table range_list_parted cascade;
-NOTICE: drop cascades to 13 other objects
-DETAIL: drop cascades to table part_1_10
-drop cascades to table part_1_10_ab
-drop cascades to table part_1_10_cd
-drop cascades to table part_10_20
-drop cascades to table part_10_20_ab
-drop cascades to table part_10_20_cd
-drop cascades to table part_21_30
-drop cascades to table part_21_30_ab
-drop cascades to table part_21_30_cd
-drop cascades to table part_40_inf
-drop cascades to table part_40_inf_ab
-drop cascades to table part_40_inf_cd
-drop cascades to table part_40_inf_null
+drop table list_parted;
+drop table range_list_parted;
diff --git a/src/test/regress/expected/insert.out b/src/test/regress/expected/insert.out
index 81af3ef497..31cfa4e76e 100644
--- a/src/test/regress/expected/insert.out
+++ b/src/test/regress/expected/insert.out
@@ -314,10 +314,7 @@ select tableoid::regclass::text, a, min(b) as min_b, max(b) as max_b from list_p
(9 rows)
-- cleanup
-drop table part1, part2, part3, part4, range_parted;
-drop table part_ee_ff3_1, part_ee_ff3_2, part_ee_ff1, part_ee_ff2, part_ee_ff3;
-drop table part_ee_ff, part_gg2_2, part_gg2_1, part_gg2, part_gg1, part_gg;
-drop table part_aa_bb, part_cc_dd, part_null, list_parted;
+drop table range_parted, list_parted;
-- more tests for certain multi-level partitioning scenarios
create table p (a int, b int) partition by range (a, b);
create table p1 (b int not null, a int not null) partition by range ((b+0));
@@ -387,4 +384,4 @@ with ins (a, b, c) as
(5 rows)
-- cleanup
-drop table p, p1, p11, p12, p2, p3, p4;
+drop table p;
diff --git a/src/test/regress/expected/update.out b/src/test/regress/expected/update.out
index a1e9255450..9366f04255 100644
--- a/src/test/regress/expected/update.out
+++ b/src/test/regress/expected/update.out
@@ -219,9 +219,4 @@ DETAIL: Failing row contains (b, 9).
-- ok
update range_parted set b = b + 1 where b = 10;
-- cleanup
-drop table range_parted cascade;
-NOTICE: drop cascades to 4 other objects
-DETAIL: drop cascades to table part_a_1_a_10
-drop cascades to table part_a_10_a_20
-drop cascades to table part_b_1_b_10
-drop cascades to table part_b_10_b_20
+drop table range_parted;
diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql
index f7b754f0be..37f327bf6d 100644
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -2199,10 +2199,8 @@ ALTER TABLE part_2 INHERIT inh_test;
ALTER TABLE list_parted2 DROP COLUMN b;
ALTER TABLE list_parted2 ALTER COLUMN b TYPE text;
--- cleanup: avoid using CASCADE
-DROP TABLE list_parted, part_1;
-DROP TABLE list_parted2, part_2, part_5, part_5_a;
-DROP TABLE range_parted, part1, part2;
+-- cleanup
+DROP TABLE list_parted, list_parted2, range_parted;
-- more tests for certain multi-level partitioning scenarios
create table p (a int, b int) partition by range (a, b);
@@ -2227,5 +2225,5 @@ insert into p1 (a, b) values (2, 3);
-- check that partition validation scan correctly detects violating rows
alter table p attach partition p1 for values from (1, 2) to (1, 10);
--- cleanup: avoid using CASCADE
-drop table p, p1, p11;
+-- cleanup
+drop table p;
diff --git a/src/test/regress/sql/create_table.sql b/src/test/regress/sql/create_table.sql
index f41dd71475..1f0fa8e16d 100644
--- a/src/test/regress/sql/create_table.sql
+++ b/src/test/regress/sql/create_table.sql
@@ -595,10 +595,5 @@ CREATE TABLE part_c_1_10 PARTITION OF part_c FOR VALUES FROM (1) TO (10);
-- returned.
\d parted
--- cleanup: avoid using CASCADE
-DROP TABLE parted, part_a, part_b, part_c, part_c_1_10;
-DROP TABLE list_parted, part_1, part_2, part_null;
-DROP TABLE range_parted;
-DROP TABLE list_parted2, part_ab, part_null_z;
-DROP TABLE range_parted2, part0, part1, part2, part3;
-DROP TABLE range_parted3, part00, part10, part11, part12;
+-- cleanup
+DROP TABLE parted, list_parted, range_parted, list_parted2, range_parted2, range_parted3;
diff --git a/src/test/regress/sql/inherit.sql b/src/test/regress/sql/inherit.sql
index a8b7eb1c8d..836ec22c20 100644
--- a/src/test/regress/sql/inherit.sql
+++ b/src/test/regress/sql/inherit.sql
@@ -612,5 +612,5 @@ explain (costs off) select * from range_list_parted where b is null;
explain (costs off) select * from range_list_parted where a is not null and a < 67;
explain (costs off) select * from range_list_parted where a >= 30;
-drop table list_parted cascade;
-drop table range_list_parted cascade;
+drop table list_parted;
+drop table range_list_parted;
diff --git a/src/test/regress/sql/insert.sql b/src/test/regress/sql/insert.sql
index 454e1ce2e7..dfdc24eba8 100644
--- a/src/test/regress/sql/insert.sql
+++ b/src/test/regress/sql/insert.sql
@@ -186,10 +186,7 @@ insert into list_parted (b) values (1);
select tableoid::regclass::text, a, min(b) as min_b, max(b) as max_b from list_parted group by 1, 2 order by 1;
-- cleanup
-drop table part1, part2, part3, part4, range_parted;
-drop table part_ee_ff3_1, part_ee_ff3_2, part_ee_ff1, part_ee_ff2, part_ee_ff3;
-drop table part_ee_ff, part_gg2_2, part_gg2_1, part_gg2, part_gg1, part_gg;
-drop table part_aa_bb, part_cc_dd, part_null, list_parted;
+drop table range_parted, list_parted;
-- more tests for certain multi-level partitioning scenarios
create table p (a int, b int) partition by range (a, b);
@@ -241,4 +238,4 @@ with ins (a, b, c) as
select a, b, min(c), max(c) from ins group by a, b order by 1;
-- cleanup
-drop table p, p1, p11, p12, p2, p3, p4;
+drop table p;
diff --git a/src/test/regress/sql/update.sql b/src/test/regress/sql/update.sql
index d7721ed376..663711997b 100644
--- a/src/test/regress/sql/update.sql
+++ b/src/test/regress/sql/update.sql
@@ -126,4 +126,4 @@ update range_parted set b = b - 1 where b = 10;
update range_parted set b = b + 1 where b = 10;
-- cleanup
-drop table range_parted cascade;
+drop table range_parted;
--
2.11.0
On Wed, Feb 22, 2017 at 11:11 AM, Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:
On 2017/02/22 13:46, Ashutosh Bapat wrote:
Looks good to me. In the attached patch I have added a comment
explaining the reason to make partition tables "Auto" dependent upon
the corresponding partitioned tables.Good call.
+ /* + * Unlike inheritance children, partition tables are expected to be dropped + * when the parent partitioned table gets dropped. + */Hmm. Partitions *are* inheritance children, so we perhaps don't need the
part before the comma. Also, adding "automatically" somewhere in there
would be nice.Or, one could just write: /* add an auto dependency for partitions */
I changed it in the attached patch to
+ /*
+ * Partition tables are expected to be dropped when the parent partitioned
+ * table gets dropped.
+ */In the tests we are firing commands to drop partitioned table, but are
not checking whether those tables or the partitions are getting
dropped or not. Except for drop_if_exists.sql, I did not find that we
really check this. Should we try a query on pg_class to ensure that
the tables get really dropped?I don't see why this patch should do it, if dependency.sql itself does
not? I mean dropping AUTO dependent objects is one of the contracts of
dependency.c, so perhaps it would make sense to query pg_class in
dependency.sql to check if AUTO dependencies work correctly.
Hmm, I agree.
--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company
Attachments:
0001-Allow-dropping-partitioned-table-without-CASCADE_v3.patchapplication/octet-stream; name=0001-Allow-dropping-partitioned-table-without-CASCADE_v3.patchDownload
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index f33aa70..2f14abb 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -289,9 +289,11 @@ static List *MergeAttributes(List *schema, List *supers, char relpersistence,
static bool MergeCheckConstraint(List *constraints, char *name, Node *expr);
static void MergeAttributesIntoExisting(Relation child_rel, Relation parent_rel);
static void MergeConstraintsIntoExisting(Relation child_rel, Relation parent_rel);
-static void StoreCatalogInheritance(Oid relationId, List *supers);
+static void StoreCatalogInheritance(Oid relationId, List *supers,
+ bool child_is_partition);
static void StoreCatalogInheritance1(Oid relationId, Oid parentOid,
- int16 seqNumber, Relation inhRelation);
+ int16 seqNumber, Relation inhRelation,
+ bool child_is_partition);
static int findAttrByName(const char *attributeName, List *schema);
static void AlterIndexNamespaces(Relation classRel, Relation rel,
Oid oldNspOid, Oid newNspOid, ObjectAddresses *objsMoved);
@@ -730,7 +732,7 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
typaddress);
/* Store inheritance information for new rel. */
- StoreCatalogInheritance(relationId, inheritOids);
+ StoreCatalogInheritance(relationId, inheritOids, stmt->partbound != NULL);
/*
* We must bump the command counter to make the newly-created relation
@@ -2245,7 +2247,8 @@ MergeCheckConstraint(List *constraints, char *name, Node *expr)
* supers is a list of the OIDs of the new relation's direct ancestors.
*/
static void
-StoreCatalogInheritance(Oid relationId, List *supers)
+StoreCatalogInheritance(Oid relationId, List *supers,
+ bool child_is_partition)
{
Relation relation;
int16 seqNumber;
@@ -2275,7 +2278,8 @@ StoreCatalogInheritance(Oid relationId, List *supers)
{
Oid parentOid = lfirst_oid(entry);
- StoreCatalogInheritance1(relationId, parentOid, seqNumber, relation);
+ StoreCatalogInheritance1(relationId, parentOid, seqNumber, relation,
+ child_is_partition);
seqNumber++;
}
@@ -2288,7 +2292,8 @@ StoreCatalogInheritance(Oid relationId, List *supers)
*/
static void
StoreCatalogInheritance1(Oid relationId, Oid parentOid,
- int16 seqNumber, Relation inhRelation)
+ int16 seqNumber, Relation inhRelation,
+ bool child_is_partition)
{
TupleDesc desc = RelationGetDescr(inhRelation);
Datum values[Natts_pg_inherits];
@@ -2322,7 +2327,14 @@ StoreCatalogInheritance1(Oid relationId, Oid parentOid,
childobject.objectId = relationId;
childobject.objectSubId = 0;
- recordDependencyOn(&childobject, &parentobject, DEPENDENCY_NORMAL);
+ /*
+ * Partition tables are expected to be dropped when the parent partitioned
+ * table gets dropped.
+ */
+ if (child_is_partition)
+ recordDependencyOn(&childobject, &parentobject, DEPENDENCY_AUTO);
+ else
+ recordDependencyOn(&childobject, &parentobject, DEPENDENCY_NORMAL);
/*
* Post creation hook of this inheritance. Since object_access_hook
@@ -10749,7 +10761,9 @@ CreateInheritance(Relation child_rel, Relation parent_rel)
StoreCatalogInheritance1(RelationGetRelid(child_rel),
RelationGetRelid(parent_rel),
inhseqno + 1,
- catalogRelation);
+ catalogRelation,
+ parent_rel->rd_rel->relkind ==
+ RELKIND_PARTITIONED_TABLE);
/* Now we're done with pg_inherits */
heap_close(catalogRelation, RowExclusiveLock);
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index b0e80a7..0d1554a 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -3334,10 +3334,8 @@ ALTER TABLE list_parted2 DROP COLUMN b;
ERROR: cannot drop column named in partition key
ALTER TABLE list_parted2 ALTER COLUMN b TYPE text;
ERROR: cannot alter type of column named in partition key
--- cleanup: avoid using CASCADE
-DROP TABLE list_parted, part_1;
-DROP TABLE list_parted2, part_2, part_5, part_5_a;
-DROP TABLE range_parted, part1, part2;
+-- cleanup
+DROP TABLE list_parted, list_parted2, range_parted;
-- more tests for certain multi-level partitioning scenarios
create table p (a int, b int) partition by range (a, b);
create table p1 (b int, a int not null) partition by range (b);
@@ -3366,5 +3364,5 @@ insert into p1 (a, b) values (2, 3);
-- check that partition validation scan correctly detects violating rows
alter table p attach partition p1 for values from (1, 2) to (1, 10);
ERROR: partition constraint is violated by some row
--- cleanup: avoid using CASCADE
-drop table p, p1, p11;
+-- cleanup
+drop table p;
diff --git a/src/test/regress/expected/create_table.out b/src/test/regress/expected/create_table.out
index fc92cd9..bfad755 100644
--- a/src/test/regress/expected/create_table.out
+++ b/src/test/regress/expected/create_table.out
@@ -659,10 +659,5 @@ Check constraints:
"check_a" CHECK (length(a) > 0)
Number of partitions: 3 (Use \d+ to list them.)
--- cleanup: avoid using CASCADE
-DROP TABLE parted, part_a, part_b, part_c, part_c_1_10;
-DROP TABLE list_parted, part_1, part_2, part_null;
-DROP TABLE range_parted;
-DROP TABLE list_parted2, part_ab, part_null_z;
-DROP TABLE range_parted2, part0, part1, part2, part3;
-DROP TABLE range_parted3, part00, part10, part11, part12;
+-- cleanup
+DROP TABLE parted, list_parted, range_parted, list_parted2, range_parted2, range_parted3;
diff --git a/src/test/regress/expected/inherit.out b/src/test/regress/expected/inherit.out
index a8c8b28..795d9f5 100644
--- a/src/test/regress/expected/inherit.out
+++ b/src/test/regress/expected/inherit.out
@@ -1843,23 +1843,5 @@ explain (costs off) select * from range_list_parted where a >= 30;
Filter: (a >= 30)
(11 rows)
-drop table list_parted cascade;
-NOTICE: drop cascades to 3 other objects
-DETAIL: drop cascades to table part_ab_cd
-drop cascades to table part_ef_gh
-drop cascades to table part_null_xy
-drop table range_list_parted cascade;
-NOTICE: drop cascades to 13 other objects
-DETAIL: drop cascades to table part_1_10
-drop cascades to table part_1_10_ab
-drop cascades to table part_1_10_cd
-drop cascades to table part_10_20
-drop cascades to table part_10_20_ab
-drop cascades to table part_10_20_cd
-drop cascades to table part_21_30
-drop cascades to table part_21_30_ab
-drop cascades to table part_21_30_cd
-drop cascades to table part_40_inf
-drop cascades to table part_40_inf_ab
-drop cascades to table part_40_inf_cd
-drop cascades to table part_40_inf_null
+drop table list_parted;
+drop table range_list_parted;
diff --git a/src/test/regress/expected/insert.out b/src/test/regress/expected/insert.out
index 81af3ef..31cfa4e 100644
--- a/src/test/regress/expected/insert.out
+++ b/src/test/regress/expected/insert.out
@@ -314,10 +314,7 @@ select tableoid::regclass::text, a, min(b) as min_b, max(b) as max_b from list_p
(9 rows)
-- cleanup
-drop table part1, part2, part3, part4, range_parted;
-drop table part_ee_ff3_1, part_ee_ff3_2, part_ee_ff1, part_ee_ff2, part_ee_ff3;
-drop table part_ee_ff, part_gg2_2, part_gg2_1, part_gg2, part_gg1, part_gg;
-drop table part_aa_bb, part_cc_dd, part_null, list_parted;
+drop table range_parted, list_parted;
-- more tests for certain multi-level partitioning scenarios
create table p (a int, b int) partition by range (a, b);
create table p1 (b int not null, a int not null) partition by range ((b+0));
@@ -387,4 +384,4 @@ with ins (a, b, c) as
(5 rows)
-- cleanup
-drop table p, p1, p11, p12, p2, p3, p4;
+drop table p;
diff --git a/src/test/regress/expected/update.out b/src/test/regress/expected/update.out
index a1e9255..9366f04 100644
--- a/src/test/regress/expected/update.out
+++ b/src/test/regress/expected/update.out
@@ -219,9 +219,4 @@ DETAIL: Failing row contains (b, 9).
-- ok
update range_parted set b = b + 1 where b = 10;
-- cleanup
-drop table range_parted cascade;
-NOTICE: drop cascades to 4 other objects
-DETAIL: drop cascades to table part_a_1_a_10
-drop cascades to table part_a_10_a_20
-drop cascades to table part_b_1_b_10
-drop cascades to table part_b_10_b_20
+drop table range_parted;
diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql
index 7513769..0787e78 100644
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -2194,10 +2194,8 @@ ALTER TABLE part_2 INHERIT inh_test;
ALTER TABLE list_parted2 DROP COLUMN b;
ALTER TABLE list_parted2 ALTER COLUMN b TYPE text;
--- cleanup: avoid using CASCADE
-DROP TABLE list_parted, part_1;
-DROP TABLE list_parted2, part_2, part_5, part_5_a;
-DROP TABLE range_parted, part1, part2;
+-- cleanup
+DROP TABLE list_parted, list_parted2, range_parted;
-- more tests for certain multi-level partitioning scenarios
create table p (a int, b int) partition by range (a, b);
@@ -2222,5 +2220,5 @@ insert into p1 (a, b) values (2, 3);
-- check that partition validation scan correctly detects violating rows
alter table p attach partition p1 for values from (1, 2) to (1, 10);
--- cleanup: avoid using CASCADE
-drop table p, p1, p11;
+-- cleanup
+drop table p;
diff --git a/src/test/regress/sql/create_table.sql b/src/test/regress/sql/create_table.sql
index 5f25c43..553c084 100644
--- a/src/test/regress/sql/create_table.sql
+++ b/src/test/regress/sql/create_table.sql
@@ -593,10 +593,5 @@ CREATE TABLE part_c_1_10 PARTITION OF part_c FOR VALUES FROM (1) TO (10);
-- returned.
\d parted
--- cleanup: avoid using CASCADE
-DROP TABLE parted, part_a, part_b, part_c, part_c_1_10;
-DROP TABLE list_parted, part_1, part_2, part_null;
-DROP TABLE range_parted;
-DROP TABLE list_parted2, part_ab, part_null_z;
-DROP TABLE range_parted2, part0, part1, part2, part3;
-DROP TABLE range_parted3, part00, part10, part11, part12;
+-- cleanup
+DROP TABLE parted, list_parted, range_parted, list_parted2, range_parted2, range_parted3;
diff --git a/src/test/regress/sql/inherit.sql b/src/test/regress/sql/inherit.sql
index a8b7eb1..836ec22 100644
--- a/src/test/regress/sql/inherit.sql
+++ b/src/test/regress/sql/inherit.sql
@@ -612,5 +612,5 @@ explain (costs off) select * from range_list_parted where b is null;
explain (costs off) select * from range_list_parted where a is not null and a < 67;
explain (costs off) select * from range_list_parted where a >= 30;
-drop table list_parted cascade;
-drop table range_list_parted cascade;
+drop table list_parted;
+drop table range_list_parted;
diff --git a/src/test/regress/sql/insert.sql b/src/test/regress/sql/insert.sql
index 454e1ce..dfdc24e 100644
--- a/src/test/regress/sql/insert.sql
+++ b/src/test/regress/sql/insert.sql
@@ -186,10 +186,7 @@ insert into list_parted (b) values (1);
select tableoid::regclass::text, a, min(b) as min_b, max(b) as max_b from list_parted group by 1, 2 order by 1;
-- cleanup
-drop table part1, part2, part3, part4, range_parted;
-drop table part_ee_ff3_1, part_ee_ff3_2, part_ee_ff1, part_ee_ff2, part_ee_ff3;
-drop table part_ee_ff, part_gg2_2, part_gg2_1, part_gg2, part_gg1, part_gg;
-drop table part_aa_bb, part_cc_dd, part_null, list_parted;
+drop table range_parted, list_parted;
-- more tests for certain multi-level partitioning scenarios
create table p (a int, b int) partition by range (a, b);
@@ -241,4 +238,4 @@ with ins (a, b, c) as
select a, b, min(c), max(c) from ins group by a, b order by 1;
-- cleanup
-drop table p, p1, p11, p12, p2, p3, p4;
+drop table p;
diff --git a/src/test/regress/sql/update.sql b/src/test/regress/sql/update.sql
index d7721ed..6637119 100644
--- a/src/test/regress/sql/update.sql
+++ b/src/test/regress/sql/update.sql
@@ -126,4 +126,4 @@ update range_parted set b = b - 1 where b = 10;
update range_parted set b = b + 1 where b = 10;
-- cleanup
-drop table range_parted cascade;
+drop table range_parted;
On 2017/02/22 21:24, Ashutosh Bapat wrote:
On Wed, Feb 22, 2017 at 11:11 AM, Amit Langote wrote:
+ /* + * Unlike inheritance children, partition tables are expected to be dropped + * when the parent partitioned table gets dropped. + */Hmm. Partitions *are* inheritance children, so we perhaps don't need the
part before the comma. Also, adding "automatically" somewhere in there
would be nice.Or, one could just write: /* add an auto dependency for partitions */
I changed it in the attached patch to + /* + * Partition tables are expected to be dropped when the parent partitioned + * table gets dropped. + */
OK, thanks.
Thanks,
Amit
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
I think this is ready for committer.
On Thu, Feb 23, 2017 at 12:02 PM, Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:
On 2017/02/22 21:24, Ashutosh Bapat wrote:
On Wed, Feb 22, 2017 at 11:11 AM, Amit Langote wrote:
+ /* + * Unlike inheritance children, partition tables are expected to be dropped + * when the parent partitioned table gets dropped. + */Hmm. Partitions *are* inheritance children, so we perhaps don't need the
part before the comma. Also, adding "automatically" somewhere in there
would be nice.Or, one could just write: /* add an auto dependency for partitions */
I changed it in the attached patch to + /* + * Partition tables are expected to be dropped when the parent partitioned + * table gets dropped. + */OK, thanks.
Thanks,
Amit
--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 23 February 2017 at 06:40, Ashutosh Bapat
<ashutosh.bapat@enterprisedb.com> wrote:
I think this is ready for committer.
Thanks for writing and reviewing this. I'll be happy to review and
commit. Please add to CF.
--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 2017/02/24 1:33, Simon Riggs wrote:
On 23 February 2017 at 06:40, Ashutosh Bapat
<ashutosh.bapat@enterprisedb.com> wrote:I think this is ready for committer.
Thanks for writing and reviewing this. I'll be happy to review and
commit. Please add to CF.
Thanks. I've added it to CF: https://commitfest.postgresql.org/13/1030/
Regards,
Amit
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 23 February 2017 at 16:33, Simon Riggs <simon@2ndquadrant.com> wrote:
I'll be happy to review
Patch looks OK so far, but fails on a partition that has partitions,
probably because of the way we test relkind in the call to
StoreCatalogInheritance1().
Please add a test for that so we can check automatically.
Thanks very much.
--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 2017/02/26 5:30, Simon Riggs wrote:
On 23 February 2017 at 16:33, Simon Riggs <simon@2ndquadrant.com> wrote:
I'll be happy to review
Patch looks OK so far, but fails on a partition that has partitions,
probably because of the way we test relkind in the call to
StoreCatalogInheritance1().
Thanks for the review.
I could not reproduce the failure you are seeing; could you perhaps share
the failing test case? Here's mine that seems to work as expected:
create table p (a int, b char) partition by list (a);
create table p1 (a int, b char) partition by list (b);
alter table p attach partition p1 for values in (1);
-- add a partition to p1
create table p1a (like p1);
alter table p1 attach partition p1a for values in ('a');
create table p2 partition of p for values in (1)
\d+ p
<snip>
Partition key: LIST (a)
Partitions: p1 FOR VALUES IN (1),
p2 FOR VALUES IN (2)
-- this works (remember that p1 is a partitioned table)
drop table p1;
DROP TABLE
\d+ p
<snip>
Partition key: LIST (a)
Partitions: p2 FOR VALUES IN (2)
Please add a test for that so we can check automatically.
OK, done.
Thanks,
Amit
Attachments:
0001-Allow-dropping-partitioned-table-without-CASCADE.patchtext/x-diff; name=0001-Allow-dropping-partitioned-table-without-CASCADE.patchDownload
From 419768af093d1d7b4bd9f57e2c9481227499aa1c Mon Sep 17 00:00:00 2001
From: amit <amitlangote09@gmail.com>
Date: Thu, 16 Feb 2017 15:56:44 +0900
Subject: [PATCH] Allow dropping partitioned table without CASCADE
Currently, a normal dependency is created between a inheritance
parent and child when creating the child. That means one must
specify CASCADE to drop the parent table if a child table exists.
When creating partitions as inheritance children, create auto
dependency instead, so that partitions get dropped automatically
when the parent is dropped i.e., without specifying CASCADE.
---
src/backend/commands/tablecmds.c | 30 ++++++++++++++++-------
src/test/regress/expected/alter_table.out | 10 ++++----
src/test/regress/expected/create_table.out | 38 ++++++++++++++++++++++++------
src/test/regress/expected/inherit.out | 22 ++---------------
src/test/regress/expected/insert.out | 7 ++----
src/test/regress/expected/update.out | 7 +-----
src/test/regress/sql/alter_table.sql | 10 ++++----
src/test/regress/sql/create_table.sql | 23 ++++++++++++------
src/test/regress/sql/inherit.sql | 4 ++--
src/test/regress/sql/insert.sql | 7 ++----
src/test/regress/sql/update.sql | 2 +-
11 files changed, 87 insertions(+), 73 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 3cea220421..3753d66c9e 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -289,9 +289,11 @@ static List *MergeAttributes(List *schema, List *supers, char relpersistence,
static bool MergeCheckConstraint(List *constraints, char *name, Node *expr);
static void MergeAttributesIntoExisting(Relation child_rel, Relation parent_rel);
static void MergeConstraintsIntoExisting(Relation child_rel, Relation parent_rel);
-static void StoreCatalogInheritance(Oid relationId, List *supers);
+static void StoreCatalogInheritance(Oid relationId, List *supers,
+ bool child_is_partition);
static void StoreCatalogInheritance1(Oid relationId, Oid parentOid,
- int16 seqNumber, Relation inhRelation);
+ int16 seqNumber, Relation inhRelation,
+ bool child_is_partition);
static int findAttrByName(const char *attributeName, List *schema);
static void AlterIndexNamespaces(Relation classRel, Relation rel,
Oid oldNspOid, Oid newNspOid, ObjectAddresses *objsMoved);
@@ -725,7 +727,7 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
typaddress);
/* Store inheritance information for new rel. */
- StoreCatalogInheritance(relationId, inheritOids);
+ StoreCatalogInheritance(relationId, inheritOids, stmt->partbound != NULL);
/*
* We must bump the command counter to make the newly-created relation
@@ -2240,7 +2242,8 @@ MergeCheckConstraint(List *constraints, char *name, Node *expr)
* supers is a list of the OIDs of the new relation's direct ancestors.
*/
static void
-StoreCatalogInheritance(Oid relationId, List *supers)
+StoreCatalogInheritance(Oid relationId, List *supers,
+ bool child_is_partition)
{
Relation relation;
int16 seqNumber;
@@ -2270,7 +2273,8 @@ StoreCatalogInheritance(Oid relationId, List *supers)
{
Oid parentOid = lfirst_oid(entry);
- StoreCatalogInheritance1(relationId, parentOid, seqNumber, relation);
+ StoreCatalogInheritance1(relationId, parentOid, seqNumber, relation,
+ child_is_partition);
seqNumber++;
}
@@ -2283,7 +2287,8 @@ StoreCatalogInheritance(Oid relationId, List *supers)
*/
static void
StoreCatalogInheritance1(Oid relationId, Oid parentOid,
- int16 seqNumber, Relation inhRelation)
+ int16 seqNumber, Relation inhRelation,
+ bool child_is_partition)
{
TupleDesc desc = RelationGetDescr(inhRelation);
Datum values[Natts_pg_inherits];
@@ -2317,7 +2322,14 @@ StoreCatalogInheritance1(Oid relationId, Oid parentOid,
childobject.objectId = relationId;
childobject.objectSubId = 0;
- recordDependencyOn(&childobject, &parentobject, DEPENDENCY_NORMAL);
+ /*
+ * Partitions are expected to get dropped automatically when the parent
+ * partitioned table is dropped.
+ */
+ if (child_is_partition)
+ recordDependencyOn(&childobject, &parentobject, DEPENDENCY_AUTO);
+ else
+ recordDependencyOn(&childobject, &parentobject, DEPENDENCY_NORMAL);
/*
* Post creation hook of this inheritance. Since object_access_hook
@@ -10744,7 +10756,9 @@ CreateInheritance(Relation child_rel, Relation parent_rel)
StoreCatalogInheritance1(RelationGetRelid(child_rel),
RelationGetRelid(parent_rel),
inhseqno + 1,
- catalogRelation);
+ catalogRelation,
+ parent_rel->rd_rel->relkind ==
+ RELKIND_PARTITIONED_TABLE);
/* Now we're done with pg_inherits */
heap_close(catalogRelation, RowExclusiveLock);
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index 9885fcba89..c15bbdcbd1 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -3339,10 +3339,8 @@ ALTER TABLE list_parted2 DROP COLUMN b;
ERROR: cannot drop column named in partition key
ALTER TABLE list_parted2 ALTER COLUMN b TYPE text;
ERROR: cannot alter type of column named in partition key
--- cleanup: avoid using CASCADE
-DROP TABLE list_parted, part_1;
-DROP TABLE list_parted2, part_2, part_5, part_5_a;
-DROP TABLE range_parted, part1, part2;
+-- cleanup
+DROP TABLE list_parted, list_parted2, range_parted;
-- more tests for certain multi-level partitioning scenarios
create table p (a int, b int) partition by range (a, b);
create table p1 (b int, a int not null) partition by range (b);
@@ -3371,5 +3369,5 @@ insert into p1 (a, b) values (2, 3);
-- check that partition validation scan correctly detects violating rows
alter table p attach partition p1 for values from (1, 2) to (1, 10);
ERROR: partition constraint is violated by some row
--- cleanup: avoid using CASCADE
-drop table p, p1, p11;
+-- cleanup
+drop table p;
diff --git a/src/test/regress/expected/create_table.out b/src/test/regress/expected/create_table.out
index 20eb3d35f9..7b72757988 100644
--- a/src/test/regress/expected/create_table.out
+++ b/src/test/regress/expected/create_table.out
@@ -667,10 +667,34 @@ Check constraints:
"check_a" CHECK (length(a) > 0)
Number of partitions: 3 (Use \d+ to list them.)
--- cleanup: avoid using CASCADE
-DROP TABLE parted, part_a, part_b, part_c, part_c_1_10;
-DROP TABLE list_parted, part_1, part_2, part_null;
-DROP TABLE range_parted;
-DROP TABLE list_parted2, part_ab, part_null_z;
-DROP TABLE range_parted2, part0, part1, part2, part3;
-DROP TABLE range_parted3, part00, part10, part11, part12;
+-- cleanup
+DROP TABLE parted, list_parted, range_parted, list_parted2, range_parted2, range_parted3;
+-- a test to check dropping partitioned table works
+create table drop_parted (a int, b char) partition by list (a);
+create table drop_parted1 (a int, b char) partition by list (b);
+alter table drop_parted attach partition drop_parted1 for values in (1);
+create table drop_parted1a (like drop_parted1);
+alter table drop_parted1 attach partition drop_parted1a for values in ('a');
+create table drop_parted2 partition of drop_parted for values in (2);
+\d+ drop_parted
+ Table "public.drop_parted"
+ Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
+--------+--------------+-----------+----------+---------+----------+--------------+-------------
+ a | integer | | | | plain | |
+ b | character(1) | | | | extended | |
+Partition key: LIST (a)
+Partitions: drop_parted1 FOR VALUES IN (1),
+ drop_parted2 FOR VALUES IN (2)
+
+drop table drop_parted1;
+\d+ drop_parted
+ Table "public.drop_parted"
+ Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
+--------+--------------+-----------+----------+---------+----------+--------------+-------------
+ a | integer | | | | plain | |
+ b | character(1) | | | | extended | |
+Partition key: LIST (a)
+Partitions: drop_parted2 FOR VALUES IN (2)
+
+-- cleanup (will take drop_parted2 with it)
+drop table drop_parted;
diff --git a/src/test/regress/expected/inherit.out b/src/test/regress/expected/inherit.out
index a8c8b28a75..795d9f575c 100644
--- a/src/test/regress/expected/inherit.out
+++ b/src/test/regress/expected/inherit.out
@@ -1843,23 +1843,5 @@ explain (costs off) select * from range_list_parted where a >= 30;
Filter: (a >= 30)
(11 rows)
-drop table list_parted cascade;
-NOTICE: drop cascades to 3 other objects
-DETAIL: drop cascades to table part_ab_cd
-drop cascades to table part_ef_gh
-drop cascades to table part_null_xy
-drop table range_list_parted cascade;
-NOTICE: drop cascades to 13 other objects
-DETAIL: drop cascades to table part_1_10
-drop cascades to table part_1_10_ab
-drop cascades to table part_1_10_cd
-drop cascades to table part_10_20
-drop cascades to table part_10_20_ab
-drop cascades to table part_10_20_cd
-drop cascades to table part_21_30
-drop cascades to table part_21_30_ab
-drop cascades to table part_21_30_cd
-drop cascades to table part_40_inf
-drop cascades to table part_40_inf_ab
-drop cascades to table part_40_inf_cd
-drop cascades to table part_40_inf_null
+drop table list_parted;
+drop table range_list_parted;
diff --git a/src/test/regress/expected/insert.out b/src/test/regress/expected/insert.out
index 81af3ef497..31cfa4e76e 100644
--- a/src/test/regress/expected/insert.out
+++ b/src/test/regress/expected/insert.out
@@ -314,10 +314,7 @@ select tableoid::regclass::text, a, min(b) as min_b, max(b) as max_b from list_p
(9 rows)
-- cleanup
-drop table part1, part2, part3, part4, range_parted;
-drop table part_ee_ff3_1, part_ee_ff3_2, part_ee_ff1, part_ee_ff2, part_ee_ff3;
-drop table part_ee_ff, part_gg2_2, part_gg2_1, part_gg2, part_gg1, part_gg;
-drop table part_aa_bb, part_cc_dd, part_null, list_parted;
+drop table range_parted, list_parted;
-- more tests for certain multi-level partitioning scenarios
create table p (a int, b int) partition by range (a, b);
create table p1 (b int not null, a int not null) partition by range ((b+0));
@@ -387,4 +384,4 @@ with ins (a, b, c) as
(5 rows)
-- cleanup
-drop table p, p1, p11, p12, p2, p3, p4;
+drop table p;
diff --git a/src/test/regress/expected/update.out b/src/test/regress/expected/update.out
index a1e9255450..9366f04255 100644
--- a/src/test/regress/expected/update.out
+++ b/src/test/regress/expected/update.out
@@ -219,9 +219,4 @@ DETAIL: Failing row contains (b, 9).
-- ok
update range_parted set b = b + 1 where b = 10;
-- cleanup
-drop table range_parted cascade;
-NOTICE: drop cascades to 4 other objects
-DETAIL: drop cascades to table part_a_1_a_10
-drop cascades to table part_a_10_a_20
-drop cascades to table part_b_1_b_10
-drop cascades to table part_b_10_b_20
+drop table range_parted;
diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql
index f7b754f0be..37f327bf6d 100644
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -2199,10 +2199,8 @@ ALTER TABLE part_2 INHERIT inh_test;
ALTER TABLE list_parted2 DROP COLUMN b;
ALTER TABLE list_parted2 ALTER COLUMN b TYPE text;
--- cleanup: avoid using CASCADE
-DROP TABLE list_parted, part_1;
-DROP TABLE list_parted2, part_2, part_5, part_5_a;
-DROP TABLE range_parted, part1, part2;
+-- cleanup
+DROP TABLE list_parted, list_parted2, range_parted;
-- more tests for certain multi-level partitioning scenarios
create table p (a int, b int) partition by range (a, b);
@@ -2227,5 +2225,5 @@ insert into p1 (a, b) values (2, 3);
-- check that partition validation scan correctly detects violating rows
alter table p attach partition p1 for values from (1, 2) to (1, 10);
--- cleanup: avoid using CASCADE
-drop table p, p1, p11;
+-- cleanup
+drop table p;
diff --git a/src/test/regress/sql/create_table.sql b/src/test/regress/sql/create_table.sql
index f41dd71475..93c9b245d2 100644
--- a/src/test/regress/sql/create_table.sql
+++ b/src/test/regress/sql/create_table.sql
@@ -595,10 +595,19 @@ CREATE TABLE part_c_1_10 PARTITION OF part_c FOR VALUES FROM (1) TO (10);
-- returned.
\d parted
--- cleanup: avoid using CASCADE
-DROP TABLE parted, part_a, part_b, part_c, part_c_1_10;
-DROP TABLE list_parted, part_1, part_2, part_null;
-DROP TABLE range_parted;
-DROP TABLE list_parted2, part_ab, part_null_z;
-DROP TABLE range_parted2, part0, part1, part2, part3;
-DROP TABLE range_parted3, part00, part10, part11, part12;
+-- cleanup
+DROP TABLE parted, list_parted, range_parted, list_parted2, range_parted2, range_parted3;
+
+-- a test to check dropping partitioned table works
+create table drop_parted (a int, b char) partition by list (a);
+create table drop_parted1 (a int, b char) partition by list (b);
+alter table drop_parted attach partition drop_parted1 for values in (1);
+create table drop_parted1a (like drop_parted1);
+alter table drop_parted1 attach partition drop_parted1a for values in ('a');
+create table drop_parted2 partition of drop_parted for values in (2);
+\d+ drop_parted
+drop table drop_parted1;
+\d+ drop_parted
+
+-- cleanup (will take drop_parted2 with it)
+drop table drop_parted;
diff --git a/src/test/regress/sql/inherit.sql b/src/test/regress/sql/inherit.sql
index a8b7eb1c8d..836ec22c20 100644
--- a/src/test/regress/sql/inherit.sql
+++ b/src/test/regress/sql/inherit.sql
@@ -612,5 +612,5 @@ explain (costs off) select * from range_list_parted where b is null;
explain (costs off) select * from range_list_parted where a is not null and a < 67;
explain (costs off) select * from range_list_parted where a >= 30;
-drop table list_parted cascade;
-drop table range_list_parted cascade;
+drop table list_parted;
+drop table range_list_parted;
diff --git a/src/test/regress/sql/insert.sql b/src/test/regress/sql/insert.sql
index 454e1ce2e7..dfdc24eba8 100644
--- a/src/test/regress/sql/insert.sql
+++ b/src/test/regress/sql/insert.sql
@@ -186,10 +186,7 @@ insert into list_parted (b) values (1);
select tableoid::regclass::text, a, min(b) as min_b, max(b) as max_b from list_parted group by 1, 2 order by 1;
-- cleanup
-drop table part1, part2, part3, part4, range_parted;
-drop table part_ee_ff3_1, part_ee_ff3_2, part_ee_ff1, part_ee_ff2, part_ee_ff3;
-drop table part_ee_ff, part_gg2_2, part_gg2_1, part_gg2, part_gg1, part_gg;
-drop table part_aa_bb, part_cc_dd, part_null, list_parted;
+drop table range_parted, list_parted;
-- more tests for certain multi-level partitioning scenarios
create table p (a int, b int) partition by range (a, b);
@@ -241,4 +238,4 @@ with ins (a, b, c) as
select a, b, min(c), max(c) from ins group by a, b order by 1;
-- cleanup
-drop table p, p1, p11, p12, p2, p3, p4;
+drop table p;
diff --git a/src/test/regress/sql/update.sql b/src/test/regress/sql/update.sql
index d7721ed376..663711997b 100644
--- a/src/test/regress/sql/update.sql
+++ b/src/test/regress/sql/update.sql
@@ -126,4 +126,4 @@ update range_parted set b = b - 1 where b = 10;
update range_parted set b = b + 1 where b = 10;
-- cleanup
-drop table range_parted cascade;
+drop table range_parted;
--
2.11.0
On Mon, Feb 27, 2017 at 8:08 AM, Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:
On 2017/02/26 5:30, Simon Riggs wrote:
On 23 February 2017 at 16:33, Simon Riggs <simon@2ndquadrant.com> wrote:
I'll be happy to review
Patch looks OK so far, but fails on a partition that has partitions,
probably because of the way we test relkind in the call to
StoreCatalogInheritance1().Thanks for the review.
I could not reproduce the failure you are seeing; could you perhaps share
the failing test case? Here's mine that seems to work as expected:create table p (a int, b char) partition by list (a);
create table p1 (a int, b char) partition by list (b);
alter table p attach partition p1 for values in (1);-- add a partition to p1
create table p1a (like p1);
alter table p1 attach partition p1a for values in ('a');create table p2 partition of p for values in (1)
\d+ p
<snip>
Partition key: LIST (a)
Partitions: p1 FOR VALUES IN (1),
p2 FOR VALUES IN (2)-- this works (remember that p1 is a partitioned table)
drop table p1;
DROP TABLE\d+ p
<snip>
Partition key: LIST (a)
Partitions: p2 FOR VALUES IN (2)Please add a test for that so we can check automatically.
OK, done.
Isn't list_range_parted multilevel partitioned table. It gets dropped
in the testcases. So, I guess, we already have a testcase there.
--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 2017/02/27 13:35, Ashutosh Bapat wrote:
On Mon, Feb 27, 2017 at 8:08 AM, Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:On 2017/02/26 5:30, Simon Riggs wrote:
On 23 February 2017 at 16:33, Simon Riggs <simon@2ndquadrant.com> wrote:
I'll be happy to review
Patch looks OK so far, but fails on a partition that has partitions,
probably because of the way we test relkind in the call to
StoreCatalogInheritance1().Thanks for the review.
I could not reproduce the failure you are seeing; could you perhaps share
the failing test case? Here's mine that seems to work as expected:create table p (a int, b char) partition by list (a);
create table p1 (a int, b char) partition by list (b);
alter table p attach partition p1 for values in (1);-- add a partition to p1
create table p1a (like p1);
alter table p1 attach partition p1a for values in ('a');create table p2 partition of p for values in (1)
\d+ p
<snip>
Partition key: LIST (a)
Partitions: p1 FOR VALUES IN (1),
p2 FOR VALUES IN (2)-- this works (remember that p1 is a partitioned table)
drop table p1;
DROP TABLE\d+ p
<snip>
Partition key: LIST (a)
Partitions: p2 FOR VALUES IN (2)Please add a test for that so we can check automatically.
OK, done.
Isn't list_range_parted multilevel partitioned table. It gets dropped
in the testcases. So, I guess, we already have a testcase there.
I thought Simon meant the test case where a partition that is itself
partitioned is dropped. At least that's what I took from "... fails *on*
partition that has partitions". So in the example I posted, drop table p1.
Anyway, there might be the confusion that *only* the root level
partitioned table is of RELKIND_PARTITIONED_TABLE. That's not true - any
partitioned table (even one that's a partition) is of that relkind. So
the condition in the call to StoreCatalogInheritance1() is correct. The
following hunk:
@@ -10744,7 +10756,9 @@ CreateInheritance(Relation child_rel, Relation
parent_rel)
StoreCatalogInheritance1(RelationGetRelid(child_rel),
RelationGetRelid(parent_rel),
inhseqno + 1,
- catalogRelation);
+ catalogRelation,
+ parent_rel->rd_rel->relkind ==
+ RELKIND_PARTITIONED_TABLE);Thanks,
Amit
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Isn't list_range_parted multilevel partitioned table. It gets dropped
in the testcases. So, I guess, we already have a testcase there.I thought Simon meant the test case where a partition that is itself
partitioned is dropped. At least that's what I took from "... fails *on*
partition that has partitions". So in the example I posted, drop table p1.
Ok. Thanks for the explanation.
Anyway, there might be the confusion that *only* the root level
partitioned table is of RELKIND_PARTITIONED_TABLE. That's not true - any
partitioned table (even one that's a partition) is of that relkind. So
the condition in the call to StoreCatalogInheritance1() is correct. The
following hunk:@@ -10744,7 +10756,9 @@ CreateInheritance(Relation child_rel, Relation parent_rel) StoreCatalogInheritance1(RelationGetRelid(child_rel), RelationGetRelid(parent_rel), inhseqno + 1, - catalogRelation); + catalogRelation, + parent_rel->rd_rel->relkind == + RELKIND_PARTITIONED_TABLE);Thanks,
Amit
I agree.
--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 27 February 2017 at 02:38, Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:
On 2017/02/26 5:30, Simon Riggs wrote:
On 23 February 2017 at 16:33, Simon Riggs <simon@2ndquadrant.com> wrote:
I'll be happy to review
Patch looks OK so far, but fails on a partition that has partitions,
probably because of the way we test relkind in the call to
StoreCatalogInheritance1().Thanks for the review.
I could not reproduce the failure you are seeing; could you perhaps share
the failing test case?
I used a slight modification of the case mentioned on the docs. I
confirm this fails repeatably for me on current HEAD.
CREATE TABLE cities (
city_id bigserial not null,
name text not null,
population bigint
) PARTITION BY LIST (left(lower(name), 1));
CREATE TABLE cities_ab
PARTITION OF cities (
CONSTRAINT city_id_nonzero CHECK (city_id != 0)
) FOR VALUES IN ('a', 'b')
PARTITION BY RANGE (population);
drop table cities;
ERROR: cannot drop table cities because other objects depend on it
DETAIL: table cities_ab depends on table cities
HINT: Use DROP ... CASCADE to drop the dependent objects too.
I notice also that
\d+ <tablename>
does not show which partitions have subpartitions.
I'm worried that these things illustrate something about the catalog
representation that we may need to improve, but I don't have anything
concrete to say on that at present.
--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
I used a slight modification of the case mentioned on the docs. I
confirm this fails repeatably for me on current HEAD.CREATE TABLE cities (
city_id bigserial not null,
name text not null,
population bigint
) PARTITION BY LIST (left(lower(name), 1));CREATE TABLE cities_ab
PARTITION OF cities (
CONSTRAINT city_id_nonzero CHECK (city_id != 0)
) FOR VALUES IN ('a', 'b')
PARTITION BY RANGE (population);drop table cities;
ERROR: cannot drop table cities because other objects depend on it
DETAIL: table cities_ab depends on table cities
HINT: Use DROP ... CASCADE to drop the dependent objects too.
I think that's what this patch fixes. Do you see this behaviour after
applying the patch?
I notice also that
\d+ <tablename>
does not show which partitions have subpartitions.
I will confirm this once I have access to the code.
I'm worried that these things illustrate something about the catalog
representation that we may need to improve, but I don't have anything
concrete to say on that at present.
AFAIK, catalogs have everything needed to fix this; it's just the
matter to using that information wherever it's needed.
--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 5 March 2017 at 07:59, Ashutosh Bapat
<ashutosh.bapat@enterprisedb.com> wrote:
I used a slight modification of the case mentioned on the docs. I
confirm this fails repeatably for me on current HEAD.CREATE TABLE cities (
city_id bigserial not null,
name text not null,
population bigint
) PARTITION BY LIST (left(lower(name), 1));CREATE TABLE cities_ab
PARTITION OF cities (
CONSTRAINT city_id_nonzero CHECK (city_id != 0)
) FOR VALUES IN ('a', 'b')
PARTITION BY RANGE (population);drop table cities;
ERROR: cannot drop table cities because other objects depend on it
DETAIL: table cities_ab depends on table cities
HINT: Use DROP ... CASCADE to drop the dependent objects too.I think that's what this patch fixes. Do you see this behaviour after
applying the patch?
It does seems as if I've made a mistake there. The patch passes.
Thanks for checking.
I will apply tomorrow if no further comments.
--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 2017/03/05 16:20, Simon Riggs wrote:
I notice also that
\d+ <tablename>
does not show which partitions have subpartitions.
Do you mean showing just whether a partition is itself partitioned or
showing its partitions and so on (because those partitions may themselves
be partitioned)? Maybe, we could do the former.
I'm worried that these things illustrate something about the catalog
representation that we may need to improve, but I don't have anything
concrete to say on that at present.
Perhaps. As Ashutosh said though, it does not seem like a big problem in
this particular case.
Thanks,
Amit
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 6 March 2017 at 00:51, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote:
On 2017/03/05 16:20, Simon Riggs wrote:
I notice also that
\d+ <tablename>
does not show which partitions have subpartitions.Do you mean showing just whether a partition is itself partitioned or
showing its partitions and so on (because those partitions may themselves
be partitioned)? Maybe, we could do the former.
I think \d+ should show the full information, in some form.
--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Mon, Mar 6, 2017 at 8:35 AM, Simon Riggs <simon@2ndquadrant.com> wrote:
On 6 March 2017 at 00:51, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote:
On 2017/03/05 16:20, Simon Riggs wrote:
I notice also that
\d+ <tablename>
does not show which partitions have subpartitions.Do you mean showing just whether a partition is itself partitioned or
showing its partitions and so on (because those partitions may themselves
be partitioned)? Maybe, we could do the former.I think \d+ should show the full information, in some form.
For a multi-level inheritance hierarchy, we don't show children which
are further inherited. Same behaviour has been carried over to
partitioning. I don't say that that's good or bad.
Given the recursive structure of partitioned tables, it looks readable
and manageable to print only the direct partitions in \d+. May be we
want to indicate the partitions that are further partitioned. If user
wants information about partitioned partitions, s/he can execute \d+
on the partition, repeating this process to any desired level. This
would work well in the interactive mode, keeping the output of \d+
manageable. Further someone writing a script to consume \d+ output of
a multi-level partitioned table, can code the above process in a
script.
Thinking about how to display partition which are further partitioned,
there are two options. Assume a partitioned table t1 with partitions
t1p1, which is further partitioned and t1p2. One could display \d+ t1
as
\d+ t1
Table "public.t1"
Column | Type | Collation | Nullable | Default | Storage | Stats
target | Description
--------+---------+-----------+----------+---------+---------+--------------+-------------
a | integer | | not null | | plain | |
Partition key: RANGE (a)
Partitions: t1p1 FOR VALUES FROM (0) TO (100), HAS PARTITIONS
t1p2 FOR VALUES FROM (100) TO (200)
OR
\d+ t1
Table "public.t1"
Column | Type | Collation | Nullable | Default | Storage | Stats
target | Description
--------+---------+-----------+----------+---------+---------+--------------+-------------
a | integer | | not null | | plain | |
Partition key: RANGE (a)
Partitions: t1p1 FOR VALUES FROM (0) TO (100), PARTITION BY LIST(a)
t1p2 FOR VALUES FROM (100) TO (200)
To me the first option looks fine. If the user is interested in
looking at the subpartitioned in any case s/he will have to execute
\d+. So beyond indicating that a partition has subpartitions, what
other information to include is debatable, given that \d+ on that
partition is going to print it anyway.
--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 6 March 2017 at 04:00, Ashutosh Bapat
<ashutosh.bapat@enterprisedb.com> wrote:
On Mon, Mar 6, 2017 at 8:35 AM, Simon Riggs <simon@2ndquadrant.com> wrote:
On 6 March 2017 at 00:51, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote:
On 2017/03/05 16:20, Simon Riggs wrote:
I notice also that
\d+ <tablename>
does not show which partitions have subpartitions.Do you mean showing just whether a partition is itself partitioned or
showing its partitions and so on (because those partitions may themselves
be partitioned)? Maybe, we could do the former.I think \d+ should show the full information, in some form.
For a multi-level inheritance hierarchy, we don't show children which
are further inherited. Same behaviour has been carried over to
partitioning. I don't say that that's good or bad.Given the recursive structure of partitioned tables, it looks readable
and manageable to print only the direct partitions in \d+. May be we
want to indicate the partitions that are further partitioned. If user
wants information about partitioned partitions, s/he can execute \d+
on the partition, repeating this process to any desired level. This
would work well in the interactive mode, keeping the output of \d+
manageable. Further someone writing a script to consume \d+ output of
a multi-level partitioned table, can code the above process in a
script.Thinking about how to display partition which are further partitioned,
there are two options. Assume a partitioned table t1 with partitions
t1p1, which is further partitioned and t1p2. One could display \d+ t1
as\d+ t1
Table "public.t1"
Column | Type | Collation | Nullable | Default | Storage | Stats
target | Description
--------+---------+-----------+----------+---------+---------+--------------+-------------
a | integer | | not null | | plain | |
Partition key: RANGE (a)
Partitions: t1p1 FOR VALUES FROM (0) TO (100), HAS PARTITIONS
t1p2 FOR VALUES FROM (100) TO (200)OR
\d+ t1
Table "public.t1"
Column | Type | Collation | Nullable | Default | Storage | Stats
target | Description
--------+---------+-----------+----------+---------+---------+--------------+-------------
a | integer | | not null | | plain | |
Partition key: RANGE (a)
Partitions: t1p1 FOR VALUES FROM (0) TO (100), PARTITION BY LIST(a)
t1p2 FOR VALUES FROM (100) TO (200)To me the first option looks fine.
+1
lowercase please
--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Mon, Mar 6, 2017 at 10:55 AM, Simon Riggs <simon@2ndquadrant.com> wrote:
On 6 March 2017 at 04:00, Ashutosh Bapat
<ashutosh.bapat@enterprisedb.com> wrote:On Mon, Mar 6, 2017 at 8:35 AM, Simon Riggs <simon@2ndquadrant.com> wrote:
On 6 March 2017 at 00:51, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote:
On 2017/03/05 16:20, Simon Riggs wrote:
I notice also that
\d+ <tablename>
does not show which partitions have subpartitions.Do you mean showing just whether a partition is itself partitioned or
showing its partitions and so on (because those partitions may themselves
be partitioned)? Maybe, we could do the former.I think \d+ should show the full information, in some form.
For a multi-level inheritance hierarchy, we don't show children which
are further inherited. Same behaviour has been carried over to
partitioning. I don't say that that's good or bad.Given the recursive structure of partitioned tables, it looks readable
and manageable to print only the direct partitions in \d+. May be we
want to indicate the partitions that are further partitioned. If user
wants information about partitioned partitions, s/he can execute \d+
on the partition, repeating this process to any desired level. This
would work well in the interactive mode, keeping the output of \d+
manageable. Further someone writing a script to consume \d+ output of
a multi-level partitioned table, can code the above process in a
script.Thinking about how to display partition which are further partitioned,
there are two options. Assume a partitioned table t1 with partitions
t1p1, which is further partitioned and t1p2. One could display \d+ t1
as\d+ t1
Table "public.t1"
Column | Type | Collation | Nullable | Default | Storage | Stats
target | Description
--------+---------+-----------+----------+---------+---------+--------------+-------------
a | integer | | not null | | plain | |
Partition key: RANGE (a)
Partitions: t1p1 FOR VALUES FROM (0) TO (100), HAS PARTITIONS
t1p2 FOR VALUES FROM (100) TO (200)OR
\d+ t1
Table "public.t1"
Column | Type | Collation | Nullable | Default | Storage | Stats
target | Description
--------+---------+-----------+----------+---------+---------+--------------+-------------
a | integer | | not null | | plain | |
Partition key: RANGE (a)
Partitions: t1p1 FOR VALUES FROM (0) TO (100), PARTITION BY LIST(a)
t1p2 FOR VALUES FROM (100) TO (200)To me the first option looks fine.
+1
Just to confirm, you want the output to look like this
\d+ t1
Table "public.t1"
Column | Type | Collation | Nullable | Default | Storage | Stats
target | Description
--------+---------+-----------+----------+---------+---------+--------------+-------------
a | integer | | not null | | plain | |
Partition key: RANGE (a)
Partitions: t1p1 FOR VALUES FROM (0) TO (100), HAS PARTITIONS
t1p2 FOR VALUES FROM (100) TO (200)
lowercase please
Except for HAS PARTITIONS, everything is part of today's output. Given
the current output, HAS PARTITIONS should be in upper case.
--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 6 March 2017 at 05:29, Ashutosh Bapat
<ashutosh.bapat@enterprisedb.com> wrote:
Just to confirm, you want the output to look like this
\d+ t1
Table "public.t1"
Column | Type | Collation | Nullable | Default | Storage | Stats
target | Description
--------+---------+-----------+----------+---------+---------+--------------+-------------
a | integer | | not null | | plain | |
Partition key: RANGE (a)
Partitions: t1p1 FOR VALUES FROM (0) TO (100), HAS PARTITIONS
t1p2 FOR VALUES FROM (100) TO (200)lowercase please
Except for HAS PARTITIONS, everything is part of today's output. Given
the current output, HAS PARTITIONS should be in upper case.
"has partitions" is not part of the DDL, whereas "FOR VALUES FROM (0)
TO (100)" is. So ISTM sensible to differentiate between DDL and
non-ddl using upper and lower case.
--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Mon, Mar 6, 2017 at 11:05 AM, Simon Riggs <simon@2ndquadrant.com> wrote:
On 6 March 2017 at 05:29, Ashutosh Bapat
<ashutosh.bapat@enterprisedb.com> wrote:Just to confirm, you want the output to look like this
\d+ t1
Table "public.t1"
Column | Type | Collation | Nullable | Default | Storage | Stats
target | Description
--------+---------+-----------+----------+---------+---------+--------------+-------------
a | integer | | not null | | plain | |
Partition key: RANGE (a)
Partitions: t1p1 FOR VALUES FROM (0) TO (100), HAS PARTITIONS
t1p2 FOR VALUES FROM (100) TO (200)lowercase please
Except for HAS PARTITIONS, everything is part of today's output. Given
the current output, HAS PARTITIONS should be in upper case."has partitions" is not part of the DDL, whereas "FOR VALUES FROM (0)
TO (100)" is. So ISTM sensible to differentiate between DDL and
non-ddl using upper and lower case.
Make sense. Will try to cook up a patch soon.
--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 2017/03/06 14:25, Simon Riggs wrote:
On 6 March 2017 at 04:00, Ashutosh Bapat wrote:
Thinking about how to display partition which are further partitioned,
there are two options. Assume a partitioned table t1 with partitions
t1p1, which is further partitioned and t1p2. One could display \d+ t1
as\d+ t1
Table "public.t1"
Column | Type | Collation | Nullable | Default | Storage | Stats
target | Description
--------+---------+-----------+----------+---------+---------+--------------+-------------
a | integer | | not null | | plain | |
Partition key: RANGE (a)
Partitions: t1p1 FOR VALUES FROM (0) TO (100), HAS PARTITIONS
t1p2 FOR VALUES FROM (100) TO (200)OR
\d+ t1
Table "public.t1"
Column | Type | Collation | Nullable | Default | Storage | Stats
target | Description
--------+---------+-----------+----------+---------+---------+--------------+-------------
a | integer | | not null | | plain | |
Partition key: RANGE (a)
Partitions: t1p1 FOR VALUES FROM (0) TO (100), PARTITION BY LIST(a)
t1p2 FOR VALUES FROM (100) TO (200)To me the first option looks fine.
+1
lowercase please
+1
Thanks,
Amit
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Mon, Mar 6, 2017 at 11:12 AM, Ashutosh Bapat
<ashutosh.bapat@enterprisedb.com> wrote:
On Mon, Mar 6, 2017 at 11:05 AM, Simon Riggs <simon@2ndquadrant.com> wrote:
On 6 March 2017 at 05:29, Ashutosh Bapat
<ashutosh.bapat@enterprisedb.com> wrote:Just to confirm, you want the output to look like this
\d+ t1
Table "public.t1"
Column | Type | Collation | Nullable | Default | Storage | Stats
target | Description
--------+---------+-----------+----------+---------+---------+--------------+-------------
a | integer | | not null | | plain | |
Partition key: RANGE (a)
Partitions: t1p1 FOR VALUES FROM (0) TO (100), HAS PARTITIONS
t1p2 FOR VALUES FROM (100) TO (200)lowercase please
Except for HAS PARTITIONS, everything is part of today's output. Given
the current output, HAS PARTITIONS should be in upper case."has partitions" is not part of the DDL, whereas "FOR VALUES FROM (0)
TO (100)" is. So ISTM sensible to differentiate between DDL and
non-ddl using upper and lower case.Make sense. Will try to cook up a patch soon.
here's the patch. I have added a testcase in insert.sql to test \d+
output for a partitioned table which has partitions which are further
partitioned and also some partitions which are not partitioned
themselves. I have also refactored a statement few lines above,
replacing an if condition with ? : operator similar to code few lines
below.
--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company
Attachments:
pg_part_desc.patchapplication/octet-stream; name=pg_part_desc.patchDownload
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index e2e4cbc..f8617cf 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -2684,7 +2684,7 @@ describeOneTableDetails(const char *schemaname,
/* print child tables (with additional info if partitions) */
if (pset.sversion >= 100000)
printfPQExpBuffer(&buf,
- "SELECT c.oid::pg_catalog.regclass, pg_get_expr(c.relpartbound, c.oid)"
+ "SELECT c.oid::pg_catalog.regclass, pg_get_expr(c.relpartbound, c.oid), c.relkind"
" FROM pg_catalog.pg_class c, pg_catalog.pg_inherits i"
" WHERE c.oid=i.inhrelid AND"
" i.inhparent = '%s' AND"
@@ -2709,13 +2709,12 @@ describeOneTableDetails(const char *schemaname,
if (!verbose)
{
+ const char *ct = tableinfo.relkind != 'P' ? _("child tables") : _("partitions");
+
/* print the number of child tables, if any */
if (tuples > 0)
{
- if (tableinfo.relkind != 'P')
- printfPQExpBuffer(&buf, _("Number of child tables: %d (Use \\d+ to list them.)"), tuples);
- else
- printfPQExpBuffer(&buf, _("Number of partitions: %d (Use \\d+ to list them.)"), tuples);
+ printfPQExpBuffer(&buf, _("Number of %s: %d (Use \\d+ to list them.)"), ct, tuples);
printTableAddFooter(&cont, buf.data);
}
}
@@ -2738,12 +2737,21 @@ describeOneTableDetails(const char *schemaname,
}
else
{
+ char *partitioned_note;
+
+ if (*PQgetvalue(result, i, 2) == 'P')
+ partitioned_note = " has partitions";
+ else
+ partitioned_note = "";
+
if (i == 0)
- printfPQExpBuffer(&buf, "%s: %s %s",
- ct, PQgetvalue(result, i, 0), PQgetvalue(result, i, 1));
+ printfPQExpBuffer(&buf, "%s: %s %s%s",
+ ct, PQgetvalue(result, i, 0), PQgetvalue(result, i, 1),
+ partitioned_note);
else
- printfPQExpBuffer(&buf, "%*s %s %s",
- ctw, "", PQgetvalue(result, i, 0), PQgetvalue(result, i, 1));
+ printfPQExpBuffer(&buf, "%*s %s %s%s",
+ ctw, "", PQgetvalue(result, i, 0), PQgetvalue(result, i, 1),
+ partitioned_note);
}
if (i < tuples - 1)
appendPQExpBufferChar(&buf, ',');
diff --git a/src/test/regress/expected/insert.out b/src/test/regress/expected/insert.out
index 81af3ef..40f1c5e 100644
--- a/src/test/regress/expected/insert.out
+++ b/src/test/regress/expected/insert.out
@@ -313,6 +313,21 @@ select tableoid::regclass::text, a, min(b) as min_b, max(b) as max_b from list_p
part_null | | 1 | 1
(9 rows)
+-- test \d+ output on a table which has both partitioned and unpartitioned
+-- partitions
+\d+ list_parted
+ Table "public.list_parted"
+ Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
+--------+---------+-----------+----------+---------+----------+--------------+-------------
+ a | text | | | | extended | |
+ b | integer | | | | plain | |
+Partition key: LIST (lower(a))
+Partitions: part_aa_bb FOR VALUES IN ('aa', 'bb'),
+ part_cc_dd FOR VALUES IN ('cc', 'dd'),
+ part_ee_ff FOR VALUES IN ('ee', 'ff') has partitions,
+ part_gg FOR VALUES IN ('gg') has partitions,
+ part_null FOR VALUES IN (NULL)
+
-- cleanup
drop table part1, part2, part3, part4, range_parted;
drop table part_ee_ff3_1, part_ee_ff3_2, part_ee_ff1, part_ee_ff2, part_ee_ff3;
diff --git a/src/test/regress/sql/insert.sql b/src/test/regress/sql/insert.sql
index 454e1ce..855a364 100644
--- a/src/test/regress/sql/insert.sql
+++ b/src/test/regress/sql/insert.sql
@@ -185,6 +185,10 @@ insert into list_parted select 'gg', s.a from generate_series(1, 9) s(a);
insert into list_parted (b) values (1);
select tableoid::regclass::text, a, min(b) as min_b, max(b) as max_b from list_parted group by 1, 2 order by 1;
+-- test \d+ output on a table which has both partitioned and unpartitioned
+-- partitions
+\d+ list_parted
+
-- cleanup
drop table part1, part2, part3, part4, range_parted;
drop table part_ee_ff3_1, part_ee_ff3_2, part_ee_ff1, part_ee_ff2, part_ee_ff3;
Hi Ashutosh,
On 2017/03/06 18:19, Ashutosh Bapat wrote:
On Mon, Mar 6, 2017 at 11:12 AM, Ashutosh Bapat wrote:
On Mon, Mar 6, 2017 at 11:05 AM, Simon Riggs <simon@2ndquadrant.com> wrote:
"has partitions" is not part of the DDL, whereas "FOR VALUES FROM (0)
TO (100)" is. So ISTM sensible to differentiate between DDL and
non-ddl using upper and lower case.Make sense. Will try to cook up a patch soon.
here's the patch. I have added a testcase in insert.sql to test \d+
output for a partitioned table which has partitions which are further
partitioned and also some partitions which are not partitioned
themselves. I have also refactored a statement few lines above,
replacing an if condition with ? : operator similar to code few lines
below.
Thanks for cooking up the patch. Looks good and works as expected.
Regression tests pass.
About the other statement you changed, I just realized that we should
perhaps do one more thing. Show the Number of partitions, even if it's 0.
In case of inheritance, the parent table stands on its own when there are
no child tables, but a partitioned table doesn't in the same sense. I
tried to implement that in attached patch 0002. Example below:
create table p (a int) partition by list (a);
\d p
<snip>
Partition key: LIST (a)
Number of partitions: 0
\d+ p
<snip>
Partition key: LIST (a)
Number of partitions: 0
create table p1 partition of p for values in (1);
\d p
<snip>
Partition key: LIST (a)
Number of partitions: 1 (Use \d+ to list them.)
\d+ p
<snip>
Partition key: LIST (a)
Partitions: p1 FOR VALUES IN (1)
0001 is your original patch.
Thanks,
Amit
Attachments:
0001-Ashutosh-s-patch-to-improve-dt-of-partitioned-tables.patchtext/x-diff; name=0001-Ashutosh-s-patch-to-improve-dt-of-partitioned-tables.patchDownload
From f972a1231e66b84de0b1922e6a6fd96ea661ae20 Mon Sep 17 00:00:00 2001
From: amit <amitlangote09@gmail.com>
Date: Wed, 8 Mar 2017 11:21:55 +0900
Subject: [PATCH 1/2] Ashutosh's patch to improve \dt+ of partitioned tables
---
src/bin/psql/describe.c | 26 +++++++++++++++++---------
src/test/regress/expected/insert.out | 15 +++++++++++++++
src/test/regress/sql/insert.sql | 4 ++++
3 files changed, 36 insertions(+), 9 deletions(-)
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index e2e4cbcc08..f8617cf328 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -2684,7 +2684,7 @@ describeOneTableDetails(const char *schemaname,
/* print child tables (with additional info if partitions) */
if (pset.sversion >= 100000)
printfPQExpBuffer(&buf,
- "SELECT c.oid::pg_catalog.regclass, pg_get_expr(c.relpartbound, c.oid)"
+ "SELECT c.oid::pg_catalog.regclass, pg_get_expr(c.relpartbound, c.oid), c.relkind"
" FROM pg_catalog.pg_class c, pg_catalog.pg_inherits i"
" WHERE c.oid=i.inhrelid AND"
" i.inhparent = '%s' AND"
@@ -2709,13 +2709,12 @@ describeOneTableDetails(const char *schemaname,
if (!verbose)
{
+ const char *ct = tableinfo.relkind != 'P' ? _("child tables") : _("partitions");
+
/* print the number of child tables, if any */
if (tuples > 0)
{
- if (tableinfo.relkind != 'P')
- printfPQExpBuffer(&buf, _("Number of child tables: %d (Use \\d+ to list them.)"), tuples);
- else
- printfPQExpBuffer(&buf, _("Number of partitions: %d (Use \\d+ to list them.)"), tuples);
+ printfPQExpBuffer(&buf, _("Number of %s: %d (Use \\d+ to list them.)"), ct, tuples);
printTableAddFooter(&cont, buf.data);
}
}
@@ -2738,12 +2737,21 @@ describeOneTableDetails(const char *schemaname,
}
else
{
+ char *partitioned_note;
+
+ if (*PQgetvalue(result, i, 2) == 'P')
+ partitioned_note = " has partitions";
+ else
+ partitioned_note = "";
+
if (i == 0)
- printfPQExpBuffer(&buf, "%s: %s %s",
- ct, PQgetvalue(result, i, 0), PQgetvalue(result, i, 1));
+ printfPQExpBuffer(&buf, "%s: %s %s%s",
+ ct, PQgetvalue(result, i, 0), PQgetvalue(result, i, 1),
+ partitioned_note);
else
- printfPQExpBuffer(&buf, "%*s %s %s",
- ctw, "", PQgetvalue(result, i, 0), PQgetvalue(result, i, 1));
+ printfPQExpBuffer(&buf, "%*s %s %s%s",
+ ctw, "", PQgetvalue(result, i, 0), PQgetvalue(result, i, 1),
+ partitioned_note);
}
if (i < tuples - 1)
appendPQExpBufferChar(&buf, ',');
diff --git a/src/test/regress/expected/insert.out b/src/test/regress/expected/insert.out
index 116854e142..c8ff863882 100644
--- a/src/test/regress/expected/insert.out
+++ b/src/test/regress/expected/insert.out
@@ -313,6 +313,21 @@ select tableoid::regclass::text, a, min(b) as min_b, max(b) as max_b from list_p
part_null | | 1 | 1
(9 rows)
+-- test \d+ output on a table which has both partitioned and unpartitioned
+-- partitions
+\d+ list_parted
+ Table "public.list_parted"
+ Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
+--------+---------+-----------+----------+---------+----------+--------------+-------------
+ a | text | | | | extended | |
+ b | integer | | | | plain | |
+Partition key: LIST (lower(a))
+Partitions: part_aa_bb FOR VALUES IN ('aa', 'bb'),
+ part_cc_dd FOR VALUES IN ('cc', 'dd'),
+ part_ee_ff FOR VALUES IN ('ee', 'ff') has partitions,
+ part_gg FOR VALUES IN ('gg') has partitions,
+ part_null FOR VALUES IN (NULL)
+
-- cleanup
drop table range_parted, list_parted;
-- more tests for certain multi-level partitioning scenarios
diff --git a/src/test/regress/sql/insert.sql b/src/test/regress/sql/insert.sql
index c56c3c22f8..3126eae7f3 100644
--- a/src/test/regress/sql/insert.sql
+++ b/src/test/regress/sql/insert.sql
@@ -185,6 +185,10 @@ insert into list_parted select 'gg', s.a from generate_series(1, 9) s(a);
insert into list_parted (b) values (1);
select tableoid::regclass::text, a, min(b) as min_b, max(b) as max_b from list_parted group by 1, 2 order by 1;
+-- test \d+ output on a table which has both partitioned and unpartitioned
+-- partitions
+\d+ list_parted
+
-- cleanup
drop table range_parted, list_parted;
--
2.11.0
0002-Show-number-of-partitions-in-the-d-output-even-if-it.patchtext/x-diff; name=0002-Show-number-of-partitions-in-the-d-output-even-if-it.patchDownload
From 53571763ca6ead18e70fc7f4ffc33ef13140fd56 Mon Sep 17 00:00:00 2001
From: amit <amitlangote09@gmail.com>
Date: Wed, 8 Mar 2017 11:28:07 +0900
Subject: [PATCH 2/2] Show number of partitions in the \d(+) output even if
it's 0
---
src/bin/psql/describe.c | 28 ++++++++++++++++++++++++----
src/test/regress/expected/create_table.out | 2 ++
2 files changed, 26 insertions(+), 4 deletions(-)
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index f8617cf328..cce7c8859d 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -2707,14 +2707,34 @@ describeOneTableDetails(const char *schemaname,
else
tuples = PQntuples(result);
- if (!verbose)
+ /*
+ * If partitioned table, always show the number of partitions,
+ * even if it's zero.
+ */
+ if (tableinfo.relkind == 'P')
{
- const char *ct = tableinfo.relkind != 'P' ? _("child tables") : _("partitions");
+ /*
+ * If there are zero partitions, we should always print that.
+ * Otherwise, print only if verbose is not specified.
+ */
+ if (tuples == 0)
+ {
+ printfPQExpBuffer(&buf, _("Number of partitions: %d"), tuples);
+ printTableAddFooter(&cont, buf.data);
+ }
+ else if (!verbose)
+ {
+ printfPQExpBuffer(&buf, _("Number of partitions: %d (Use \\d+ to list them.)"), tuples);
+ printTableAddFooter(&cont, buf.data);
+ }
+ }
+ if (!verbose)
+ {
/* print the number of child tables, if any */
- if (tuples > 0)
+ if (tableinfo.relkind != 'P' && tuples > 0)
{
- printfPQExpBuffer(&buf, _("Number of %s: %d (Use \\d+ to list them.)"), ct, tuples);
+ printfPQExpBuffer(&buf, _("Number of child tables: %d (Use \\d+ to list them.)"), tuples);
printTableAddFooter(&cont, buf.data);
}
}
diff --git a/src/test/regress/expected/create_table.out b/src/test/regress/expected/create_table.out
index c07a474b3d..19fe113a82 100644
--- a/src/test/regress/expected/create_table.out
+++ b/src/test/regress/expected/create_table.out
@@ -440,6 +440,7 @@ ERROR: cannot inherit from partitioned table "partitioned2"
c | text | | not null |
d | text | | not null |
Partition key: RANGE (a oid_ops, plusone(b), c, d COLLATE "C")
+Number of partitions: 0
\d partitioned2
Table "public.partitioned2"
@@ -447,6 +448,7 @@ Partition key: RANGE (a oid_ops, plusone(b), c, d COLLATE "C")
--------+---------+-----------+----------+---------
a | integer | | |
Partition key: LIST ((a + 1))
+Number of partitions: 0
DROP TABLE partitioned, partitioned2;
--
--
2.11.0
About the other statement you changed, I just realized that we should
perhaps do one more thing. Show the Number of partitions, even if it's 0.
In case of inheritance, the parent table stands on its own when there are
no child tables, but a partitioned table doesn't in the same sense. I
tried to implement that in attached patch 0002. Example below:create table p (a int) partition by list (a);
\d p
<snip>
Partition key: LIST (a)
Number of partitions: 0\d+ p
<snip>
Partition key: LIST (a)
Number of partitions: 0create table p1 partition of p for values in (1);
\d p
<snip>
Partition key: LIST (a)
Number of partitions: 1 (Use \d+ to list them.)\d+ p
<snip>
Partition key: LIST (a)
Partitions: p1 FOR VALUES IN (1)
I liked that. PFA 0002 updated. I changed one of \d output to \d+ to
better test partitioned tables without partitions in verbose and
non-verbose mode. Also, refactored the your code to have less number
of conditions. Please let me know if it looks good.
--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company
Attachments:
0001-Improve-d-output-of-a-partitioned-table.patchapplication/octet-stream; name=0001-Improve-d-output-of-a-partitioned-table.patchDownload
From cf867c7763242935caa2b7fb6868273f77004c68 Mon Sep 17 00:00:00 2001
From: Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>
Date: Wed, 8 Mar 2017 11:34:06 +0530
Subject: [PATCH 1/2] Improve \d+ output of a partitioned table
While displaying partitions in \d+ output of a partitioned table
annotate the partitioned partitions as "has partitions".
Refactored an existing statement to use ? : instead of if condition
according to the surrounding code.
---
src/bin/psql/describe.c | 26 +++++++++++++++++---------
src/test/regress/expected/insert.out | 15 +++++++++++++++
src/test/regress/sql/insert.sql | 4 ++++
3 files changed, 36 insertions(+), 9 deletions(-)
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index e2e4cbc..f8617cf 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -2684,7 +2684,7 @@ describeOneTableDetails(const char *schemaname,
/* print child tables (with additional info if partitions) */
if (pset.sversion >= 100000)
printfPQExpBuffer(&buf,
- "SELECT c.oid::pg_catalog.regclass, pg_get_expr(c.relpartbound, c.oid)"
+ "SELECT c.oid::pg_catalog.regclass, pg_get_expr(c.relpartbound, c.oid), c.relkind"
" FROM pg_catalog.pg_class c, pg_catalog.pg_inherits i"
" WHERE c.oid=i.inhrelid AND"
" i.inhparent = '%s' AND"
@@ -2709,13 +2709,12 @@ describeOneTableDetails(const char *schemaname,
if (!verbose)
{
+ const char *ct = tableinfo.relkind != 'P' ? _("child tables") : _("partitions");
+
/* print the number of child tables, if any */
if (tuples > 0)
{
- if (tableinfo.relkind != 'P')
- printfPQExpBuffer(&buf, _("Number of child tables: %d (Use \\d+ to list them.)"), tuples);
- else
- printfPQExpBuffer(&buf, _("Number of partitions: %d (Use \\d+ to list them.)"), tuples);
+ printfPQExpBuffer(&buf, _("Number of %s: %d (Use \\d+ to list them.)"), ct, tuples);
printTableAddFooter(&cont, buf.data);
}
}
@@ -2738,12 +2737,21 @@ describeOneTableDetails(const char *schemaname,
}
else
{
+ char *partitioned_note;
+
+ if (*PQgetvalue(result, i, 2) == 'P')
+ partitioned_note = " has partitions";
+ else
+ partitioned_note = "";
+
if (i == 0)
- printfPQExpBuffer(&buf, "%s: %s %s",
- ct, PQgetvalue(result, i, 0), PQgetvalue(result, i, 1));
+ printfPQExpBuffer(&buf, "%s: %s %s%s",
+ ct, PQgetvalue(result, i, 0), PQgetvalue(result, i, 1),
+ partitioned_note);
else
- printfPQExpBuffer(&buf, "%*s %s %s",
- ctw, "", PQgetvalue(result, i, 0), PQgetvalue(result, i, 1));
+ printfPQExpBuffer(&buf, "%*s %s %s%s",
+ ctw, "", PQgetvalue(result, i, 0), PQgetvalue(result, i, 1),
+ partitioned_note);
}
if (i < tuples - 1)
appendPQExpBufferChar(&buf, ',');
diff --git a/src/test/regress/expected/insert.out b/src/test/regress/expected/insert.out
index 116854e..c8ff863 100644
--- a/src/test/regress/expected/insert.out
+++ b/src/test/regress/expected/insert.out
@@ -313,6 +313,21 @@ select tableoid::regclass::text, a, min(b) as min_b, max(b) as max_b from list_p
part_null | | 1 | 1
(9 rows)
+-- test \d+ output on a table which has both partitioned and unpartitioned
+-- partitions
+\d+ list_parted
+ Table "public.list_parted"
+ Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
+--------+---------+-----------+----------+---------+----------+--------------+-------------
+ a | text | | | | extended | |
+ b | integer | | | | plain | |
+Partition key: LIST (lower(a))
+Partitions: part_aa_bb FOR VALUES IN ('aa', 'bb'),
+ part_cc_dd FOR VALUES IN ('cc', 'dd'),
+ part_ee_ff FOR VALUES IN ('ee', 'ff') has partitions,
+ part_gg FOR VALUES IN ('gg') has partitions,
+ part_null FOR VALUES IN (NULL)
+
-- cleanup
drop table range_parted, list_parted;
-- more tests for certain multi-level partitioning scenarios
diff --git a/src/test/regress/sql/insert.sql b/src/test/regress/sql/insert.sql
index c56c3c2..3126eae 100644
--- a/src/test/regress/sql/insert.sql
+++ b/src/test/regress/sql/insert.sql
@@ -185,6 +185,10 @@ insert into list_parted select 'gg', s.a from generate_series(1, 9) s(a);
insert into list_parted (b) values (1);
select tableoid::regclass::text, a, min(b) as min_b, max(b) as max_b from list_parted group by 1, 2 order by 1;
+-- test \d+ output on a table which has both partitioned and unpartitioned
+-- partitions
+\d+ list_parted
+
-- cleanup
drop table range_parted, list_parted;
--
1.7.9.5
0002-Number-of-partitions-for-a-partitioned-table.patchapplication/octet-stream; name=0002-Number-of-partitions-for-a-partitioned-table.patchDownload
From c26b513e72811058ddf729854d54f4084670208b Mon Sep 17 00:00:00 2001
From: Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>
Date: Wed, 8 Mar 2017 11:39:51 +0530
Subject: [PATCH 2/2] Number of partitions for a partitioned table
For a partitioned table show the number of partitions even if it's 0.
Patch by Amit Langote, with some code and test changes by Ashutosh Bapat
---
src/bin/psql/describe.c | 19 +++++++++++++++----
src/test/regress/expected/create_table.out | 12 +++++++-----
src/test/regress/sql/create_table.sql | 2 +-
3 files changed, 23 insertions(+), 10 deletions(-)
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index f8617cf..40549c9 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -2707,13 +2707,23 @@ describeOneTableDetails(const char *schemaname,
else
tuples = PQntuples(result);
- if (!verbose)
+ /*
+ * For a partitioned table with no partitions, always print the number
+ * of partitions as zero, even when verbose output is expected.
+ * Otherwise, we will not print "Partitions" section for a partitioned
+ * table without any partitions.
+ */
+ if (tableinfo.relkind == 'P' && tuples == 0)
{
- const char *ct = tableinfo.relkind != 'P' ? _("child tables") : _("partitions");
-
- /* print the number of child tables, if any */
+ printfPQExpBuffer(&buf, _("Number of partitions: %d"), tuples);
+ printTableAddFooter(&cont, buf.data);
+ }
+ else if (!verbose)
+ {
+ /* print the number of child tables, if any. */
if (tuples > 0)
{
+ const char *ct = tableinfo.relkind != 'P' ? _("child tables") : _("partitions");
printfPQExpBuffer(&buf, _("Number of %s: %d (Use \\d+ to list them.)"), ct, tuples);
printTableAddFooter(&cont, buf.data);
}
@@ -2759,6 +2769,7 @@ describeOneTableDetails(const char *schemaname,
printTableAddFooter(&cont, buf.data);
}
}
+
PQclear(result);
/* Table type */
diff --git a/src/test/regress/expected/create_table.out b/src/test/regress/expected/create_table.out
index c07a474..f518cec 100644
--- a/src/test/regress/expected/create_table.out
+++ b/src/test/regress/expected/create_table.out
@@ -440,13 +440,15 @@ ERROR: cannot inherit from partitioned table "partitioned2"
c | text | | not null |
d | text | | not null |
Partition key: RANGE (a oid_ops, plusone(b), c, d COLLATE "C")
+Number of partitions: 0
-\d partitioned2
- Table "public.partitioned2"
- Column | Type | Collation | Nullable | Default
---------+---------+-----------+----------+---------
- a | integer | | |
+\d+ partitioned2
+ Table "public.partitioned2"
+ Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
+--------+---------+-----------+----------+---------+---------+--------------+-------------
+ a | integer | | | | plain | |
Partition key: LIST ((a + 1))
+Number of partitions: 0
DROP TABLE partitioned, partitioned2;
--
diff --git a/src/test/regress/sql/create_table.sql b/src/test/regress/sql/create_table.sql
index 1f0fa8e..15e3274 100644
--- a/src/test/regress/sql/create_table.sql
+++ b/src/test/regress/sql/create_table.sql
@@ -426,7 +426,7 @@ CREATE TABLE fail () INHERITS (partitioned2);
-- Partition key in describe output
\d partitioned
-\d partitioned2
+\d+ partitioned2
DROP TABLE partitioned, partitioned2;
--
1.7.9.5
On 2017/03/08 18:27, Ashutosh Bapat wrote:
About the other statement you changed, I just realized that we should
perhaps do one more thing. Show the Number of partitions, even if it's 0.
In case of inheritance, the parent table stands on its own when there are
no child tables, but a partitioned table doesn't in the same sense. I
tried to implement that in attached patch 0002. Example below:create table p (a int) partition by list (a);
\d p
<snip>
Partition key: LIST (a)
Number of partitions: 0\d+ p
<snip>
Partition key: LIST (a)
Number of partitions: 0create table p1 partition of p for values in (1);
\d p
<snip>
Partition key: LIST (a)
Number of partitions: 1 (Use \d+ to list them.)\d+ p
<snip>
Partition key: LIST (a)
Partitions: p1 FOR VALUES IN (1)I liked that. PFA 0002 updated. I changed one of \d output to \d+ to
better test partitioned tables without partitions in verbose and
non-verbose mode. Also, refactored the your code to have less number
of conditions. Please let me know if it looks good.
Thanks, looks good.
Regards,
Amit
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Added this to 2017/7 commitfest to keep a track of it.
On Wed, Mar 8, 2017 at 3:39 PM, Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:
On 2017/03/08 18:27, Ashutosh Bapat wrote:
About the other statement you changed, I just realized that we should
perhaps do one more thing. Show the Number of partitions, even if it's 0.
In case of inheritance, the parent table stands on its own when there are
no child tables, but a partitioned table doesn't in the same sense. I
tried to implement that in attached patch 0002. Example below:create table p (a int) partition by list (a);
\d p
<snip>
Partition key: LIST (a)
Number of partitions: 0\d+ p
<snip>
Partition key: LIST (a)
Number of partitions: 0create table p1 partition of p for values in (1);
\d p
<snip>
Partition key: LIST (a)
Number of partitions: 1 (Use \d+ to list them.)\d+ p
<snip>
Partition key: LIST (a)
Partitions: p1 FOR VALUES IN (1)I liked that. PFA 0002 updated. I changed one of \d output to \d+ to
better test partitioned tables without partitions in verbose and
non-verbose mode. Also, refactored the your code to have less number
of conditions. Please let me know if it looks good.Thanks, looks good.
Regards,
Amit
--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Hi,
Thomas's application to track patches told me that this patch needs
rebase. It also required some changes to the code. Here's the updated
version. I have squashed those two patches together.
On Tue, Mar 14, 2017 at 6:35 PM, Ashutosh Bapat
<ashutosh.bapat@enterprisedb.com> wrote:
Added this to 2017/7 commitfest to keep a track of it.
On Wed, Mar 8, 2017 at 3:39 PM, Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:On 2017/03/08 18:27, Ashutosh Bapat wrote:
About the other statement you changed, I just realized that we should
perhaps do one more thing. Show the Number of partitions, even if it's 0.
In case of inheritance, the parent table stands on its own when there are
no child tables, but a partitioned table doesn't in the same sense. I
tried to implement that in attached patch 0002. Example below:create table p (a int) partition by list (a);
\d p
<snip>
Partition key: LIST (a)
Number of partitions: 0\d+ p
<snip>
Partition key: LIST (a)
Number of partitions: 0create table p1 partition of p for values in (1);
\d p
<snip>
Partition key: LIST (a)
Number of partitions: 1 (Use \d+ to list them.)\d+ p
<snip>
Partition key: LIST (a)
Partitions: p1 FOR VALUES IN (1)I liked that. PFA 0002 updated. I changed one of \d output to \d+ to
better test partitioned tables without partitions in verbose and
non-verbose mode. Also, refactored the your code to have less number
of conditions. Please let me know if it looks good.Thanks, looks good.
Regards,
Amit--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company
--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company
Attachments:
0001-Improve-d-output-of-a-partitioned-table_v2.patchtext/x-patch; charset=US-ASCII; name=0001-Improve-d-output-of-a-partitioned-table_v2.patchDownload
From 5e5af858931f11b86e8b535331849e58d9a08281 Mon Sep 17 00:00:00 2001
From: Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>
Date: Mon, 4 Sep 2017 09:56:41 +0530
Subject: [PATCH] Improve \d+ output of a partitioned table
While displaying partitions in \d+ output of a partitioned table
annotate the partitioned partitions as "has partitions".
For a partitioned table show the number of partitions even if it's 0.
Refactored an existing statement to use ? : instead of if condition
according to the surrounding code.
Ashutosh Bapat and Amit Langote.
---
src/bin/psql/describe.c | 41 +++++++++++++++++++++-------
src/test/regress/expected/create_table.out | 13 +++++----
src/test/regress/expected/foreign_data.out | 3 ++
src/test/regress/expected/insert.out | 15 ++++++++++
src/test/regress/sql/create_table.sql | 2 +-
src/test/regress/sql/insert.sql | 4 +++
6 files changed, 62 insertions(+), 16 deletions(-)
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index f6049cc..db97de0 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -2828,7 +2828,7 @@ describeOneTableDetails(const char *schemaname,
/* print child tables (with additional info if partitions) */
if (pset.sversion >= 100000)
printfPQExpBuffer(&buf,
- "SELECT c.oid::pg_catalog.regclass, pg_catalog.pg_get_expr(c.relpartbound, c.oid)"
+ "SELECT c.oid::pg_catalog.regclass, pg_get_expr(c.relpartbound, c.oid), c.relkind"
" FROM pg_catalog.pg_class c, pg_catalog.pg_inherits i"
" WHERE c.oid=i.inhrelid AND i.inhparent = '%s'"
" ORDER BY c.oid::pg_catalog.regclass::pg_catalog.text;", oid);
@@ -2851,15 +2851,26 @@ describeOneTableDetails(const char *schemaname,
else
tuples = PQntuples(result);
- if (!verbose)
+ /*
+ * For a partitioned table with no partitions, always print the number
+ * of partitions as zero, even when verbose output is expected.
+ * Otherwise, we will not print "Partitions" section for a partitioned
+ * table without any partitions.
+ */
+ if (tableinfo.relkind == RELKIND_PARTITIONED_TABLE && tuples == 0)
+ {
+ printfPQExpBuffer(&buf, _("Number of partitions: %d"), tuples);
+ printTableAddFooter(&cont, buf.data);
+ }
+ else if (!verbose)
{
+ const char *ct = tableinfo.relkind != RELKIND_PARTITIONED_TABLE ?
+ _("child tables") : _("partitions");
+
/* print the number of child tables, if any */
if (tuples > 0)
{
- if (tableinfo.relkind != RELKIND_PARTITIONED_TABLE)
- printfPQExpBuffer(&buf, _("Number of child tables: %d (Use \\d+ to list them.)"), tuples);
- else
- printfPQExpBuffer(&buf, _("Number of partitions: %d (Use \\d+ to list them.)"), tuples);
+ printfPQExpBuffer(&buf, _("Number of %s: %d (Use \\d+ to list them.)"), ct, tuples);
printTableAddFooter(&cont, buf.data);
}
}
@@ -2883,12 +2894,21 @@ describeOneTableDetails(const char *schemaname,
}
else
{
+ char *partitioned_note;
+
+ if (*PQgetvalue(result, i, 2) == RELKIND_PARTITIONED_TABLE)
+ partitioned_note = " has partitions";
+ else
+ partitioned_note = "";
+
if (i == 0)
- printfPQExpBuffer(&buf, "%s: %s %s",
- ct, PQgetvalue(result, i, 0), PQgetvalue(result, i, 1));
+ printfPQExpBuffer(&buf, "%s: %s %s%s",
+ ct, PQgetvalue(result, i, 0), PQgetvalue(result, i, 1),
+ partitioned_note);
else
- printfPQExpBuffer(&buf, "%*s %s %s",
- ctw, "", PQgetvalue(result, i, 0), PQgetvalue(result, i, 1));
+ printfPQExpBuffer(&buf, "%*s %s %s%s",
+ ctw, "", PQgetvalue(result, i, 0), PQgetvalue(result, i, 1),
+ partitioned_note);
}
if (i < tuples - 1)
appendPQExpBufferChar(&buf, ',');
@@ -2896,6 +2916,7 @@ describeOneTableDetails(const char *schemaname,
printTableAddFooter(&cont, buf.data);
}
}
+
PQclear(result);
/* Table type */
diff --git a/src/test/regress/expected/create_table.out b/src/test/regress/expected/create_table.out
index babda89..a35d19e 100644
--- a/src/test/regress/expected/create_table.out
+++ b/src/test/regress/expected/create_table.out
@@ -428,13 +428,15 @@ ERROR: cannot inherit from partitioned table "partitioned2"
c | text | | |
d | text | | |
Partition key: RANGE (a oid_ops, plusone(b), c, d COLLATE "C")
+Number of partitions: 0
-\d partitioned2
- Table "public.partitioned2"
- Column | Type | Collation | Nullable | Default
---------+---------+-----------+----------+---------
- a | integer | | |
+\d+ partitioned2
+ Table "public.partitioned2"
+ Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
+--------+---------+-----------+----------+---------+---------+--------------+-------------
+ a | integer | | | | plain | |
Partition key: LIST (((a + 1)))
+Number of partitions: 0
DROP TABLE partitioned, partitioned2;
--
@@ -768,5 +770,6 @@ SELECT obj_description('parted_col_comment'::regclass);
a | integer | | | | plain | | Partition key
b | text | | | | extended | |
Partition key: LIST (a)
+Number of partitions: 0
DROP TABLE parted_col_comment;
diff --git a/src/test/regress/expected/foreign_data.out b/src/test/regress/expected/foreign_data.out
index 927d018..34f7aa9 100644
--- a/src/test/regress/expected/foreign_data.out
+++ b/src/test/regress/expected/foreign_data.out
@@ -1882,6 +1882,7 @@ DROP FOREIGN TABLE pt2_1;
c2 | text | | | | extended | |
c3 | date | | | | plain | |
Partition key: LIST (c1)
+Number of partitions: 0
CREATE FOREIGN TABLE pt2_1 (
c1 integer NOT NULL,
@@ -1966,6 +1967,7 @@ ALTER TABLE pt2 ALTER c2 SET NOT NULL;
c2 | text | | not null | | extended | |
c3 | date | | | | plain | |
Partition key: LIST (c1)
+Number of partitions: 0
\d+ pt2_1
Foreign table "public.pt2_1"
@@ -1995,6 +1997,7 @@ ALTER TABLE pt2 ADD CONSTRAINT pt2chk1 CHECK (c1 > 0);
Partition key: LIST (c1)
Check constraints:
"pt2chk1" CHECK (c1 > 0)
+Number of partitions: 0
\d+ pt2_1
Foreign table "public.pt2_1"
diff --git a/src/test/regress/expected/insert.out b/src/test/regress/expected/insert.out
index e159d62..c6aa7f8 100644
--- a/src/test/regress/expected/insert.out
+++ b/src/test/regress/expected/insert.out
@@ -314,6 +314,21 @@ select tableoid::regclass::text, a, min(b) as min_b, max(b) as max_b from list_p
part_null | | 1 | 1
(9 rows)
+-- test \d+ output on a table which has both partitioned and unpartitioned
+-- partitions
+\d+ list_parted
+ Table "public.list_parted"
+ Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
+--------+---------+-----------+----------+---------+----------+--------------+-------------
+ a | text | | | | extended | |
+ b | integer | | | | plain | |
+Partition key: LIST (lower(a))
+Partitions: part_aa_bb FOR VALUES IN ('aa', 'bb'),
+ part_cc_dd FOR VALUES IN ('cc', 'dd'),
+ part_ee_ff FOR VALUES IN ('ee', 'ff') has partitions,
+ part_gg FOR VALUES IN ('gg') has partitions,
+ part_null FOR VALUES IN (NULL)
+
-- cleanup
drop table range_parted, list_parted;
-- more tests for certain multi-level partitioning scenarios
diff --git a/src/test/regress/sql/create_table.sql b/src/test/regress/sql/create_table.sql
index 1c0ce927..325626c 100644
--- a/src/test/regress/sql/create_table.sql
+++ b/src/test/regress/sql/create_table.sql
@@ -421,7 +421,7 @@ CREATE TABLE fail () INHERITS (partitioned2);
-- Partition key in describe output
\d partitioned
-\d partitioned2
+\d+ partitioned2
DROP TABLE partitioned, partitioned2;
diff --git a/src/test/regress/sql/insert.sql b/src/test/regress/sql/insert.sql
index 6f17872..5017519 100644
--- a/src/test/regress/sql/insert.sql
+++ b/src/test/regress/sql/insert.sql
@@ -185,6 +185,10 @@ insert into list_parted select 'gg', s.a from generate_series(1, 9) s(a);
insert into list_parted (b) values (1);
select tableoid::regclass::text, a, min(b) as min_b, max(b) as max_b from list_parted group by 1, 2 order by 1;
+-- test \d+ output on a table which has both partitioned and unpartitioned
+-- partitions
+\d+ list_parted
+
-- cleanup
drop table range_parted, list_parted;
--
1.7.9.5
Hi Ashutosh,
On 2017/09/04 13:51, Ashutosh Bapat wrote:
Hi,
Thomas's application to track patches told me that this patch needs
rebase. It also required some changes to the code. Here's the updated
version. I have squashed those two patches together.
Thanks for the rebased patch.
Would it make sense to post them in a new thread? When this message
popped into my inbox in a thread titled "dropping partitioned tables
without CASCADE", I wondered for a second if there is still something left
to be done about that. :)
Thanks,
Amit
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Mon, Sep 4, 2017 at 10:34 AM, Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:
Hi Ashutosh,
On 2017/09/04 13:51, Ashutosh Bapat wrote:
Hi,
Thomas's application to track patches told me that this patch needs
rebase. It also required some changes to the code. Here's the updated
version. I have squashed those two patches together.Thanks for the rebased patch.
Would it make sense to post them in a new thread? When this message
popped into my inbox in a thread titled "dropping partitioned tables
without CASCADE", I wondered for a second if there is still something left
to be done about that. :)
That would be good. We will have to update the thread in commitfest
entry though.
--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
if (tuples > 0)
{
- if (tableinfo.relkind != RELKIND_PARTITIONED_TABLE)
- printfPQExpBuffer(&buf, _("Number of child tables: %d (Use \\d+ to list them.)"), tuples);
- else
- printfPQExpBuffer(&buf, _("Number of partitions: %d (Use \\d+ to list them.)"), tuples);
+ printfPQExpBuffer(&buf, _("Number of %s: %d (Use \\d+ to list them.)"), ct, tuples);
printTableAddFooter(&cont, buf.data);
}Please don't do this, because it breaks translatability. I think the
original is fine.
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Mon, Sep 4, 2017 at 3:48 PM, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
if (tuples > 0) { - if (tableinfo.relkind != RELKIND_PARTITIONED_TABLE) - printfPQExpBuffer(&buf, _("Number of child tables: %d (Use \\d+ to list them.)"), tuples); - else - printfPQExpBuffer(&buf, _("Number of partitions: %d (Use \\d+ to list them.)"), tuples); + printfPQExpBuffer(&buf, _("Number of %s: %d (Use \\d+ to list them.)"), ct, tuples); printTableAddFooter(&cont, buf.data); }Please don't do this, because it breaks translatability. I think the
original is fine.
We have used this style in the "else" case of if (!verbose). So, I
just copied it. I have removed that change in the attached patch.
--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company
Attachments:
0001-Improve-d-output-of-a-partitioned-table_v3.patchtext/x-patch; charset=US-ASCII; name=0001-Improve-d-output-of-a-partitioned-table_v3.patchDownload
From c0a153f0535c4d1dca637996d4cd5e6f62c11afe Mon Sep 17 00:00:00 2001
From: Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>
Date: Mon, 4 Sep 2017 09:56:41 +0530
Subject: [PATCH] Improve \d+ output of a partitioned table
While displaying partitions in \d+ output of a partitioned table
annotate the partitioned partitions as "has partitions".
For a partitioned table show the number of partitions even if it's 0.
Ashutosh Bapat and Amit Langote.
---
src/bin/psql/describe.c | 32 ++++++++++++++++++++++------
src/test/regress/expected/create_table.out | 13 ++++++-----
src/test/regress/expected/foreign_data.out | 3 +++
src/test/regress/expected/insert.out | 15 +++++++++++++
src/test/regress/sql/create_table.sql | 2 +-
src/test/regress/sql/insert.sql | 4 ++++
6 files changed, 57 insertions(+), 12 deletions(-)
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index f6049cc..5adf5a4 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -2828,7 +2828,7 @@ describeOneTableDetails(const char *schemaname,
/* print child tables (with additional info if partitions) */
if (pset.sversion >= 100000)
printfPQExpBuffer(&buf,
- "SELECT c.oid::pg_catalog.regclass, pg_catalog.pg_get_expr(c.relpartbound, c.oid)"
+ "SELECT c.oid::pg_catalog.regclass, pg_get_expr(c.relpartbound, c.oid), c.relkind"
" FROM pg_catalog.pg_class c, pg_catalog.pg_inherits i"
" WHERE c.oid=i.inhrelid AND i.inhparent = '%s'"
" ORDER BY c.oid::pg_catalog.regclass::pg_catalog.text;", oid);
@@ -2851,7 +2851,18 @@ describeOneTableDetails(const char *schemaname,
else
tuples = PQntuples(result);
- if (!verbose)
+ /*
+ * For a partitioned table with no partitions, always print the number
+ * of partitions as zero, even when verbose output is expected.
+ * Otherwise, we will not print "Partitions" section for a partitioned
+ * table without any partitions.
+ */
+ if (tableinfo.relkind == RELKIND_PARTITIONED_TABLE && tuples == 0)
+ {
+ printfPQExpBuffer(&buf, _("Number of partitions: %d"), tuples);
+ printTableAddFooter(&cont, buf.data);
+ }
+ else if (!verbose)
{
/* print the number of child tables, if any */
if (tuples > 0)
@@ -2883,12 +2894,21 @@ describeOneTableDetails(const char *schemaname,
}
else
{
+ char *partitioned_note;
+
+ if (*PQgetvalue(result, i, 2) == RELKIND_PARTITIONED_TABLE)
+ partitioned_note = " has partitions";
+ else
+ partitioned_note = "";
+
if (i == 0)
- printfPQExpBuffer(&buf, "%s: %s %s",
- ct, PQgetvalue(result, i, 0), PQgetvalue(result, i, 1));
+ printfPQExpBuffer(&buf, "%s: %s %s%s",
+ ct, PQgetvalue(result, i, 0), PQgetvalue(result, i, 1),
+ partitioned_note);
else
- printfPQExpBuffer(&buf, "%*s %s %s",
- ctw, "", PQgetvalue(result, i, 0), PQgetvalue(result, i, 1));
+ printfPQExpBuffer(&buf, "%*s %s %s%s",
+ ctw, "", PQgetvalue(result, i, 0), PQgetvalue(result, i, 1),
+ partitioned_note);
}
if (i < tuples - 1)
appendPQExpBufferChar(&buf, ',');
diff --git a/src/test/regress/expected/create_table.out b/src/test/regress/expected/create_table.out
index babda89..a35d19e 100644
--- a/src/test/regress/expected/create_table.out
+++ b/src/test/regress/expected/create_table.out
@@ -428,13 +428,15 @@ ERROR: cannot inherit from partitioned table "partitioned2"
c | text | | |
d | text | | |
Partition key: RANGE (a oid_ops, plusone(b), c, d COLLATE "C")
+Number of partitions: 0
-\d partitioned2
- Table "public.partitioned2"
- Column | Type | Collation | Nullable | Default
---------+---------+-----------+----------+---------
- a | integer | | |
+\d+ partitioned2
+ Table "public.partitioned2"
+ Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
+--------+---------+-----------+----------+---------+---------+--------------+-------------
+ a | integer | | | | plain | |
Partition key: LIST (((a + 1)))
+Number of partitions: 0
DROP TABLE partitioned, partitioned2;
--
@@ -768,5 +770,6 @@ SELECT obj_description('parted_col_comment'::regclass);
a | integer | | | | plain | | Partition key
b | text | | | | extended | |
Partition key: LIST (a)
+Number of partitions: 0
DROP TABLE parted_col_comment;
diff --git a/src/test/regress/expected/foreign_data.out b/src/test/regress/expected/foreign_data.out
index 927d018..34f7aa9 100644
--- a/src/test/regress/expected/foreign_data.out
+++ b/src/test/regress/expected/foreign_data.out
@@ -1882,6 +1882,7 @@ DROP FOREIGN TABLE pt2_1;
c2 | text | | | | extended | |
c3 | date | | | | plain | |
Partition key: LIST (c1)
+Number of partitions: 0
CREATE FOREIGN TABLE pt2_1 (
c1 integer NOT NULL,
@@ -1966,6 +1967,7 @@ ALTER TABLE pt2 ALTER c2 SET NOT NULL;
c2 | text | | not null | | extended | |
c3 | date | | | | plain | |
Partition key: LIST (c1)
+Number of partitions: 0
\d+ pt2_1
Foreign table "public.pt2_1"
@@ -1995,6 +1997,7 @@ ALTER TABLE pt2 ADD CONSTRAINT pt2chk1 CHECK (c1 > 0);
Partition key: LIST (c1)
Check constraints:
"pt2chk1" CHECK (c1 > 0)
+Number of partitions: 0
\d+ pt2_1
Foreign table "public.pt2_1"
diff --git a/src/test/regress/expected/insert.out b/src/test/regress/expected/insert.out
index e159d62..c6aa7f8 100644
--- a/src/test/regress/expected/insert.out
+++ b/src/test/regress/expected/insert.out
@@ -314,6 +314,21 @@ select tableoid::regclass::text, a, min(b) as min_b, max(b) as max_b from list_p
part_null | | 1 | 1
(9 rows)
+-- test \d+ output on a table which has both partitioned and unpartitioned
+-- partitions
+\d+ list_parted
+ Table "public.list_parted"
+ Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
+--------+---------+-----------+----------+---------+----------+--------------+-------------
+ a | text | | | | extended | |
+ b | integer | | | | plain | |
+Partition key: LIST (lower(a))
+Partitions: part_aa_bb FOR VALUES IN ('aa', 'bb'),
+ part_cc_dd FOR VALUES IN ('cc', 'dd'),
+ part_ee_ff FOR VALUES IN ('ee', 'ff') has partitions,
+ part_gg FOR VALUES IN ('gg') has partitions,
+ part_null FOR VALUES IN (NULL)
+
-- cleanup
drop table range_parted, list_parted;
-- more tests for certain multi-level partitioning scenarios
diff --git a/src/test/regress/sql/create_table.sql b/src/test/regress/sql/create_table.sql
index 1c0ce927..325626c 100644
--- a/src/test/regress/sql/create_table.sql
+++ b/src/test/regress/sql/create_table.sql
@@ -421,7 +421,7 @@ CREATE TABLE fail () INHERITS (partitioned2);
-- Partition key in describe output
\d partitioned
-\d partitioned2
+\d+ partitioned2
DROP TABLE partitioned, partitioned2;
diff --git a/src/test/regress/sql/insert.sql b/src/test/regress/sql/insert.sql
index 6f17872..5017519 100644
--- a/src/test/regress/sql/insert.sql
+++ b/src/test/regress/sql/insert.sql
@@ -185,6 +185,10 @@ insert into list_parted select 'gg', s.a from generate_series(1, 9) s(a);
insert into list_parted (b) values (1);
select tableoid::regclass::text, a, min(b) as min_b, max(b) as max_b from list_parted group by 1, 2 order by 1;
+-- test \d+ output on a table which has both partitioned and unpartitioned
+-- partitions
+\d+ list_parted
+
-- cleanup
drop table range_parted, list_parted;
--
1.7.9.5
I picked this patch for review and started looking at the implementation
details.
Consider the below test:
1)
postgres=# create table foo (a int, b int) partition by list (a);
CREATE TABLE
postgres=# \d foo
Table "public.foo"
Column | Type | Collation | Nullable | Default
--------+---------+-----------+----------+---------
a | integer | | |
b | integer | | |
Partition key: LIST (a)
Number of partitions: 0
postgres=# \d+ foo
Table "public.foo"
Column | Type | Collation | Nullable | Default | Storage | Stats target
| Description
--------+---------+-----------+----------+---------+---------+--------------+-------------
a | integer | | | | plain |
|
b | integer | | | | plain |
|
Partition key: LIST (a)
Number of partitions: 0
In the above case, verbose as well as normal output give information
about number of partitions.
2) Add partition to the foo;
create table foo_p1 partition of foo for values in (1, 2, 3) partition by
list (b);
postgres=# \d foo
Table "public.foo"
Column | Type | Collation | Nullable | Default
--------+---------+-----------+----------+---------
a | integer | | |
b | integer | | |
Partition key: LIST (a)
*Number of partitions: 1 (Use \d+ to list them.)*
postgres=# \d+ foo
Table "public.foo"
Column | Type | Collation | Nullable | Default | Storage | Stats target
| Description
--------+---------+-----------+----------+---------+---------+--------------+-------------
a | integer | | | | plain |
|
b | integer | | | | plain |
|
Partition key: LIST (a)
*Partitions: foo_p1 FOR VALUES IN (1, 2, 3) has partitions*
Above verbose output for foo says, foo_p1 "has partitions". But if I do
postgres=# \d foo_p1
Table "public.foo_p1"
Column | Type | Collation | Nullable | Default
--------+---------+-----------+----------+---------
a | integer | | |
b | integer | | |
Partition of: foo FOR VALUES IN (1, 2, 3)
Partition key: LIST (b)
*Number of partitions: 0*
it tell "Number of partitions: 0".
I feel like information is conflicting with each other. AFAIU, idea about
adding
"has partitions" was to let know that it's a partitioned table. So can you
directly
add the "is partitioned" in place of "has partitions"?
Did those change in the attached patch and update regression expected
output.
Also run pgindent on the patch.
Thanks,
On Mon, Sep 4, 2017 at 6:22 PM, Ashutosh Bapat <
ashutosh.bapat@enterprisedb.com> wrote:
On Mon, Sep 4, 2017 at 3:48 PM, Alvaro Herrera <alvherre@alvh.no-ip.org>
wrote:if (tuples > 0)
{
- if (tableinfo.relkind !=RELKIND_PARTITIONED_TABLE)
- printfPQExpBuffer(&buf,
_("Number of child tables: %d (Use \\d+ to list them.)"), tuples);
- else
- printfPQExpBuffer(&buf,_("Number of partitions: %d (Use \\d+ to list them.)"), tuples);
+ printfPQExpBuffer(&buf, _("Number of %s:
%d (Use \\d+ to list them.)"), ct, tuples);
printTableAddFooter(&cont, buf.data);
}Please don't do this, because it breaks translatability. I think the
original is fine.We have used this style in the "else" case of if (!verbose). So, I
just copied it. I have removed that change in the attached patch.--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
...
Rushabh Lathia
www.EnterpriseDB.com
Attachments:
0001-Improve-d-output-of-a-partitioned-table_v4.patchtext/x-patch; charset=US-ASCII; name=0001-Improve-d-output-of-a-partitioned-table_v4.patchDownload
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index f6049cc..ab9a637 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -2828,7 +2828,7 @@ describeOneTableDetails(const char *schemaname,
/* print child tables (with additional info if partitions) */
if (pset.sversion >= 100000)
printfPQExpBuffer(&buf,
- "SELECT c.oid::pg_catalog.regclass, pg_catalog.pg_get_expr(c.relpartbound, c.oid)"
+ "SELECT c.oid::pg_catalog.regclass, pg_get_expr(c.relpartbound, c.oid), c.relkind"
" FROM pg_catalog.pg_class c, pg_catalog.pg_inherits i"
" WHERE c.oid=i.inhrelid AND i.inhparent = '%s'"
" ORDER BY c.oid::pg_catalog.regclass::pg_catalog.text;", oid);
@@ -2851,7 +2851,18 @@ describeOneTableDetails(const char *schemaname,
else
tuples = PQntuples(result);
- if (!verbose)
+ /*
+ * For a partitioned table with no partitions, always print the number
+ * of partitions as zero, even when verbose output is expected.
+ * Otherwise, we will not print "Partitions" section for a partitioned
+ * table without any partitions.
+ */
+ if (tableinfo.relkind == RELKIND_PARTITIONED_TABLE && tuples == 0)
+ {
+ printfPQExpBuffer(&buf, _("Number of partitions: %d"), tuples);
+ printTableAddFooter(&cont, buf.data);
+ }
+ else if (!verbose)
{
/* print the number of child tables, if any */
if (tuples > 0)
@@ -2883,12 +2894,21 @@ describeOneTableDetails(const char *schemaname,
}
else
{
+ char *partitioned_note;
+
+ if (*(PQgetvalue(result, i, 2)) == RELKIND_PARTITIONED_TABLE)
+ partitioned_note = " is partitioned";
+ else
+ partitioned_note = "";
+
if (i == 0)
- printfPQExpBuffer(&buf, "%s: %s %s",
- ct, PQgetvalue(result, i, 0), PQgetvalue(result, i, 1));
+ printfPQExpBuffer(&buf, "%s: %s %s%s",
+ ct, PQgetvalue(result, i, 0), PQgetvalue(result, i, 1),
+ partitioned_note);
else
- printfPQExpBuffer(&buf, "%*s %s %s",
- ctw, "", PQgetvalue(result, i, 0), PQgetvalue(result, i, 1));
+ printfPQExpBuffer(&buf, "%*s %s %s%s",
+ ctw, "", PQgetvalue(result, i, 0), PQgetvalue(result, i, 1),
+ partitioned_note);
}
if (i < tuples - 1)
appendPQExpBufferChar(&buf, ',');
diff --git a/src/test/regress/expected/create_table.out b/src/test/regress/expected/create_table.out
index babda89..a35d19e 100644
--- a/src/test/regress/expected/create_table.out
+++ b/src/test/regress/expected/create_table.out
@@ -428,13 +428,15 @@ ERROR: cannot inherit from partitioned table "partitioned2"
c | text | | |
d | text | | |
Partition key: RANGE (a oid_ops, plusone(b), c, d COLLATE "C")
+Number of partitions: 0
-\d partitioned2
- Table "public.partitioned2"
- Column | Type | Collation | Nullable | Default
---------+---------+-----------+----------+---------
- a | integer | | |
+\d+ partitioned2
+ Table "public.partitioned2"
+ Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
+--------+---------+-----------+----------+---------+---------+--------------+-------------
+ a | integer | | | | plain | |
Partition key: LIST (((a + 1)))
+Number of partitions: 0
DROP TABLE partitioned, partitioned2;
--
@@ -768,5 +770,6 @@ SELECT obj_description('parted_col_comment'::regclass);
a | integer | | | | plain | | Partition key
b | text | | | | extended | |
Partition key: LIST (a)
+Number of partitions: 0
DROP TABLE parted_col_comment;
diff --git a/src/test/regress/expected/foreign_data.out b/src/test/regress/expected/foreign_data.out
index 927d018..34f7aa9 100644
--- a/src/test/regress/expected/foreign_data.out
+++ b/src/test/regress/expected/foreign_data.out
@@ -1882,6 +1882,7 @@ DROP FOREIGN TABLE pt2_1;
c2 | text | | | | extended | |
c3 | date | | | | plain | |
Partition key: LIST (c1)
+Number of partitions: 0
CREATE FOREIGN TABLE pt2_1 (
c1 integer NOT NULL,
@@ -1966,6 +1967,7 @@ ALTER TABLE pt2 ALTER c2 SET NOT NULL;
c2 | text | | not null | | extended | |
c3 | date | | | | plain | |
Partition key: LIST (c1)
+Number of partitions: 0
\d+ pt2_1
Foreign table "public.pt2_1"
@@ -1995,6 +1997,7 @@ ALTER TABLE pt2 ADD CONSTRAINT pt2chk1 CHECK (c1 > 0);
Partition key: LIST (c1)
Check constraints:
"pt2chk1" CHECK (c1 > 0)
+Number of partitions: 0
\d+ pt2_1
Foreign table "public.pt2_1"
diff --git a/src/test/regress/expected/insert.out b/src/test/regress/expected/insert.out
index e159d62..fbe88ef 100644
--- a/src/test/regress/expected/insert.out
+++ b/src/test/regress/expected/insert.out
@@ -314,6 +314,21 @@ select tableoid::regclass::text, a, min(b) as min_b, max(b) as max_b from list_p
part_null | | 1 | 1
(9 rows)
+-- test \d+ output on a table which has both partitioned and unpartitioned
+-- partitions
+\d+ list_parted
+ Table "public.list_parted"
+ Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
+--------+---------+-----------+----------+---------+----------+--------------+-------------
+ a | text | | | | extended | |
+ b | integer | | | | plain | |
+Partition key: LIST (lower(a))
+Partitions: part_aa_bb FOR VALUES IN ('aa', 'bb'),
+ part_cc_dd FOR VALUES IN ('cc', 'dd'),
+ part_ee_ff FOR VALUES IN ('ee', 'ff') is partitioned,
+ part_gg FOR VALUES IN ('gg') is partitioned,
+ part_null FOR VALUES IN (NULL)
+
-- cleanup
drop table range_parted, list_parted;
-- more tests for certain multi-level partitioning scenarios
diff --git a/src/test/regress/sql/create_table.sql b/src/test/regress/sql/create_table.sql
index 1c0ce927..325626c 100644
--- a/src/test/regress/sql/create_table.sql
+++ b/src/test/regress/sql/create_table.sql
@@ -421,7 +421,7 @@ CREATE TABLE fail () INHERITS (partitioned2);
-- Partition key in describe output
\d partitioned
-\d partitioned2
+\d+ partitioned2
DROP TABLE partitioned, partitioned2;
diff --git a/src/test/regress/sql/insert.sql b/src/test/regress/sql/insert.sql
index 6f17872..5017519 100644
--- a/src/test/regress/sql/insert.sql
+++ b/src/test/regress/sql/insert.sql
@@ -185,6 +185,10 @@ insert into list_parted select 'gg', s.a from generate_series(1, 9) s(a);
insert into list_parted (b) values (1);
select tableoid::regclass::text, a, min(b) as min_b, max(b) as max_b from list_parted group by 1, 2 order by 1;
+-- test \d+ output on a table which has both partitioned and unpartitioned
+-- partitions
+\d+ list_parted
+
-- cleanup
drop table range_parted, list_parted;
On Wed, Sep 6, 2017 at 1:06 PM, Rushabh Lathia <rushabh.lathia@gmail.com> wrote:
2) Add partition to the foo;
create table foo_p1 partition of foo for values in (1, 2, 3) partition by
list (b);postgres=# \d foo
Table "public.foo"
Column | Type | Collation | Nullable | Default
--------+---------+-----------+----------+---------
a | integer | | |
b | integer | | |
Partition key: LIST (a)
Number of partitions: 1 (Use \d+ to list them.)postgres=# \d+ foo
Table "public.foo"
Column | Type | Collation | Nullable | Default | Storage | Stats target
| Description
--------+---------+-----------+----------+---------+---------+--------------+-------------
a | integer | | | | plain |
|
b | integer | | | | plain |
|
Partition key: LIST (a)
Partitions: foo_p1 FOR VALUES IN (1, 2, 3) has partitionsAbove verbose output for foo says, foo_p1 "has partitions". But if I do
postgres=# \d foo_p1
Table "public.foo_p1"
Column | Type | Collation | Nullable | Default
--------+---------+-----------+----------+---------
a | integer | | |
b | integer | | |
Partition of: foo FOR VALUES IN (1, 2, 3)
Partition key: LIST (b)
Number of partitions: 0it tell "Number of partitions: 0".
I feel like information is conflicting with each other. AFAIU, idea about
adding
"has partitions" was to let know that it's a partitioned table. So can you
directly
add the "is partitioned" in place of "has partitions"?Did those change in the attached patch and update regression expected
output.
Looks better.
Also run pgindent on the patch.
Thanks for the changes. The patch looks good to me.
--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Okay, I have marked this as ready for committer.
Thanks,
On Wed, Sep 6, 2017 at 2:50 PM, Ashutosh Bapat <
ashutosh.bapat@enterprisedb.com> wrote:
On Wed, Sep 6, 2017 at 1:06 PM, Rushabh Lathia <rushabh.lathia@gmail.com>
wrote:2) Add partition to the foo;
create table foo_p1 partition of foo for values in (1, 2, 3) partition by
list (b);postgres=# \d foo
Table "public.foo"
Column | Type | Collation | Nullable | Default
--------+---------+-----------+----------+---------
a | integer | | |
b | integer | | |
Partition key: LIST (a)
Number of partitions: 1 (Use \d+ to list them.)postgres=# \d+ foo
Table "public.foo"
Column | Type | Collation | Nullable | Default | Storage | Statstarget
| Description
--------+---------+-----------+----------+---------+---------+--------------+-------------
a | integer | | | | plain |
|
b | integer | | | | plain |
|
Partition key: LIST (a)
Partitions: foo_p1 FOR VALUES IN (1, 2, 3) has partitionsAbove verbose output for foo says, foo_p1 "has partitions". But if I do
postgres=# \d foo_p1
Table "public.foo_p1"
Column | Type | Collation | Nullable | Default
--------+---------+-----------+----------+---------
a | integer | | |
b | integer | | |
Partition of: foo FOR VALUES IN (1, 2, 3)
Partition key: LIST (b)
Number of partitions: 0it tell "Number of partitions: 0".
I feel like information is conflicting with each other. AFAIU, idea about
adding
"has partitions" was to let know that it's a partitioned table. So canyou
directly
add the "is partitioned" in place of "has partitions"?Did those change in the attached patch and update regression expected
output.Looks better.
Also run pgindent on the patch.
Thanks for the changes. The patch looks good to me.
--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company
--
Rushabh Lathia
On 2017/09/06 18:46, Rushabh Lathia wrote:
Okay, I have marked this as ready for committer.
Thanks Ashutosh and Rushabh for rebasing and improving the patch. Looks
good to me too.
Regards,
Amit
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 2017/09/06 19:14, Amit Langote wrote:
On 2017/09/06 18:46, Rushabh Lathia wrote:
Okay, I have marked this as ready for committer.
Thanks Ashutosh and Rushabh for rebasing and improving the patch. Looks
good to me too.
Patch needed to be rebased after the default partitions patch went in, so
done. Per build status on http://commitfest.cputube.org :)
Thanks,
Amit
Attachments:
0001-Improve-d-output-of-a-partitioned-table_v5.patchtext/plain; charset=UTF-8; name=0001-Improve-d-output-of-a-partitioned-table_v5.patchDownload
From 0ac21ff604b5dccf818f9d69c945ff845d1771bf Mon Sep 17 00:00:00 2001
From: amit <amitlangote09@gmail.com>
Date: Wed, 13 Sep 2017 09:56:34 +0900
Subject: [PATCH] Some enhancments for \d+ output of partitioned tables
---
src/bin/psql/describe.c | 32 ++++++++++++++++++++++++------
src/test/regress/expected/create_table.out | 13 +++++++-----
src/test/regress/expected/foreign_data.out | 3 +++
src/test/regress/expected/insert.out | 17 ++++++++++++++++
src/test/regress/sql/create_table.sql | 2 +-
src/test/regress/sql/insert.sql | 4 ++++
6 files changed, 59 insertions(+), 12 deletions(-)
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index d22ec68431..855e6870e9 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -2831,7 +2831,7 @@ describeOneTableDetails(const char *schemaname,
/* print child tables (with additional info if partitions) */
if (pset.sversion >= 100000)
printfPQExpBuffer(&buf,
- "SELECT c.oid::pg_catalog.regclass, pg_catalog.pg_get_expr(c.relpartbound, c.oid)"
+ "SELECT c.oid::pg_catalog.regclass, pg_get_expr(c.relpartbound, c.oid), c.relkind"
" FROM pg_catalog.pg_class c, pg_catalog.pg_inherits i"
" WHERE c.oid=i.inhrelid AND i.inhparent = '%s'"
" ORDER BY c.oid::pg_catalog.regclass::pg_catalog.text;", oid);
@@ -2854,7 +2854,18 @@ describeOneTableDetails(const char *schemaname,
else
tuples = PQntuples(result);
- if (!verbose)
+ /*
+ * For a partitioned table with no partitions, always print the number
+ * of partitions as zero, even when verbose output is expected.
+ * Otherwise, we will not print "Partitions" section for a partitioned
+ * table without any partitions.
+ */
+ if (tableinfo.relkind == RELKIND_PARTITIONED_TABLE && tuples == 0)
+ {
+ printfPQExpBuffer(&buf, _("Number of partitions: %d"), tuples);
+ printTableAddFooter(&cont, buf.data);
+ }
+ else if (!verbose)
{
/* print the number of child tables, if any */
if (tuples > 0)
@@ -2886,12 +2897,21 @@ describeOneTableDetails(const char *schemaname,
}
else
{
+ char *partitioned_note;
+
+ if (*(PQgetvalue(result, i, 2)) == RELKIND_PARTITIONED_TABLE)
+ partitioned_note = " is partitioned";
+ else
+ partitioned_note = "";
+
if (i == 0)
- printfPQExpBuffer(&buf, "%s: %s %s",
- ct, PQgetvalue(result, i, 0), PQgetvalue(result, i, 1));
+ printfPQExpBuffer(&buf, "%s: %s %s%s",
+ ct, PQgetvalue(result, i, 0), PQgetvalue(result, i, 1),
+ partitioned_note);
else
- printfPQExpBuffer(&buf, "%*s %s %s",
- ctw, "", PQgetvalue(result, i, 0), PQgetvalue(result, i, 1));
+ printfPQExpBuffer(&buf, "%*s %s %s%s",
+ ctw, "", PQgetvalue(result, i, 0), PQgetvalue(result, i, 1),
+ partitioned_note);
}
if (i < tuples - 1)
appendPQExpBufferChar(&buf, ',');
diff --git a/src/test/regress/expected/create_table.out b/src/test/regress/expected/create_table.out
index 58c755be50..0a4316f269 100644
--- a/src/test/regress/expected/create_table.out
+++ b/src/test/regress/expected/create_table.out
@@ -428,13 +428,15 @@ ERROR: cannot inherit from partitioned table "partitioned2"
c | text | | |
d | text | | |
Partition key: RANGE (a oid_ops, plusone(b), c, d COLLATE "C")
+Number of partitions: 0
-\d partitioned2
- Table "public.partitioned2"
- Column | Type | Collation | Nullable | Default
---------+---------+-----------+----------+---------
- a | integer | | |
+\d+ partitioned2
+ Table "public.partitioned2"
+ Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
+--------+---------+-----------+----------+---------+---------+--------------+-------------
+ a | integer | | | | plain | |
Partition key: LIST (((a + 1)))
+Number of partitions: 0
DROP TABLE partitioned, partitioned2;
--
@@ -788,5 +790,6 @@ SELECT obj_description('parted_col_comment'::regclass);
a | integer | | | | plain | | Partition key
b | text | | | | extended | |
Partition key: LIST (a)
+Number of partitions: 0
DROP TABLE parted_col_comment;
diff --git a/src/test/regress/expected/foreign_data.out b/src/test/regress/expected/foreign_data.out
index c6e558b07f..e8e29aa954 100644
--- a/src/test/regress/expected/foreign_data.out
+++ b/src/test/regress/expected/foreign_data.out
@@ -1882,6 +1882,7 @@ DROP FOREIGN TABLE pt2_1;
c2 | text | | | | extended | |
c3 | date | | | | plain | |
Partition key: LIST (c1)
+Number of partitions: 0
CREATE FOREIGN TABLE pt2_1 (
c1 integer NOT NULL,
@@ -1966,6 +1967,7 @@ ALTER TABLE pt2 ALTER c2 SET NOT NULL;
c2 | text | | not null | | extended | |
c3 | date | | | | plain | |
Partition key: LIST (c1)
+Number of partitions: 0
\d+ pt2_1
Foreign table "public.pt2_1"
@@ -1995,6 +1997,7 @@ ALTER TABLE pt2 ADD CONSTRAINT pt2chk1 CHECK (c1 > 0);
Partition key: LIST (c1)
Check constraints:
"pt2chk1" CHECK (c1 > 0)
+Number of partitions: 0
\d+ pt2_1
Foreign table "public.pt2_1"
diff --git a/src/test/regress/expected/insert.out b/src/test/regress/expected/insert.out
index 73a5600f19..046bc27d1d 100644
--- a/src/test/regress/expected/insert.out
+++ b/src/test/regress/expected/insert.out
@@ -382,6 +382,23 @@ select tableoid::regclass::text, a, min(b) as min_b, max(b) as max_b from list_p
part_null | | 1 | 1
(9 rows)
+-- test \d+ output on a table which has both partitioned and unpartitioned
+-- partitions
+\d+ list_parted
+ Table "public.list_parted"
+ Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
+--------+---------+-----------+----------+---------+----------+--------------+-------------
+ a | text | | | | extended | |
+ b | integer | | | | plain | |
+Partition key: LIST (lower(a))
+Partitions: part_aa_bb FOR VALUES IN ('aa', 'bb'),
+ part_cc_dd FOR VALUES IN ('cc', 'dd'),
+ part_default DEFAULT is partitioned,
+ part_ee_ff FOR VALUES IN ('ee', 'ff') is partitioned,
+ part_gg FOR VALUES IN ('gg') is partitioned,
+ part_null FOR VALUES IN (NULL),
+ part_xx_yy FOR VALUES IN ('xx', 'yy') is partitioned
+
-- cleanup
drop table range_parted, list_parted;
-- test that a default partition added as the first partition accepts any value
diff --git a/src/test/regress/sql/create_table.sql b/src/test/regress/sql/create_table.sql
index eeab5d91ff..a9195b72c6 100644
--- a/src/test/regress/sql/create_table.sql
+++ b/src/test/regress/sql/create_table.sql
@@ -421,7 +421,7 @@ CREATE TABLE fail () INHERITS (partitioned2);
-- Partition key in describe output
\d partitioned
-\d partitioned2
+\d+ partitioned2
DROP TABLE partitioned, partitioned2;
diff --git a/src/test/regress/sql/insert.sql b/src/test/regress/sql/insert.sql
index a2948e4dd0..2fdd751a71 100644
--- a/src/test/regress/sql/insert.sql
+++ b/src/test/regress/sql/insert.sql
@@ -222,6 +222,10 @@ insert into list_parted select 'gg', s.a from generate_series(1, 9) s(a);
insert into list_parted (b) values (1);
select tableoid::regclass::text, a, min(b) as min_b, max(b) as max_b from list_parted group by 1, 2 order by 1;
+-- test \d+ output on a table which has both partitioned and unpartitioned
+-- partitions
+\d+ list_parted
+
-- cleanup
drop table range_parted, list_parted;
--
2.11.0
Thanks Amit for taking care of this.
On Wed, Sep 13, 2017 at 6:31 AM, Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:
On 2017/09/06 19:14, Amit Langote wrote:
On 2017/09/06 18:46, Rushabh Lathia wrote:
Okay, I have marked this as ready for committer.
Thanks Ashutosh and Rushabh for rebasing and improving the patch. Looks
good to me too.Patch needed to be rebased after the default partitions patch went in, so
done. Per build status on http://commitfest.cputube.org :)Thanks,
Amit
--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Amit Langote wrote:
On 2017/09/06 19:14, Amit Langote wrote:
On 2017/09/06 18:46, Rushabh Lathia wrote:
Okay, I have marked this as ready for committer.
Thanks Ashutosh and Rushabh for rebasing and improving the patch. Looks
good to me too.Patch needed to be rebased after the default partitions patch went in, so
done. Per build status on http://commitfest.cputube.org :)
I think adding "is partitioned" at end of line isn't good; looks like a
phrase but isn't translatable. Maybe add keyword PARTITIONED instead?
Having the DEFAULT partition show up in the middle of the list is weird.
Is it possible to put it at either start or end of the list?
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Fri, Nov 3, 2017 at 1:42 PM, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
Amit Langote wrote:
On 2017/09/06 19:14, Amit Langote wrote:
On 2017/09/06 18:46, Rushabh Lathia wrote:
Okay, I have marked this as ready for committer.
Thanks Ashutosh and Rushabh for rebasing and improving the patch. Looks
good to me too.Patch needed to be rebased after the default partitions patch went in, so
done. Per build status on http://commitfest.cputube.org :)I think adding "is partitioned" at end of line isn't good; looks like a
phrase but isn't translatable. Maybe add keyword PARTITIONED instead?
In that case may be we should separate bounds and "PARTITIONED" with a
",". "part_default DEFAULT, PARTITIONED" would read better than
"part_default DEFAULT PARTITIONED"?
Having the DEFAULT partition show up in the middle of the list is weird.
Agreed. But that's true even without this patch.
Is it possible to put it at either start or end of the list?
Right now, we could do that if we order the list by bound expression;
lexically DEFAULT would come before FOR VALUES ... . But that's not
future-safe; we may have a bound expression starting with A, B or C.
Beyond that it really gets tricky to order the partitions by bounds.
The goal of this patch is to mark the partitioned partitions as such
and show the number of partitions. While your suggestion is a valid
request, it's kind of beyond the scope of this patch. Someone might
want to extend this request and say that partitions should be listed
in the order of their bounds (I do feel that we should do some effort
in that direction). But I am not sure whether it should be done in
this patch.
--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Ashutosh Bapat wrote:
On Fri, Nov 3, 2017 at 1:42 PM, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
I think adding "is partitioned" at end of line isn't good; looks like a
phrase but isn't translatable. Maybe add keyword PARTITIONED instead?In that case may be we should separate bounds and "PARTITIONED" with a
",". "part_default DEFAULT, PARTITIONED" would read better than
"part_default DEFAULT PARTITIONED"?
Hmm, I vote +0.5 for the comma.
Having the DEFAULT partition show up in the middle of the list is weird.
Agreed. But that's true even without this patch.
Yes.
Is it possible to put it at either start or end of the list?
Right now, we could do that if we order the list by bound expression;
lexically DEFAULT would come before FOR VALUES ... . But that's not
future-safe; we may have a bound expression starting with A, B or C.
Beyond that it really gets tricky to order the partitions by bounds.
I was just thinking in changing the query to be "order by
is_the_default_partition, partition_name" instead of just "order by
partition_name". Sorting by bounds rather than name (a feature whose
worth should definitely be discussed separately IMV) sounds a lot more
complicated.
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 2017/11/03 21:39, Alvaro Herrera wrote:
Ashutosh Bapat wrote:
On Fri, Nov 3, 2017 at 1:42 PM, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
I think adding "is partitioned" at end of line isn't good; looks like a
phrase but isn't translatable. Maybe add keyword PARTITIONED instead?In that case may be we should separate bounds and "PARTITIONED" with a
",". "part_default DEFAULT, PARTITIONED" would read better than
"part_default DEFAULT PARTITIONED"?Hmm, I vote +0.5 for the comma.
Me too.
Is it possible to put it at either start or end of the list?
Right now, we could do that if we order the list by bound expression;
lexically DEFAULT would come before FOR VALUES ... . But that's not
future-safe; we may have a bound expression starting with A, B or C.
Beyond that it really gets tricky to order the partitions by bounds.I was just thinking in changing the query to be "order by
is_the_default_partition, partition_name" instead of just "order by
partition_name". Sorting by bounds rather than name (a feature whose
worth should definitely be discussed separately IMV) sounds a lot more
complicated.
Yeah, it sounds like a desirable feature, but as you both say, should be
discussed separately. Since the facility to order partitions in the bound
order is internal to the server yet, we'd need some new server-side
functionality to expose the same with sane SQL-callable interface, which
clearly needs its own separate discussion.
Thanks,
Amit
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Somehow the earlier patches missed qualifying pg_get_expr() by
pg_catalog. Fixed it along with annotating the partitioned partition
as ", PARTITIONED".
On Fri, Nov 3, 2017 at 6:09 PM, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
Right now, we could do that if we order the list by bound expression;
lexically DEFAULT would come before FOR VALUES ... . But that's not
future-safe; we may have a bound expression starting with A, B or C.
Beyond that it really gets tricky to order the partitions by bounds.I was just thinking in changing the query to be "order by
is_the_default_partition, partition_name" instead of just "order by
partition_name". Sorting by bounds rather than name (a feature whose
worth should definitely be discussed separately IMV) sounds a lot more
complicated.
Right now we don't have a catalog column or a SQL function which can
tell whether a given partition is default partition based on the
partition bounds or otherwise. That's what it seemed when you
suggested ordering by "is_the_default_partition". Instead I have
ordered the partitions by pg_catalog.pg_get_expr(...) = 'DEFAULT'. We
can introduce a SQL function which takes child and parent oids and
return true if it's default partition and use that here, but that
seems more than what you are suggesting here. I have added that as a
separate patch.
If we tackle the problem of listing partitions by their bounds
somehow, DEFAULT partition listing would be tackled anyway. So, may be
we should leave it outside the scope of this patch.
--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company
Attachments:
0001-Improve-d-output-of-a-partitioned-table.patchtext/x-patch; charset=US-ASCII; name=0001-Improve-d-output-of-a-partitioned-table.patchDownload
From 398003b2d5f6b54e6cdd8542f653786987ef3bfe Mon Sep 17 00:00:00 2001
From: Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>
Date: Mon, 4 Sep 2017 09:56:41 +0530
Subject: [PATCH 1/2] Improve \d+ output of a partitioned table
While displaying partitions in \d+ output of a partitioned table
annotate the partitioned partitions as "PARTITIONED".
For a partitioned table show the number of partitions even if it's 0.
Ashutosh Bapat and Amit Langote.
---
src/bin/psql/describe.c | 34 +++++++++++++++++++++++-----
src/test/regress/expected/create_table.out | 13 +++++++----
src/test/regress/expected/foreign_data.out | 3 +++
src/test/regress/expected/insert.out | 17 ++++++++++++++
src/test/regress/sql/create_table.sql | 2 +-
src/test/regress/sql/insert.sql | 4 ++++
6 files changed, 61 insertions(+), 12 deletions(-)
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index b7b978a..44c5089 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -2870,7 +2870,9 @@ describeOneTableDetails(const char *schemaname,
/* print child tables (with additional info if partitions) */
if (pset.sversion >= 100000)
printfPQExpBuffer(&buf,
- "SELECT c.oid::pg_catalog.regclass, pg_catalog.pg_get_expr(c.relpartbound, c.oid)"
+ "SELECT c.oid::pg_catalog.regclass,"
+ " pg_catalog.pg_get_expr(c.relpartbound, c.oid),"
+ " c.relkind"
" FROM pg_catalog.pg_class c, pg_catalog.pg_inherits i"
" WHERE c.oid=i.inhrelid AND i.inhparent = '%s'"
" ORDER BY c.oid::pg_catalog.regclass::pg_catalog.text;", oid);
@@ -2893,7 +2895,18 @@ describeOneTableDetails(const char *schemaname,
else
tuples = PQntuples(result);
- if (!verbose)
+ /*
+ * For a partitioned table with no partitions, always print the number
+ * of partitions as zero, even when verbose output is expected.
+ * Otherwise, we will not print "Partitions" section for a partitioned
+ * table without any partitions.
+ */
+ if (tableinfo.relkind == RELKIND_PARTITIONED_TABLE && tuples == 0)
+ {
+ printfPQExpBuffer(&buf, _("Number of partitions: %d"), tuples);
+ printTableAddFooter(&cont, buf.data);
+ }
+ else if (!verbose)
{
/* print the number of child tables, if any */
if (tuples > 0)
@@ -2925,12 +2938,21 @@ describeOneTableDetails(const char *schemaname,
}
else
{
+ char *partitioned_note;
+
+ if (*PQgetvalue(result, i, 2) == RELKIND_PARTITIONED_TABLE)
+ partitioned_note = ", PARTITIONED";
+ else
+ partitioned_note = "";
+
if (i == 0)
- printfPQExpBuffer(&buf, "%s: %s %s",
- ct, PQgetvalue(result, i, 0), PQgetvalue(result, i, 1));
+ printfPQExpBuffer(&buf, "%s: %s %s%s",
+ ct, PQgetvalue(result, i, 0), PQgetvalue(result, i, 1),
+ partitioned_note);
else
- printfPQExpBuffer(&buf, "%*s %s %s",
- ctw, "", PQgetvalue(result, i, 0), PQgetvalue(result, i, 1));
+ printfPQExpBuffer(&buf, "%*s %s %s%s",
+ ctw, "", PQgetvalue(result, i, 0), PQgetvalue(result, i, 1),
+ partitioned_note);
}
if (i < tuples - 1)
appendPQExpBufferChar(&buf, ',');
diff --git a/src/test/regress/expected/create_table.out b/src/test/regress/expected/create_table.out
index 60ab28a..ac6f576 100644
--- a/src/test/regress/expected/create_table.out
+++ b/src/test/regress/expected/create_table.out
@@ -428,13 +428,15 @@ ERROR: cannot inherit from partitioned table "partitioned2"
c | text | | |
d | text | | |
Partition key: RANGE (a oid_ops, plusone(b), c, d COLLATE "C")
+Number of partitions: 0
-\d partitioned2
- Table "public.partitioned2"
- Column | Type | Collation | Nullable | Default
---------+---------+-----------+----------+---------
- a | integer | | |
+\d+ partitioned2
+ Table "public.partitioned2"
+ Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
+--------+---------+-----------+----------+---------+---------+--------------+-------------
+ a | integer | | | | plain | |
Partition key: LIST (((a + 1)))
+Number of partitions: 0
DROP TABLE partitioned, partitioned2;
--
@@ -788,5 +790,6 @@ SELECT obj_description('parted_col_comment'::regclass);
a | integer | | | | plain | | Partition key
b | text | | | | extended | |
Partition key: LIST (a)
+Number of partitions: 0
DROP TABLE parted_col_comment;
diff --git a/src/test/regress/expected/foreign_data.out b/src/test/regress/expected/foreign_data.out
index 331f7a9..d2c184f 100644
--- a/src/test/regress/expected/foreign_data.out
+++ b/src/test/regress/expected/foreign_data.out
@@ -1898,6 +1898,7 @@ DROP FOREIGN TABLE pt2_1;
c2 | text | | | | extended | |
c3 | date | | | | plain | |
Partition key: LIST (c1)
+Number of partitions: 0
CREATE FOREIGN TABLE pt2_1 (
c1 integer NOT NULL,
@@ -1982,6 +1983,7 @@ ALTER TABLE pt2 ALTER c2 SET NOT NULL;
c2 | text | | not null | | extended | |
c3 | date | | | | plain | |
Partition key: LIST (c1)
+Number of partitions: 0
\d+ pt2_1
Foreign table "public.pt2_1"
@@ -2011,6 +2013,7 @@ ALTER TABLE pt2 ADD CONSTRAINT pt2chk1 CHECK (c1 > 0);
Partition key: LIST (c1)
Check constraints:
"pt2chk1" CHECK (c1 > 0)
+Number of partitions: 0
\d+ pt2_1
Foreign table "public.pt2_1"
diff --git a/src/test/regress/expected/insert.out b/src/test/regress/expected/insert.out
index b715619..b1d9c7a 100644
--- a/src/test/regress/expected/insert.out
+++ b/src/test/regress/expected/insert.out
@@ -382,6 +382,23 @@ select tableoid::regclass::text, a, min(b) as min_b, max(b) as max_b from list_p
part_null | | 1 | 1
(9 rows)
+-- test \d+ output on a table which has both partitioned and unpartitioned
+-- partitions
+\d+ list_parted
+ Table "public.list_parted"
+ Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
+--------+---------+-----------+----------+---------+----------+--------------+-------------
+ a | text | | | | extended | |
+ b | integer | | | | plain | |
+Partition key: LIST (lower(a))
+Partitions: part_aa_bb FOR VALUES IN ('aa', 'bb'),
+ part_cc_dd FOR VALUES IN ('cc', 'dd'),
+ part_default DEFAULT, PARTITIONED,
+ part_ee_ff FOR VALUES IN ('ee', 'ff'), PARTITIONED,
+ part_gg FOR VALUES IN ('gg'), PARTITIONED,
+ part_null FOR VALUES IN (NULL),
+ part_xx_yy FOR VALUES IN ('xx', 'yy'), PARTITIONED
+
-- cleanup
drop table range_parted, list_parted;
-- test that a default partition added as the first partition accepts any value
diff --git a/src/test/regress/sql/create_table.sql b/src/test/regress/sql/create_table.sql
index df6a6d7..f526b88 100644
--- a/src/test/regress/sql/create_table.sql
+++ b/src/test/regress/sql/create_table.sql
@@ -421,7 +421,7 @@ CREATE TABLE fail () INHERITS (partitioned2);
-- Partition key in describe output
\d partitioned
-\d partitioned2
+\d+ partitioned2
DROP TABLE partitioned, partitioned2;
diff --git a/src/test/regress/sql/insert.sql b/src/test/regress/sql/insert.sql
index d741514..0e4399d 100644
--- a/src/test/regress/sql/insert.sql
+++ b/src/test/regress/sql/insert.sql
@@ -222,6 +222,10 @@ insert into list_parted select 'gg', s.a from generate_series(1, 9) s(a);
insert into list_parted (b) values (1);
select tableoid::regclass::text, a, min(b) as min_b, max(b) as max_b from list_parted group by 1, 2 order by 1;
+-- test \d+ output on a table which has both partitioned and unpartitioned
+-- partitions
+\d+ list_parted
+
-- cleanup
drop table range_parted, list_parted;
--
1.7.9.5
0002-Separate-default-partition-from-rest-of-the-partitio.patchtext/x-patch; charset=US-ASCII; name=0002-Separate-default-partition-from-rest-of-the-partitio.patchDownload
From 405607115bf578e46cce65389c492aebd0f055fb Mon Sep 17 00:00:00 2001
From: Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>
Date: Mon, 6 Nov 2017 11:49:00 +0530
Subject: [PATCH 2/2] Separate default partition from rest of the partitions.
---
src/bin/psql/describe.c | 3 ++-
src/test/regress/expected/insert.out | 4 ++--
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index 44c5089..9916710 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -2875,7 +2875,8 @@ describeOneTableDetails(const char *schemaname,
" c.relkind"
" FROM pg_catalog.pg_class c, pg_catalog.pg_inherits i"
" WHERE c.oid=i.inhrelid AND i.inhparent = '%s'"
- " ORDER BY c.oid::pg_catalog.regclass::pg_catalog.text;", oid);
+ " ORDER BY pg_catalog.pg_get_expr(c.relpartbound, c.oid) = 'DEFAULT',"
+ " c.oid::pg_catalog.regclass::pg_catalog.text;", oid);
else if (pset.sversion >= 80300)
printfPQExpBuffer(&buf,
"SELECT c.oid::pg_catalog.regclass"
diff --git a/src/test/regress/expected/insert.out b/src/test/regress/expected/insert.out
index b1d9c7a..6b4abf8 100644
--- a/src/test/regress/expected/insert.out
+++ b/src/test/regress/expected/insert.out
@@ -393,11 +393,11 @@ select tableoid::regclass::text, a, min(b) as min_b, max(b) as max_b from list_p
Partition key: LIST (lower(a))
Partitions: part_aa_bb FOR VALUES IN ('aa', 'bb'),
part_cc_dd FOR VALUES IN ('cc', 'dd'),
- part_default DEFAULT, PARTITIONED,
part_ee_ff FOR VALUES IN ('ee', 'ff'), PARTITIONED,
part_gg FOR VALUES IN ('gg'), PARTITIONED,
part_null FOR VALUES IN (NULL),
- part_xx_yy FOR VALUES IN ('xx', 'yy'), PARTITIONED
+ part_xx_yy FOR VALUES IN ('xx', 'yy'), PARTITIONED,
+ part_default DEFAULT, PARTITIONED
-- cleanup
drop table range_parted, list_parted;
--
1.7.9.5