Re: ToDo: show size of partitioned table

Hi Pavel,

Thanks for sending the updated patch.

On 2018/12/19 15:38, Pavel Stehule wrote:
> út 18. 12. 2018 v 8:49 odesílatel Amit Langote <
>> On 2018/12/17 17:48, Pavel Stehule wrote:
>>> I can imagine new additional flag - line "n" nested - and then we can
>>> display nested partitioned tables with parent table info. Some like
>>>
>>> \dPt - show only root partition tables
>>> \dPnt or \dPtn - show root and nested partitioned tables
>>
>> Too much complication maybe?
>>
>
> I wrote it - the setup query is more complex, but not too much. I fixed the
> size calculation, when nested partitions tables are visible - it calculate
> partitions only from level1 group. Then the displayed size is same as total
> size

\dPn seems to work fine, but I don't quite understand why \dPn+ should
show the sizes only for nested partitions of level.  Consider the
following example outputs:

create table p (a int, b char) partition by list (a);
create table p_1 partition of p for values in (1) partition by list (b);
create table p_1_a partition of p_1 for values in ('a');
create table p_1_bc partition of p_1 for values in ('b', 'c') partition by
list (b);
create table p_1_b partition of p_1_bc for values in ('b');
create table p_1_c partition of p_1_bc for values in ('c');
create table p_2 partition of p for values in (2);
insert into p values (1, 'a');
insert into p values (1, 'b');
insert into p values (2);

\dP+
        List of partitioned relations
 Schema │ Name │ Owner │ Size  │ Description
────────┼──────┼───────┼───────┼─────────────
 public │ p    │ amit  │ 24 kB │
(1 row)

-- size of 'p' shown as 8KB, whereas it's actually 24KB as per above
-- size of 'p_1' shown as 8KB, whereas it's actually 16KB
\dPn+
                  List of partitioned relations
 Schema │  Name  │ Owner │ Parent name │    Size    │ Description
────────┼────────┼───────┼─────────────┼────────────┼─────────────
 public │ p      │ amit  │             │ 8192 bytes │
 public │ p_1    │ amit  │ p           │ 8192 bytes │
 public │ p_1_bc │ amit  │ p_1         │ 8192 bytes │
(3 rows)

Also, if the root partitioned table doesn't have a directly attached leaf
partition, it's size is shown as 0 bytes, whereas I think it should
consider the sizes of its other nested partitions.

drop table p_2;

\dPn+
                  List of partitioned relations
 Schema │  Name  │ Owner │ Parent name │    Size    │ Description
────────┼────────┼───────┼─────────────┼────────────┼─────────────
 public │ p      │ amit  │             │ 0 bytes    │
 public │ p_1    │ amit  │ p           │ 8192 bytes │
 public │ p_1_bc │ amit  │ p_1         │ 8192 bytes │
(3 rows)

If I remove the following two statements from the patched code:

+            if (show_nested_partitions)
+                appendPQExpBuffer(&buf, "\n         WHERE d.level = 1");

+            /*
+             * Assign size just for directly assigned tables, when nested
+             * partitions are visible
+             */
+            if (show_nested_partitions)
+                appendPQExpBuffer(&buf, "\n     WHERE ppt.isleaf AND
ppt.level = 1");

I get the following output, which I find more intuitive:

create table p_2 partition of p for values in (2);
insert into p values (2);

\dP+
        List of partitioned relations
 Schema │ Name │ Owner │ Size  │ Description
────────┼──────┼───────┼───────┼─────────────
 public │ p    │ amit  │ 24 kB │
(1 row)


\dPn+
                  List of partitioned relations
 Schema │  Name  │ Owner │ Parent name │    Size    │ Description
────────┼────────┼───────┼─────────────┼────────────┼─────────────
 public │ p      │ amit  │             │ 24 kB      │
 public │ p_1    │ amit  │ p           │ 16 kB      │
 public │ p_1_bc │ amit  │ p_1         │ 8192 bytes │
(3 rows)

drop table p_2;

\dPn+
                  List of partitioned relations
 Schema │  Name  │ Owner │ Parent name │    Size    │ Description
────────┼────────┼───────┼─────────────┼────────────┼─────────────
 public │ p      │ amit  │             │ 16 kB      │
 public │ p_1    │ amit  │ p           │ 16 kB      │
 public │ p_1_bc │ amit  │ p_1         │ 8192 bytes │
(3 rows)

Thoughts?

Meanwhile, some comments on the patch:

+         If modifier <literal>n</literal> is used, then nested partition
+         tables are displayed too.

Maybe, say "non-root partitioned tables" instead of "nested partition
tables".  Same comment also applies for the same text in the paragraphs
for \dPni and \dPnt too.

+/*
+ * listPartitions()
+ *
+ * handler for \dP, \dPt and \dPi

Maybe mention the 'n' modifier here?

+ */
+bool
+listPartitions(const char *pattern, bool verbose, bool show_indexes, bool
show_tables, bool show_nested_partitions)
+{

I'm not sure if psql's source code formatting guidelines are different
from the backend code, but putting all the arguments on the same line
causes the line grow well over 78 characters.  Can you wrap maybe?

+        if (pattern)
+            /* translator: object_name is "index", "table" or "relation" */
+            psql_error("Did not find any partitioned %s.\n",
+                      object_name);
+        else
+            /* translator: objects_name is "indexes", "tables" or
"relations" */
+            psql_error("Did not find any partitioned %s named \"%s\".\n",
+                       objects_name,
+                       pattern);

You're using the variable 'pattern' in the block where it's supposed to be
NULL.  Maybe it should be:

+        if (pattern)
+            /* translator: object_name is "index", "table" or "relation" */
+            psql_error("Did not find any partitioned %s named \"%s\".\n",
+                      object_name, pattern);
+        else
+            /* translator: objects_name is "indexes", "tables" or
"relations" */
+            psql_error("Did not find any partitioned %s.\n",
+                       objects_name);

Thanks,
Amit