čt 7. 2. 2019 v 9:51 odesílatel Amit Langote <langote_amit...@lab.ntt.co.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:
>

Show nested objects in rectangular output is a problem. I prefer a design
where any times the sum of displayed sizes is same like total size.

So if I have partitions on level1 of size 16KB, and on level 2 8KB, then I
would to display 16 and 8, and not 24 and 8. If I remember, this rule is
modified, when filter is used.

Maybe we can introduce more columns where totals from leaves can be
calculated.

Regards

Pavel


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

Reply via email to