Hi Pavel,

Thanks for updating the patch.

On 2019/02/08 17:26, Pavel Stehule wrote:
> I renamed originally calculated column "size" to "direct partitions size"
> .. see Alvaro's comment. Then I introduced new column "total partitions
> size" that is calculated like you propose.
> 
> Now the result of dPn+ looks like
> 
>                                      List of partitioned relations
> ┌────────┬────────┬───────┬─────────────┬────────────────────────┬───────────────────────┬─────────────┐
> │ Schema │  Name  │ Owner │ Parent name │ Direct partitions size │ Total
> partitions size │ Description │
> ╞════════╪════════╪═══════╪═════════════╪════════════════════════╪═══════════════════════╪═════════════╡
> │ public │ p      │ pavel │             │ 8192 bytes             │ 24
> kB                 │             │
> │ public │ p_1    │ pavel │ p           │ 8192 bytes             │ 16
> kB                 │             │
> │ public │ p_1_bc │ pavel │ p_1         │ 8192 bytes             │ 8192
> bytes            │             │
> └────────┴────────┴───────┴─────────────┴────────────────────────┴───────────────────────┴─────────────┘
> (3 rows)

OK, so for each listed partitioned table (root and nested), this shows the
total size of the directly attached leaf partitions *and* the total size
of all partitions in its (sub-) tree.

By the way, what I think Alvaro meant by "local size" is not what the
"direct partition size" above shows.  I think "local size" means the size
of the storage assigned to the table itself, not to partitions attached to
it, which are distinct relations.  We don't implement that concept in
Postgres today, but may in the future.  I'm not sure if we'll add a
another column to show "local size" in the future when we do implement
that concept or if Alvaro meant that there should only be "local size"
(not "direct partition size") which will always show 0 for now and "total
partition size" columns.


Anyway, I have a few more suggestions to improve the patch, but instead of
sending the minute-level changes in the email, I've gone ahead and made
those changes myself.  I've attached a delta patch that applies on top of
your v9 patch.  Summary of the changes I made is as follows:

* Documentation rewording here and there (also mentioned the "direct
partitions size" and "total partitions size" division in the \dPn output
per the latest patch)

* Wrapped some lines in code so that they don't look too wide

* Renamed show_nested_partitions to show_nested

* Changed "Partitioned relations" in the output headers to say
"Partitioned tables" where appropriate

* Fixed quiet mode output to use correct word between object_name vs
objects_name

Please merge these changes if you think they are reasonable.

Thanks,
Amit
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index 76a6ceb0dd..9fb632b0bd 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -1664,19 +1664,23 @@ testdb=>
         <term><literal>\dP[n+] [ <link 
linkend="app-psql-patterns"><replaceable 
class="parameter">pattern</replaceable></link> ]</literal></term>
         <listitem>
         <para>
-        Lists partitioned relations. If <replaceable
-        class="parameter">pattern</replaceable> is specified, only
-        entries whose relation name or schema name matches
-        the pattern are listed. If the form <literal>\dP+</literal>
-        is used, the sum of sizes of related partitions (including the
-        table and indexes, if any) and associated description
-        are also displayed.
+        Lists partitioned relations.  If <replaceable
+        class="parameter">pattern</replaceable> is specified, only entries
+        whose name matches the pattern are listed.  By default, only
+        partitioned tables are listed; supply a pattern to also include
+        partitioned indexes.  If the form <literal>\dP+</literal>
+        is used, the sum of sizes of table's partitions (including their
+        indexes) and associated description are also displayed.
         </para>
 
         <para>
-         If modifier <literal>n</literal> is used, then non-root partition
-         tables are displayed too. The size is calculated just for directly
-         assigned partitions (not for nested partitions).
+         If modifier <literal>n</literal> (which stands for
+         <quote>nested</quote>) is used, then non-root partitioned tables are
+         displayed too.  The displayed size is divided into two columns in
+         this case: one that shows the total size of only the directly
+         attached leaf partitions and another that shows total size of all
+         partitions, also considering other sub-partitioned partitions, for
+         each partitioned tables that's displayed.
         </para>
         </listitem>
       </varlistentry>
