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