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.ba...@enterprisedb.com> wrote:
> On Mon, Sep 4, 2017 at 3:48 PM, Alvaro Herrera <alvhe...@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
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;
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