@@ -1687,17 +1691,15 @@ testdb=&gt;
         <listitem>
         <para>
         Lists partitioned indexes. If <replaceable
-        class="parameter">pattern</replaceable> is specified, only
-        entries whose index name or schema name matches
-        the pattern are listed. If the form <literal>\dPi+</literal>
-        is used, the sum of sizes of related indexes and associated
-        description are also displayed.
+        class="parameter">pattern</replaceable> is specified, only entries
+        whose name matches the pattern are listed. If the form
+        <literal>\dPi+</literal> is used, the sum of sizes of index's
+        partitions and associated description are also displayed.
         </para>
 
         <para>
-         If modifier <literal>n</literal> is used, then non-root partition
-         tables are displayed too. The size is calculated just for directly
-         assigned partitions (not for nested partitions).
+         If the modifier <literal>n</literal> is used, non-root partitioned
+         indexes are displayed too.
         </para>
         </listitem>
       </varlistentry>
@@ -1708,17 +1710,15 @@ testdb=&gt;
         <listitem>
         <para>
         Lists partitioned tables. If <replaceable
-        class="parameter">pattern</replaceable> is specified, only
-        entries whose table name or schema name matches
-        the pattern are listed. If the form <literal>\dPt+</literal>
-        is used, the sum of sizes of related tables and associated
-        description are also displayed.
+        class="parameter">pattern</replaceable> is specified, only entries
+        whose name matches the pattern are listed. If the form
+        <literal>\dPt+</literal> is used, the sum of sizes of table's
+        partitions and associated description are also displayed.
         </para>
 
         <para>
-         If modifier <literal>n</literal> is used, then non-root partition
-         tables are displayed too. The size is calculated just for directly
-         assigned partitions (not for nested partitions).
+         If the modifier <literal>n</literal> is used, non-root partitioned
+         tables are displayed too.
         </para>
         </listitem>
       </varlistentry>
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 42ffbbe630..e3d4752857 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -784,27 +784,33 @@ exec_command_d(PsqlScanState scan_state, bool 
active_branch, const char *cmd)
                                break;
                        case 'P':
                                {
-                                       bool show_nested_partitions = 
strchr(cmd, 'n') ? true : false;
+                                       bool show_nested = strchr(cmd, 'n') ? 
true : false;
 
                                        switch (cmd[2])
                                        {
                                                case 'i':
                                                        /* show indexes only */
-                                                       success = 
listPartitions(pattern, show_verbose, true, false, show_nested_partitions);
+                                                       success = 
listPartitions(pattern, show_verbose,
+                                                                               
                         true, false,
+                                                                               
                         show_nested);
                                                        break;
                                                case 't':
                                                        /* show tables only */
-                                                       success = 
listPartitions(pattern, show_verbose, false, true, show_nested_partitions);
+                                                       success = 
listPartitions(pattern, show_verbose,
+                                                                               
                         false, true,
+                                                                               
                         show_nested);
                                                        break;
                                                case '+':
                                                case '\0':
                                                case 'n':
                                                        /*
-                                                        * show relations - 
when there are not pattern, then it shows
-                                                        * tables with total 
relation size, else where it shows tables
-                                                        * and indexes.
+                                                        * Show only tables if 
there is no pattern.  Also
+                                                        * show indexes if 
pattern is specified.  Show
+                                                        * total size if 
verbose output is specified.
                                                         */
-                                                       success = 
listPartitions(pattern, show_verbose, false, false, show_nested_partitions);
+                                                       success = 
listPartitions(pattern, show_verbose,
+                                                                               
                         false, false,
+                                                                               
                         show_nested);
                                                        break;
                                                default:
                                                        status = 
PSQL_CMD_UNKNOWN;
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index e69fd0d4c0..2b8628f2ff 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -3666,7 +3666,7 @@ listTables(const char *tabtypes, const char *pattern, 
bool verbose, bool showSys
  */
 bool
 listPartitions(const char *pattern, bool verbose, bool show_indexes,
-                          bool show_tables, bool show_nested_partitions)
+                          bool show_tables, bool show_nested)
 {
        PQExpBufferData buf;
        PGresult   *res;
@@ -3714,16 +3714,16 @@ listPartitions(const char *pattern, bool verbose, bool 
show_indexes,
                {
                        size_function = "pg_total_relation_size";
                        relkind_str = CppAsString2(RELKIND_PARTITIONED_TABLE);
-                       object_name = gettext_noop("relation");
-                       objects_name = gettext_noop("relations");
+                       object_name = gettext_noop("table");
+                       objects_name = gettext_noop("tables");
                }
                else
                {
                        size_function = "pg_table_size";
                        relkind_str = CppAsString2(RELKIND_PARTITIONED_TABLE)
                                                        ", " 
CppAsString2(RELKIND_PARTITIONED_INDEX);
-                       object_name = gettext_noop("relation or index");
-                       objects_name = gettext_noop("relations or indexes");
+                       object_name = gettext_noop("table or index");
+                       objects_name = gettext_noop("tables or indexes");
                        mixed_output = true;
                }
        }
@@ -3752,7 +3752,7 @@ listPartitions(const char *pattern, bool verbose, bool 
show_indexes,
                translate_columns[4] = true;
        }
 
-       if (show_nested_partitions)
+       if (show_nested)
                appendPQExpBuffer(&buf,
                                                  ",\n  c3.relname as \"%s\"",
                                                  gettext_noop("Parent name"));
@@ -3764,7 +3764,7 @@ listPartitions(const char *pattern, bool verbose, bool 
show_indexes,
 
        if (verbose)
        {
-               if (show_nested_partitions)
+               if (show_nested)
                {
                        appendPQExpBuffer(&buf,
                                                          ",\n  s.dps as 
\"%s\"",
@@ -3774,7 +3774,7 @@ listPartitions(const char *pattern, bool verbose, bool 
show_indexes,
                                                          gettext_noop("Total 
partitions size"));
                }
                else
-                       /* Both previous sizes are same in this case. Show only 
one */
+                       /* Sizes of all partitions are considered in this case. 
*/
                        appendPQExpBuffer(&buf,
                                                          ",\n  s.tps as 
\"%s\"",
                                                          
gettext_noop("Partitions size"));
@@ -3793,7 +3793,7 @@ listPartitions(const char *pattern, bool verbose, bool 
show_indexes,
                                                         "\n     LEFT JOIN 
pg_catalog.pg_index i ON i.indexrelid = c.oid"
                                                         "\n     LEFT JOIN 
pg_catalog.pg_class c2 ON i.indrelid = c2.oid");
 
-       if (show_nested_partitions)
+       if (show_nested)
                appendPQExpBufferStr(&buf,
                                                         "\n     LEFT JOIN 
pg_catalog.pg_inherits inh ON c.oid = inh.inhrelid"
                                                         "\n     LEFT JOIN 
pg_catalog.pg_class c3 ON c3.oid = inh.inhparent");
@@ -3835,7 +3835,7 @@ listPartitions(const char *pattern, bool verbose, bool 
show_indexes,
        }
 
        appendPQExpBuffer(&buf, "\nWHERE c.relkind IN (%s)", relkind_str);
-       appendPQExpBufferStr(&buf, !show_nested_partitions ? " AND NOT 
c.relispartition\n" : "\n");
+       appendPQExpBufferStr(&buf, !show_nested ? " AND NOT c.relispartition\n" 
: "\n");
 
        if (!pattern)
                appendPQExpBufferStr(&buf, "      AND n.nspname <> 
'pg_catalog'\n"
@@ -3869,14 +3869,14 @@ listPartitions(const char *pattern, bool verbose, bool 
show_indexes,
        if (PQntuples(res) == 0 && !pset.quiet)
        {
                if (pattern)
-                       /* translator: objects_name is "indexes", "tables" or 
"relations" */
+                       /* translator: objects_name is "index", "table" */
                        psql_error("Did not find any partitioned %s named 
\"%s\".\n",
-                                          objects_name,
+                                          object_name,
                                           pattern);
                else
-                       /* translator: object_name is "index", "table" or 
"relation" */
+                       /* translator: object_name is "indexes", "tables" */
                        psql_error("Did not find any partitioned %s.\n",
-                                         object_name);
+                                         objects_name);
        }
        else
        {
@@ -3884,7 +3884,7 @@ listPartitions(const char *pattern, bool verbose, bool 
show_indexes,
 
                initPQExpBuffer(&title);
 
-               /* translator: objects_name is "indexes", "tables" or 
"relations" */
+               /* translator: objects_name is "indexes", "tables" */
                appendPQExpBuffer(&title, _("List of partitioned %s"), 
objects_name);
 
                myopt.nullPrint = NULL;
diff --git a/src/bin/psql/describe.h b/src/bin/psql/describe.h
index 4fc1cacbee..e3c6dda021 100644
--- a/src/bin/psql/describe.h
+++ b/src/bin/psql/describe.h
@@ -64,7 +64,7 @@ extern bool listAllDbs(const char *pattern, bool verbose);
 extern bool listTables(const char *tabtypes, const char *pattern, bool 
verbose, bool showSystem);
 
 /* \dP[n], \dPi[n], \dPt[n] */
-extern bool listPartitions(const char *pattern, bool verbose, bool 
show_indexes, bool show_tables, bool show_nested_partitions);
+extern bool listPartitions(const char *pattern, bool verbose, bool 
show_indexes, bool show_tables, bool show_nested);
 
 /* \dD */
 extern bool listDomains(const char *pattern, bool verbose, bool showSystem);
diff --git a/src/test/regress/expected/psql.out 
b/src/test/regress/expected/psql.out
index 52bcb3f918..2167b14fe5 100644
--- a/src/test/regress/expected/psql.out
+++ b/src/test/regress/expected/psql.out
@@ -4555,7 +4555,7 @@ create index testpart_apple_index on 
testpart_apple(logdate);
 create index testpart_orange_index on testpart_orange(logdate);
 -- only partition related object should be displayed
 \dP test*apple*
-                           List of partitioned relations or indexes
+                            List of partitioned tables or indexes
   Schema  |         Name         |         Owner         |       Type        | 
   On table    
 
----------+----------------------+-----------------------+-------------------+----------------
  testpart | testpart_apple       | testrole_partitioning | partitioned table | 
@@ -4612,7 +4612,7 @@ insert into parent_tab values (generate_series(30,39));
 (1 row)
 
 \dP testpart.*
-                     List of partitioned relations or indexes
+                      List of partitioned tables or indexes
   Schema  |     Name     |         Owner         |       Type        |  On 
table  
 
----------+--------------+-----------------------+-------------------+------------
  testpart | parent_index | testrole_partitioning | partitioned index | 
parent_tab
@@ -4620,7 +4620,7 @@ insert into parent_tab values (generate_series(30,39));
 (2 rows)
 
 \dP
-         List of partitioned relations
+          List of partitioned tables
   Schema  |    Name    |         Owner         
 ----------+------------+-----------------------
  testpart | parent_tab | testrole_partitioning
@@ -4643,7 +4643,7 @@ insert into parent_tab values (generate_series(30,39));
 (2 rows)
 
 \dPn
-                List of partitioned relations
+                  List of partitioned tables
   Schema  |    Name     |         Owner         | Parent name 
 ----------+-------------+-----------------------+-------------
  testpart | child_30_40 | testrole_partitioning | parent_tab
@@ -4651,7 +4651,7 @@ insert into parent_tab values (generate_series(30,39));
 (2 rows)
 
 \dPn testpart.*
-                                List of partitioned relations or indexes
+                                 List of partitioned tables or indexes
   Schema  |        Name        |         Owner         |       Type        | 
Parent name  |  On table   
 
----------+--------------------+-----------------------+-------------------+--------------+-------------
  testpart | child_30_40        | testrole_partitioning | partitioned table | 
parent_tab   | 

Reply via email to