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 <si...@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
>From f972a1231e66b84de0b1922e6a6fd96ea661ae20 Mon Sep 17 00:00:00 2001
From: amit <amitlangot...@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

>From 53571763ca6ead18e70fc7f4ffc33ef13140fd56 Mon Sep 17 00:00:00 2001
From: amit <amitlangot...@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

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to