From: Rushabh Lathia <rushabh.lathia@gmail.com>
To: Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>
Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>
Cc: Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
Cc: Simon Riggs <simon@2ndquadrant.com>
Cc: Pg Hackers <pgsql-hackers@postgresql.org>
Subject: Re: dropping partitioned tables without CASCADE
Date: Wed, 6 Sep 2017 13:06:06 +0530
Message-ID: <CAGPqQf1iuASO2P3DnWMF510Nw8vbkubfM8tq5sAteSwohZbkHg@mail.gmail.com> (raw)
In-Reply-To: <CAFjFpReLrHoPH1G6AwPsnMXkpyFve_iFyD+BDt-o0yKymfF+Bw@mail.gmail.com>
References: <CAFjFpRe=wXrqYre1bRRd8gQf-XQnJEsf-OXyfV4aE0TGHFRDGQ@mail.gmail.com>
<20170904101827.zvw3gu4yeduhjhmx@alvherre.pgsql>
<CAFjFpReLrHoPH1G6AwPsnMXkpyFve_iFyD+BDt-o0yKymfF+Bw@mail.gmail.com>
List-Unsubscribe: <mailto:majordomo@postgresql.org?body=unsub%20pgsql-hackers>
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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackersAttachments:
[text/x-patch] 0001-Improve-d-output-of-a-partitioned-table_v4.patch (7.1K, ../CAGPqQf1iuASO2P3DnWMF510Nw8vbkubfM8tq5sAteSwohZbkHg@mail.gmail.com/3-0001-Improve-d-output-of-a-partitioned-table_v4.patch)
download | inline diff:diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.cindex 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.outindex 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.outindex 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.outindex 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.sqlindex 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.sqlindex 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;
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: rushabh.lathia@gmail.com, ashutosh.bapat@enterprisedb.com, alvherre@alvh.no-ip.org, Langote_Amit_f8@lab.ntt.co.jp, simon@2ndquadrant.com
Subject: Re: dropping partitioned tables without CASCADE
In-Reply-To: <CAGPqQf1iuASO2P3DnWMF510Nw8vbkubfM8tq5sAteSwohZbkHg@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox