Re: ToDo: show size of partitioned table

čt 7. 2. 2019 v 9:51 odesílatel Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp>
napsal:

> 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?
>
>
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)

>
> 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.
>

fixed

> +/*
> + * listPartitions()
> + *
> + * handler for \dP, \dPt and \dPi
>
> Maybe mention the 'n' modifier here?
>

fixed

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

fixed

> 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);
>

fixed

attached updated patch

Regards

Pavel

> Thanks,
> Amit
>
>