Thread
Commits
-
Tie loose ends in psql's new \dP command
- 421a2c48328c 12.0 landed
-
psql: Describe partitioned tables/indexes as such
- d56e0fde828b 12.0 landed
-
ToDo: show size of partitioned table
Pavel Stehule <pavel.stehule@gmail.com> — 2018-06-01T04:56:22Z
Hi postgres=# SELECT count(*) from data; ┌─────────┐ │ count │ ╞═════════╡ │ 1000000 │ └─────────┘ (1 row) \dt+ can display actual size of partitioned table data - now zero is displayed postgres=# \dt+ data List of relations ┌────────┬──────┬───────┬───────┬─────────┬─────────────┐ │ Schema │ Name │ Type │ Owner │ Size │ Description │ ╞════════╪══════╪═══════╪═══════╪═════════╪═════════════╡ │ public │ data │ table │ pavel │ 0 bytes │ │ └────────┴──────┴───────┴───────┴─────────┴─────────────┘ (1 row) postgres=# \dt+ data* List of relations ┌────────┬────────────┬───────┬───────┬─────────┬─────────────┐ │ Schema │ Name │ Type │ Owner │ Size │ Description │ ╞════════╪════════════╪═══════╪═══════╪═════════╪═════════════╡ │ public │ data │ table │ pavel │ 0 bytes │ │ │ public │ data_2016 │ table │ pavel │ 17 MB │ │ │ public │ data_2017 │ table │ pavel │ 17 MB │ │ │ public │ data_other │ table │ pavel │ 8224 kB │ │ └────────┴────────────┴───────┴───────┴─────────┴─────────────┘ (4 rows) or we can introduce some option for display only partitioned tables without partitions. Regards Pavel -
Re: ToDo: show size of partitioned table
Ashutosh Bapat <ashutosh.bapat@enterprisedb.com> — 2018-06-01T15:15:33Z
On Fri, Jun 1, 2018 at 12:56 AM, Pavel Stehule <pavel.stehule@gmail.com> wrote: > Hi > > postgres=# SELECT count(*) from data; > ┌─────────┐ > │ count │ > ╞═════════╡ > │ 1000000 │ > └─────────┘ > (1 row) > > \dt+ can display actual size of partitioned table data - now zero is > displayed > > postgres=# \dt+ data > List of relations > ┌────────┬──────┬───────┬───────┬─────────┬─────────────┐ > │ Schema │ Name │ Type │ Owner │ Size │ Description │ > ╞════════╪══════╪═══════╪═══════╪═════════╪═════════════╡ > │ public │ data │ table │ pavel │ 0 bytes │ │ > └────────┴──────┴───────┴───────┴─────────┴─────────────┘ > (1 row) I think we should at least display "Type" as "partitioned table" for a partitioned table, so that it's easy to understand why the size is 0; partitioned tables do not hold any data by themselves. -- Best Wishes, Ashutosh Bapat EnterpriseDB Corporation The Postgres Database Company
-
Re: ToDo: show size of partitioned table
Pavel Stehule <pavel.stehule@gmail.com> — 2018-06-01T15:21:52Z
2018-06-01 17:15 GMT+02:00 Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>: > On Fri, Jun 1, 2018 at 12:56 AM, Pavel Stehule <pavel.stehule@gmail.com> > wrote: > > Hi > > > > postgres=# SELECT count(*) from data; > > ┌─────────┐ > > │ count │ > > ╞═════════╡ > > │ 1000000 │ > > └─────────┘ > > (1 row) > > > > \dt+ can display actual size of partitioned table data - now zero is > > displayed > > > > postgres=# \dt+ data > > List of relations > > ┌────────┬──────┬───────┬───────┬─────────┬─────────────┐ > > │ Schema │ Name │ Type │ Owner │ Size │ Description │ > > ╞════════╪══════╪═══════╪═══════╪═════════╪═════════════╡ > > │ public │ data │ table │ pavel │ 0 bytes │ │ > > └────────┴──────┴───────┴───────┴─────────┴─────────────┘ > > (1 row) > > I think we should at least display "Type" as "partitioned table" for a > partitioned table, so that it's easy to understand why the size is 0; > partitioned tables do not hold any data by themselves. > should be. Some is missing still - there is not any total size across all partitions. maybe new command like \dtP+ .. show partitioned tables and their size Regards Pavel > -- > Best Wishes, > Ashutosh Bapat > EnterpriseDB Corporation > The Postgres Database Company >
-
Re: ToDo: show size of partitioned table
Jeevan Ladhe <jeevan.ladhe@enterprisedb.com> — 2018-06-19T17:41:10Z
On Fri, Jun 1, 2018 at 8:51 PM, Pavel Stehule <pavel.stehule@gmail.com> wrote: > > > 2018-06-01 17:15 GMT+02:00 Ashutosh Bapat <ashutosh.bapat@enterprisedb.com > >: > >> On Fri, Jun 1, 2018 at 12:56 AM, Pavel Stehule <pavel.stehule@gmail.com> >> wrote: >> > Hi >> > >> > postgres=# SELECT count(*) from data; >> > ┌─────────┐ >> > │ count │ >> > ╞═════════╡ >> > │ 1000000 │ >> > └─────────┘ >> > (1 row) >> > >> > \dt+ can display actual size of partitioned table data - now zero is >> > displayed >> > >> > postgres=# \dt+ data >> > List of relations >> > ┌────────┬──────┬───────┬───────┬─────────┬─────────────┐ >> > │ Schema │ Name │ Type │ Owner │ Size │ Description │ >> > ╞════════╪══════╪═══════╪═══════╪═════════╪═════════════╡ >> > │ public │ data │ table │ pavel │ 0 bytes │ │ >> > └────────┴──────┴───────┴───────┴─────────┴─────────────┘ >> > (1 row) >> >> I think we should at least display "Type" as "partitioned table" for a >> partitioned table, so that it's easy to understand why the size is 0; >> partitioned tables do not hold any data by themselves. >> > > should be. > > Yes, or maybe we can add that info in "Description". > Some is missing still - there is not any total size across all partitions. > > maybe new command like > > \dtP+ .. show partitioned tables and their size > +1 Regards, Jeevan Ladhe
-
Re: ToDo: show size of partitioned table
Amit Langote <langote_amit_f8@lab.ntt.co.jp> — 2018-06-20T05:44:18Z
On 2018/06/02 0:15, Ashutosh Bapat wrote: > I think we should at least display "Type" as "partitioned table" for a > partitioned table, so that it's easy to understand why the size is 0; > partitioned tables do not hold any data by themselves. There was a long discussion last year (during PG 10 beta period), such as [1], and it seems most of us agreed to doing the above. Maybe, we should finally do it for PG 12, if not PG 11. Regarding showing the size of partitioned tables, there are many opinions and it's not clear if showing it in \dt itself is appropriate. For one, there is no pg_relation_size() or pg_table_size() equivalent in the backend for aggregating the size of all tables in a partition tree and I think people are not quite on board about having such a function in the backend [2]. Thanks, Amit [1] https://www.postgresql.org/message-id/flat/7dfc13c5-d6b7-1ff1-4bef-d75d6d2f76d9%40lab.ntt.co.jp [2] https://www.postgresql.org/message-id/flat/495cec7e-f8d9-7e13-4807-90dbf4eec4ea%40lab.ntt.co.jp
-
Re: ToDo: show size of partitioned table
Pavel Stehule <pavel.stehule@gmail.com> — 2018-06-20T07:21:18Z
2018-06-20 7:44 GMT+02:00 Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>: > On 2018/06/02 0:15, Ashutosh Bapat wrote: > > I think we should at least display "Type" as "partitioned table" for a > > partitioned table, so that it's easy to understand why the size is 0; > > partitioned tables do not hold any data by themselves. > > There was a long discussion last year (during PG 10 beta period), such as > [1], and it seems most of us agreed to doing the above. Maybe, we should > finally do it for PG 12, if not PG 11. > > Regarding showing the size of partitioned tables, there are many opinions > and it's not clear if showing it in \dt itself is appropriate. For one, > there is no pg_relation_size() or pg_table_size() equivalent in the > backend for aggregating the size of all tables in a partition tree and I > think people are not quite on board about having such a function in the > backend [2]. > Now, the number of partitions can be low, but if the Postgres can better process high number of partitions, then for some tables we can have hundreds partitions. Then usual \dt can be not too much usable. The aggregation can be done on client side. But maybe this idea is premature. Now, for PG 12, we can start with \dtP+ command for showing partition tables only with aggregate size via all related partitions. Is it acceptable idea? Regards Pavel > > Thanks, > Amit > > [1] > https://www.postgresql.org/message-id/flat/7dfc13c5-d6b7- > 1ff1-4bef-d75d6d2f76d9%40lab.ntt.co.jp > > [2] > https://www.postgresql.org/message-id/flat/495cec7e-f8d9- > 7e13-4807-90dbf4eec4ea%40lab.ntt.co.jp > >
-
Re: ToDo: show size of partitioned table
Amit Langote <langote_amit_f8@lab.ntt.co.jp> — 2018-06-20T07:44:04Z
On 2018/06/20 16:21, Pavel Stehule wrote: > 2018-06-20 7:44 GMT+02:00 Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>: > >> On 2018/06/02 0:15, Ashutosh Bapat wrote: >>> I think we should at least display "Type" as "partitioned table" for a >>> partitioned table, so that it's easy to understand why the size is 0; >>> partitioned tables do not hold any data by themselves. >> >> There was a long discussion last year (during PG 10 beta period), such as >> [1], and it seems most of us agreed to doing the above. Maybe, we should >> finally do it for PG 12, if not PG 11. >> >> Regarding showing the size of partitioned tables, there are many opinions >> and it's not clear if showing it in \dt itself is appropriate. For one, >> there is no pg_relation_size() or pg_table_size() equivalent in the >> backend for aggregating the size of all tables in a partition tree and I >> think people are not quite on board about having such a function in the >> backend [2]. > > Now, the number of partitions can be low, but if the Postgres can better > process high number of partitions, then for some tables we can have > hundreds partitions. > > Then usual \dt can be not too much usable. The aggregation can be done on > client side. But maybe this idea is premature. Now, for PG 12, we can start > with > > \dtP+ command for showing partition tables only with aggregate size via all > related partitions. > > Is it acceptable idea? Do you mean \dt continues to show size 0 for partitioned tables, but with the new option (\dtP+) shows the actual size by aggregating across partitions? +1 to such a feature, but we need to agree on an acceptable implementation for that. How does the aggregation happen: 1. In a new dedicated function in the backend (parallel to pg_table_size)? or 2. psql issues a separate query to compute the total size of a partition tree For option 2, I had posted a patch that simplifies writing such a query and posted that here: https://www.postgresql.org/message-id/7a9c5328-5328-52a3-2a3d-bf1434b4dd1d%40lab.ntt.co.jp With that patch, the query to get the total size of a partition tree becomes as simple as: select sum(pg_table_size(p)) as size from pg_get_inheritance_tables('partitioned_table_name') p Thanks, Amit -
Re: ToDo: show size of partitioned table
Pavel Stehule <pavel.stehule@gmail.com> — 2018-06-20T07:50:11Z
2018-06-20 9:44 GMT+02:00 Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>: > On 2018/06/20 16:21, Pavel Stehule wrote: > > 2018-06-20 7:44 GMT+02:00 Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>: > > > >> On 2018/06/02 0:15, Ashutosh Bapat wrote: > >>> I think we should at least display "Type" as "partitioned table" for a > >>> partitioned table, so that it's easy to understand why the size is 0; > >>> partitioned tables do not hold any data by themselves. > >> > >> There was a long discussion last year (during PG 10 beta period), such > as > >> [1], and it seems most of us agreed to doing the above. Maybe, we > should > >> finally do it for PG 12, if not PG 11. > >> > >> Regarding showing the size of partitioned tables, there are many > opinions > >> and it's not clear if showing it in \dt itself is appropriate. For one, > >> there is no pg_relation_size() or pg_table_size() equivalent in the > >> backend for aggregating the size of all tables in a partition tree and I > >> think people are not quite on board about having such a function in the > >> backend [2]. > > > > Now, the number of partitions can be low, but if the Postgres can better > > process high number of partitions, then for some tables we can have > > hundreds partitions. > > > > Then usual \dt can be not too much usable. The aggregation can be done on > > client side. But maybe this idea is premature. Now, for PG 12, we can > start > > with > > > > \dtP+ command for showing partition tables only with aggregate size via > all > > related partitions. > > > > Is it acceptable idea? > > Do you mean \dt continues to show size 0 for partitioned tables, but with > the new option (\dtP+) shows the actual size by aggregating across > partitions? +1 to such a feature, but we need to agree on an acceptable > implementation for that. How does the aggregation happen: > yes - my proposal is no change for \dt for now. I think so we will have to change it, when partitioning will be more common and number of partitions will be high. But it is not today. \dtP shows only partitions tables (like \dtS shows only system tables), with "+" shows sum of all related partitions. > 1. In a new dedicated function in the backend (parallel to pg_table_size)? > > or > > 2. psql issues a separate query to compute the total size of a partition > tree > In this moment we can simply do sum on client side, so it is related to @2. > For option 2, I had posted a patch that simplifies writing such a query > and posted that here: > > https://www.postgresql.org/message-id/7a9c5328-5328-52a3- > 2a3d-bf1434b4dd1d%40lab.ntt.co.jp > > With that patch, the query to get the total size of a partition tree > becomes as simple as: > > select sum(pg_table_size(p)) as size > from pg_get_inheritance_tables('partitioned_table_name') p > good to know it. Thank you. Do you think so your patch should be included to this feature or will be processed independently? Regards Pavel > Thanks, > Amit > > -
Re: ToDo: show size of partitioned table
Amit Langote <langote_amit_f8@lab.ntt.co.jp> — 2018-06-20T08:03:46Z
On 2018/06/20 16:50, Pavel Stehule wrote: > 2018-06-20 9:44 GMT+02:00 Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>: >> Do you mean \dt continues to show size 0 for partitioned tables, but with >> the new option (\dtP+) shows the actual size by aggregating across >> partitions? +1 to such a feature, but we need to agree on an acceptable >> implementation for that. How does the aggregation happen: >> > > yes - my proposal is no change for \dt for now. I think so we will have to > change it, when partitioning will be more common and number of partitions > will be high. But it is not today. > > \dtP shows only partitions tables (like \dtS shows only system tables), > with "+" shows sum of all related partitions. Ah, okay. That makes sense. >> 1. In a new dedicated function in the backend (parallel to pg_table_size)? >> >> or >> >> 2. psql issues a separate query to compute the total size of a partition >> tree >> > > In this moment we can simply do sum on client side, so it is related to @2. I see, okay. >> For option 2, I had posted a patch that simplifies writing such a query >> and posted that here: >> >> https://www.postgresql.org/message-id/7a9c5328-5328-52a3- >> 2a3d-bf1434b4dd1d%40lab.ntt.co.jp >> >> With that patch, the query to get the total size of a partition tree >> becomes as simple as: >> >> select sum(pg_table_size(p)) as size >> from pg_get_inheritance_tables('partitioned_table_name') p >> > > good to know it. Thank you. Do you think so your patch should be included > to this feature or will be processed independently? It seems that it would be useful on its own, as people may want to do various things once we provide them pg_get_inheritance_table. Thanks, Amit -
Re: ToDo: show size of partitioned table
Pavel Stehule <pavel.stehule@gmail.com> — 2018-06-20T08:25:37Z
2018-06-20 10:03 GMT+02:00 Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>: > On 2018/06/20 16:50, Pavel Stehule wrote: > > 2018-06-20 9:44 GMT+02:00 Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>: > >> Do you mean \dt continues to show size 0 for partitioned tables, but > with > >> the new option (\dtP+) shows the actual size by aggregating across > >> partitions? +1 to such a feature, but we need to agree on an acceptable > >> implementation for that. How does the aggregation happen: > >> > > > > yes - my proposal is no change for \dt for now. I think so we will have > to > > change it, when partitioning will be more common and number of partitions > > will be high. But it is not today. > > > > \dtP shows only partitions tables (like \dtS shows only system tables), > > with "+" shows sum of all related partitions. > > Ah, okay. That makes sense. > > >> 1. In a new dedicated function in the backend (parallel to > pg_table_size)? > >> > >> or > >> > >> 2. psql issues a separate query to compute the total size of a partition > >> tree > >> > > > > In this moment we can simply do sum on client side, so it is related to > @2. > > I see, okay. > > >> For option 2, I had posted a patch that simplifies writing such a query > >> and posted that here: > >> > >> https://www.postgresql.org/message-id/7a9c5328-5328-52a3- > >> 2a3d-bf1434b4dd1d%40lab.ntt.co.jp > >> > >> With that patch, the query to get the total size of a partition tree > >> becomes as simple as: > >> > >> select sum(pg_table_size(p)) as size > >> from pg_get_inheritance_tables('partitioned_table_name') p > >> > > > > good to know it. Thank you. Do you think so your patch should be included > > to this feature or will be processed independently? > > It seems that it would be useful on its own, as people may want to do > various things once we provide them pg_get_inheritance_table. > ok I'll prepare patch and I'll do note about dependency on your patch. Regards Pavel > Thanks, > Amit > > -
Re: ToDo: show size of partitioned table
Pavel Stehule <pavel.stehule@gmail.com> — 2018-07-23T11:46:30Z
Hi I am sending a prototype of patch. Now, it calculates size of partitioned tables with recursive query. When any more simple method will be possible, the size calculation will be changed. postgres=# \dt+ List of relations +--------+------------+-------+-------+---------+-------------+ | Schema | Name | Type | Owner | Size | Description | +--------+------------+-------+-------+---------+-------------+ | public | data | table | pavel | 0 bytes | | | public | data_2016 | table | pavel | 15 MB | | | public | data_2017 | table | pavel | 15 MB | | | public | data_other | table | pavel | 11 MB | | +--------+------------+-------+-------+---------+-------------+ (4 rows) postgres=# \dP+ List of partitioned tables +--------+------+-------+-------+-------------+ | Schema | Name | Owner | Size | Description | +--------+------+-------+-------+-------------+ | public | data | pavel | 42 MB | | +--------+------+-------+-------+-------------+ (1 row) Regards Pavel p.s. Another patch can be replacement of relation type from "table" to "partitioned table" postgres=# \dt+ List of relations +--------+------------+-------------------+-------+---------+-------------+ | Schema | Name | Type | Owner | Size | Description | +--------+------------+-------------------+-------+---------+-------------+ | public | data | partitioned table | pavel | 0 bytes | | | public | data_2016 | table | pavel | 15 MB | | | public | data_2017 | table | pavel | 15 MB | | | public | data_other | table | pavel | 11 MB | | +--------+------------+-------------------+-------+---------+-------------+ (4 rows) A patch is simple for this case diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c index c3bdf8555d..491e58eb29 100644 --- a/src/bin/psql/describe.c +++ b/src/bin/psql/describe.c @@ -3490,8 +3490,8 @@ listTables(const char *tabtypes, const char *pattern, bool verbose, bool showSys gettext_noop("sequence"), gettext_noop("special"), gettext_noop("foreign table"), - gettext_noop("table"), /* partitioned table */ - gettext_noop("index"), /* partitioned index */ + gettext_noop("partitioned table"), /* partitioned table */ + gettext_noop("partitioned index"), /* partitioned index */ gettext_noop("Type"), gettext_noop("Owner")); -
Re: ToDo: show size of partitioned table
Amit Langote <langote_amit_f8@lab.ntt.co.jp> — 2018-07-25T09:09:05Z
Hi Pavel. On 2018/07/23 20:46, Pavel Stehule wrote: > Hi > > I am sending a prototype of patch. Now, it calculates size of partitioned > tables with recursive query. When any more simple method will be possible, > the size calculation will be changed. > > postgres=# \dt+ > List of relations > +--------+------------+-------+-------+---------+-------------+ > | Schema | Name | Type | Owner | Size | Description | > +--------+------------+-------+-------+---------+-------------+ > | public | data | table | pavel | 0 bytes | | > | public | data_2016 | table | pavel | 15 MB | | > | public | data_2017 | table | pavel | 15 MB | | > | public | data_other | table | pavel | 11 MB | | > +--------+------------+-------+-------+---------+-------------+ > (4 rows) > > postgres=# \dP+ > List of partitioned tables > +--------+------+-------+-------+-------------+ > | Schema | Name | Owner | Size | Description | > +--------+------+-------+-------+-------------+ > | public | data | pavel | 42 MB | | > +--------+------+-------+-------+-------------+ > (1 row) This looks nice, although I haven't looked at the patch yet. Also, as you said, we could later replace the method of directly querying pg_inherits by something else. > p.s. Another patch can be replacement of relation type from "table" to > "partitioned table" > > postgres=# \dt+ > List of relations > +--------+------------+-------------------+-------+---------+-------------+ > | Schema | Name | Type | Owner | Size | Description | > +--------+------------+-------------------+-------+---------+-------------+ > | public | data | partitioned table | pavel | 0 bytes | | > | public | data_2016 | table | pavel | 15 MB | | > | public | data_2017 | table | pavel | 15 MB | | > | public | data_other | table | pavel | 11 MB | | > +--------+------------+-------------------+-------+---------+-------------+ > (4 rows) > > A patch is simple for this case > > diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c > index c3bdf8555d..491e58eb29 100644 > --- a/src/bin/psql/describe.c > +++ b/src/bin/psql/describe.c > @@ -3490,8 +3490,8 @@ listTables(const char *tabtypes, const char *pattern, > bool verbose, bool showSys > gettext_noop("sequence"), > gettext_noop("special"), > gettext_noop("foreign table"), > - gettext_noop("table"), /* partitioned table */ > - gettext_noop("index"), /* partitioned index */ > + gettext_noop("partitioned table"), /* partitioned > table */ > + gettext_noop("partitioned index"), /* partitioned > index */ > gettext_noop("Type"), > gettext_noop("Owner")); Inclined to +1 this, too. Although, one might ask why partitioned tables are called so only in the psql's output, but not in the backend's error messages, for example, as was discussed in the past. Thanks, Amit -
Re: ToDo: show size of partitioned table
Pavel Stehule <pavel.stehule@gmail.com> — 2018-07-25T09:18:44Z
2018-07-25 11:09 GMT+02:00 Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>: > Hi Pavel. > > On 2018/07/23 20:46, Pavel Stehule wrote: > > Hi > > > > I am sending a prototype of patch. Now, it calculates size of partitioned > > tables with recursive query. When any more simple method will be > possible, > > the size calculation will be changed. > > > > postgres=# \dt+ > > List of relations > > +--------+------------+-------+-------+---------+-------------+ > > | Schema | Name | Type | Owner | Size | Description | > > +--------+------------+-------+-------+---------+-------------+ > > | public | data | table | pavel | 0 bytes | | > > | public | data_2016 | table | pavel | 15 MB | | > > | public | data_2017 | table | pavel | 15 MB | | > > | public | data_other | table | pavel | 11 MB | | > > +--------+------------+-------+-------+---------+-------------+ > > (4 rows) > > > > postgres=# \dP+ > > List of partitioned tables > > +--------+------+-------+-------+-------------+ > > | Schema | Name | Owner | Size | Description | > > +--------+------+-------+-------+-------------+ > > | public | data | pavel | 42 MB | | > > +--------+------+-------+-------+-------------+ > > (1 row) > > This looks nice, although I haven't looked at the patch yet. Also, as you > said, we could later replace the method of directly querying pg_inherits > by something else. > > > p.s. Another patch can be replacement of relation type from "table" to > > "partitioned table" > > > > postgres=# \dt+ > > List of relations > > +--------+------------+-------------------+-------+--------- > +-------------+ > > | Schema | Name | Type | Owner | Size | > Description | > > +--------+------------+-------------------+-------+--------- > +-------------+ > > | public | data | partitioned table | pavel | 0 bytes | > | > > | public | data_2016 | table | pavel | 15 MB | > | > > | public | data_2017 | table | pavel | 15 MB | > | > > | public | data_other | table | pavel | 11 MB | > | > > +--------+------------+-------------------+-------+--------- > +-------------+ > > (4 rows) > > > > A patch is simple for this case > > > > diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c > > index c3bdf8555d..491e58eb29 100644 > > --- a/src/bin/psql/describe.c > > +++ b/src/bin/psql/describe.c > > @@ -3490,8 +3490,8 @@ listTables(const char *tabtypes, const char > *pattern, > > bool verbose, bool showSys > > gettext_noop("sequence"), > > gettext_noop("special"), > > gettext_noop("foreign table"), > > - gettext_noop("table"), /* partitioned table */ > > - gettext_noop("index"), /* partitioned index */ > > + gettext_noop("partitioned table"), /* > partitioned > > table */ > > + gettext_noop("partitioned index"), /* > partitioned > > index */ > > gettext_noop("Type"), > > gettext_noop("Owner")); > > Inclined to +1 this, too. Although, one might ask why partitioned tables > are called so only in the psql's output, but not in the backend's error > messages, for example, as was discussed in the past. > i think so error messages is different chapter - it means revision of all error messages, and probably somewhere the name of partition, and not partition table can be correct. Personally I am not sure about benefit to change error messages. Now, the partition is table too, so the error messages are not strongly wrong. Regards Pavel > Thanks, > Amit > > -
Re: ToDo: show size of partitioned table
Mathias Brossard <postgresql@zoinx.org> — 2018-08-16T03:51:31Z
The following review has been posted through the commitfest application: make installcheck-world: tested, failed Implements feature: tested, passed Spec compliant: not tested Documentation: tested, passed Hi, I'm with Melanie Plageman running the SVPUG Patch Review Meetup. I tested this feature on my Mac. The patch applied cleanly on master, and the feature worked as expected with the SQL at the bottom of this email (Jesse Zhang suggested the two-level partitioning). installcheck passed but installcheck-world did not. I do have a feedback on the implementation. The code tries to support older PostgreSQL server versions when declarative partitions were not supported before version 10 (relkind value of 'p'). Those versions will never return any result from the query being built. So I would suggest an early return from the function. The upside would be that the query building would be simpler. I can make patch implementing that suggestion if you want. Sincerely, -- Mathias Brossard CREATE TABLE partition ( part int not null, value int not null ) PARTITION BY RANGE (part); CREATE TABLE partition_0 PARTITION OF partition FOR VALUES FROM (0) TO (10); CREATE TABLE partition_1 PARTITION OF partition FOR VALUES FROM (10) TO (20); CREATE TABLE partition_2 PARTITION OF partition FOR VALUES FROM (20) TO (30); CREATE TABLE partition_3 PARTITION OF partition FOR VALUES FROM (30) TO (40); CREATE TABLE partition_4 PARTITION OF partition FOR VALUES FROM (40) TO (50); CREATE TABLE partition_5 PARTITION OF partition FOR VALUES FROM (50) TO (60); CREATE TABLE partition_6 PARTITION OF partition FOR VALUES FROM (60) TO (70); CREATE TABLE partition_7 PARTITION OF partition FOR VALUES FROM (70) TO (80); CREATE TABLE partition_8 PARTITION OF partition FOR VALUES FROM (80) TO (90); CREATE TABLE partition_9 ( part int not null, value int not null ) PARTITION BY RANGE (part); CREATE TABLE partition_9a PARTITION OF partition_9 FOR VALUES FROM (90) TO (95); CREATE TABLE partition_9b PARTITION OF partition_9 FOR VALUES FROM (95) TO (100); ALTER TABLE partition ATTACH PARTITION partition_9 FOR VALUES FROM (90) TO (100); INSERT INTO partition SELECT i % 100 AS part, i AS value FROM generate_series(1, 1000000) AS i; ------------------- -- Below is the resulting output test=# \dt+ List of relations Schema | Name | Type | Owner | Size | Description --------+--------------+-------+-----------+---------+------------- public | partition | table | mbrossard | 0 bytes | public | partition_0 | table | mbrossard | 3568 kB | public | partition_1 | table | mbrossard | 3568 kB | public | partition_2 | table | mbrossard | 3568 kB | public | partition_3 | table | mbrossard | 3568 kB | public | partition_4 | table | mbrossard | 3568 kB | public | partition_5 | table | mbrossard | 3568 kB | public | partition_6 | table | mbrossard | 3568 kB | public | partition_7 | table | mbrossard | 3568 kB | public | partition_8 | table | mbrossard | 3568 kB | public | partition_9 | table | mbrossard | 0 bytes | public | partition_9a | table | mbrossard | 1800 kB | public | partition_9b | table | mbrossard | 1800 kB | (13 rows) test=# \dP+ List of partitioned tables Schema | Name | Owner | Size | Description --------+-------------+-----------+---------+------------- public | partition | mbrossard | 35 MB | public | partition_9 | mbrossard | 3600 kB | (2 rows) test=# \dP+ *9 List of partitioned tables Schema | Name | Owner | Size | Description --------+-------------+-----------+---------+------------- public | partition_9 | mbrossard | 3600 kB | (1 row) -
Re: ToDo: show size of partitioned table
Pavel Stehule <pavel.stehule@gmail.com> — 2018-08-16T07:45:30Z
Hi čt 16. 8. 2018 v 5:52 odesílatel Mathias Brossard <postgresql@zoinx.org> napsal: > The following review has been posted through the commitfest application: > make installcheck-world: tested, failed > Implements feature: tested, passed > Spec compliant: not tested > Documentation: tested, passed > > Hi, > > I'm with Melanie Plageman running the SVPUG Patch Review Meetup. I tested > this feature on my Mac. The patch applied cleanly on master, and the > feature worked as expected with the SQL at the bottom of this email (Jesse > Zhang suggested the two-level partitioning). installcheck passed but > installcheck-world did not. > > I do have a feedback on the implementation. The code tries to support > older PostgreSQL server versions when declarative partitions were not > supported before version 10 (relkind value of 'p'). Those versions will > never return any result from the query being built. So I would suggest an > early return from the function. The upside would be that the query building > would be simpler. I can make patch implementing that suggestion if you want. > This is question - maybe we can support older partitioning based on only inheritance - and the query can be more exact on PostgreSQL 10 and newer. Please, send any patch. You are welcome. Regards Pavel > > Sincerely, > -- Mathias Brossard > > > CREATE TABLE partition ( > part int not null, > value int not null > ) PARTITION BY RANGE (part); > > CREATE TABLE partition_0 PARTITION OF partition FOR VALUES FROM (0) TO > (10); > CREATE TABLE partition_1 PARTITION OF partition FOR VALUES FROM (10) TO > (20); > CREATE TABLE partition_2 PARTITION OF partition FOR VALUES FROM (20) TO > (30); > CREATE TABLE partition_3 PARTITION OF partition FOR VALUES FROM (30) TO > (40); > CREATE TABLE partition_4 PARTITION OF partition FOR VALUES FROM (40) TO > (50); > CREATE TABLE partition_5 PARTITION OF partition FOR VALUES FROM (50) TO > (60); > CREATE TABLE partition_6 PARTITION OF partition FOR VALUES FROM (60) TO > (70); > CREATE TABLE partition_7 PARTITION OF partition FOR VALUES FROM (70) TO > (80); > CREATE TABLE partition_8 PARTITION OF partition FOR VALUES FROM (80) TO > (90); > CREATE TABLE partition_9 ( > part int not null, > value int not null > ) PARTITION BY RANGE (part); > > CREATE TABLE partition_9a PARTITION OF partition_9 FOR VALUES FROM (90) TO > (95); > CREATE TABLE partition_9b PARTITION OF partition_9 FOR VALUES FROM (95) TO > (100); > ALTER TABLE partition ATTACH PARTITION partition_9 FOR VALUES FROM (90) TO > (100); > > INSERT INTO partition SELECT i % 100 AS part, i AS value FROM > generate_series(1, 1000000) AS i; > > ------------------- > -- Below is the resulting output > > test=# \dt+ > List of relations > Schema | Name | Type | Owner | Size | Description > --------+--------------+-------+-----------+---------+------------- > public | partition | table | mbrossard | 0 bytes | > public | partition_0 | table | mbrossard | 3568 kB | > public | partition_1 | table | mbrossard | 3568 kB | > public | partition_2 | table | mbrossard | 3568 kB | > public | partition_3 | table | mbrossard | 3568 kB | > public | partition_4 | table | mbrossard | 3568 kB | > public | partition_5 | table | mbrossard | 3568 kB | > public | partition_6 | table | mbrossard | 3568 kB | > public | partition_7 | table | mbrossard | 3568 kB | > public | partition_8 | table | mbrossard | 3568 kB | > public | partition_9 | table | mbrossard | 0 bytes | > public | partition_9a | table | mbrossard | 1800 kB | > public | partition_9b | table | mbrossard | 1800 kB | > (13 rows) > > test=# \dP+ > List of partitioned tables > Schema | Name | Owner | Size | Description > --------+-------------+-----------+---------+------------- > public | partition | mbrossard | 35 MB | > public | partition_9 | mbrossard | 3600 kB | > (2 rows) > > test=# \dP+ *9 > List of partitioned tables > Schema | Name | Owner | Size | Description > --------+-------------+-----------+---------+------------- > public | partition_9 | mbrossard | 3600 kB | > (1 row)
-
Re: ToDo: show size of partitioned table
Mathias Brossard <mathias@brossard.org> — 2018-08-17T03:26:19Z
On Thu, Aug 16, 2018 at 12:46 AM Pavel Stehule <pavel.stehule@gmail.com> wrote: > čt 16. 8. 2018 v 5:52 odesílatel Mathias Brossard <postgresql@zoinx.org> > napsal: > >> I do have a feedback on the implementation. The code tries to support >> older PostgreSQL server versions when declarative partitions were not >> supported before version 10 (relkind value of 'p'). Those versions will >> never return any result from the query being built. So I would suggest an >> early return from the function. The upside would be that the query building >> would be simpler. I can make patch implementing that suggestion if you want. >> > > This is question - maybe we can support older partitioning based on only > inheritance - and the query can be more exact on PostgreSQL 10 and newer. > > Please, send any patch. You are welcome. > In my very humble opinion, I would restrict the definition of partitions to declarative partitioning. My justification would be that partitions all use inheritance, but not all inheritance is a partition (how would you handle multiple inheritance). See patch attached that fails (in a way similar to other features) when connected to servers with version earlier than 10.0. Sincerely, -- Mathias Brossard
-
Re: ToDo: show size of partitioned table
Michael Paquier <michael@paquier.xyz> — 2018-10-01T05:57:09Z
On Wed, Jul 25, 2018 at 06:09:05PM +0900, Amit Langote wrote: > This looks nice, although I haven't looked at the patch yet. Also, as you > said, we could later replace the method of directly querying pg_inherits > by something else. Yes, at the end we may be looking at something like that which would avoid the need of any WITH RECURSIVE queries: https://commitfest.postgresql.org/19/1694/ Tha patch still applies, but I think that we'd want to do something for the partition tree function first, and then come to this one. Moved to next CF. -- Michael
-
Re: ToDo: show size of partitioned table
Amit Langote <langote_amit_f8@lab.ntt.co.jp> — 2018-10-30T06:51:46Z
Hi Mathias, Pavel, On 2018/08/17 12:26, Mathias Brossard wrote: > On Thu, Aug 16, 2018 at 12:46 AM Pavel Stehule <pavel.stehule@gmail.com> >> >> This is question - maybe we can support older partitioning based on only >> inheritance - and the query can be more exact on PostgreSQL 10 and newer. >> >> Please, send any patch. You are welcome. > > In my very humble opinion, I would restrict the definition of partitions to > declarative partitioning. My justification would be that partitions all use > inheritance, but not all inheritance is a partition (how would you handle > multiple inheritance). > > See patch attached that fails (in a way similar to other features) when > connected to servers with version earlier than 10.0. The patch to add the pg_partition_tree() function was just committed: Add pg_partition_tree to display information about partitions https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=d5eec4eefde70 Could one of you please revise the patch to use that function to produce the output of \dP+? Note that pg_partition_tree simply scans the underlying catalog to get all the tables, so if you pass it a table that's not partitioned (relkind == 'r'), but has inheritance children, the children will be returned in its output. So, if you want to limit the output of \dP to partitioned tables, be sure to include relkind = 'p' condition in the query. Thanks, Amit
-
Re: ToDo: show size of partitioned table
Pavel Stehule <pavel.stehule@gmail.com> — 2018-10-30T11:03:49Z
Hi út 30. 10. 2018 v 7:52 odesílatel Amit Langote < Langote_Amit_f8@lab.ntt.co.jp> napsal: > Hi Mathias, Pavel, > > On 2018/08/17 12:26, Mathias Brossard wrote: > > On Thu, Aug 16, 2018 at 12:46 AM Pavel Stehule <pavel.stehule@gmail.com> > >> > >> This is question - maybe we can support older partitioning based on only > >> inheritance - and the query can be more exact on PostgreSQL 10 and > newer. > >> > >> Please, send any patch. You are welcome. > > > > In my very humble opinion, I would restrict the definition of partitions > to > > declarative partitioning. My justification would be that partitions all > use > > inheritance, but not all inheritance is a partition (how would you handle > > multiple inheritance). > > > > See patch attached that fails (in a way similar to other features) when > > connected to servers with version earlier than 10.0. > > The patch to add the pg_partition_tree() function was just committed: > > Add pg_partition_tree to display information about partitions > > https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=d5eec4eefde70 > > Could one of you please revise the patch to use that function to produce > the output of \dP+? > here it is. It is based on Mathias's patch. Although we can use pg_partition_tree on PostgreSQL, we still should to support PostgreSQL 10, 11 where this function is not available Regards Pavel > Note that pg_partition_tree simply scans the underlying catalog to get all > the tables, so if you pass it a table that's not partitioned (relkind == > 'r'), but has inheritance children, the children will be returned in its > output. So, if you want to limit the output of \dP to partitioned tables, > be sure to include relkind = 'p' condition in the query. > > Thanks, > Amit > >
-
Re: ToDo: show size of partitioned table
Amit Langote <amitlangote09@gmail.com> — 2018-10-30T12:24:01Z
On Tue, Oct 30, 2018 at 8:04 PM Pavel Stehule <pavel.stehule@gmail.com> wrote: > út 30. 10. 2018 v 7:52 odesílatel Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> napsal: >> The patch to add the pg_partition_tree() function was just committed: >> >> Add pg_partition_tree to display information about partitions >> https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=d5eec4eefde70 >> >> Could one of you please revise the patch to use that function to produce >> the output of \dP+? > > > here it is. > > It is based on Mathias's patch. Although we can use pg_partition_tree on PostgreSQL, we still should to support PostgreSQL 10, 11 where this function is not available Ah, I forgot that psql will need to consider 10 and 11 servers too. Thanks, Amit
-
Re: ToDo: show size of partitioned table
Michael Paquier <michael@paquier.xyz> — 2018-10-30T13:48:55Z
On Tue, Oct 30, 2018 at 09:24:01PM +0900, Amit Langote wrote: >> It is based on Mathias's patch. Although we can use >> pg_partition_tree on PostgreSQL, we still should to support >> PostgreSQL 10, 11 where this function is not available > > Ah, I forgot that psql will need to consider 10 and 11 servers too. + /* PostgreSQL 11 has pg_partition_tree function */ That's v12 here. + appendPQExpBuffer(&buf, + ",\n (SELECT pg_catalog.pg_size_pretty(sum(pg_catalog.pg_table_size(" + "relid)))\n" + " FROM pg_partition_tree(c.oid)) AS \"%s\"" You need to do schema qualification for functions and relations. -- Michael -
Re: ToDo: show size of partitioned table
Amit Langote <langote_amit_f8@lab.ntt.co.jp> — 2018-10-31T02:26:59Z
On 2018/10/30 20:03, Pavel Stehule wrote: > út 30. 10. 2018 v 7:52 odesílatel Amit Langote < > Langote_Amit_f8@lab.ntt.co.jp> napsal: >> Could one of you please revise the patch to use that function to produce >> the output of \dP+? >> > > here it is. > > It is based on Mathias's patch. Although we can use pg_partition_tree on > PostgreSQL, we still should to support PostgreSQL 10, 11 where this > function is not available Thanks for updating the patch. Just a couple of comments: + is used, a sum of size of related partitions and a description I suggest: is used, the sum of sizes of related partitions and associated description + appendPQExpBufferStr(&buf, "\nWHERE c.relkind IN ('p')\n"); I wonder if we should list partitioned indexes ('I') as well, because their size information is not available with \di+. But maybe, they should have a separate command. + if (PQntuples(res) == 0 && !pset.quiet) + { + if (pattern) + psql_error("Did not find any relation named \"%s\".\n", + pattern); + else + psql_error("Did not find any relations.\n"); + } I think we should use "partitioned table" instead of "relation" in the above error messages, because this command is specifically finding partitioned tables. (If we decide to include partitioned indexes as well, then the above error message should say "partitioned relation") + fprintf(output, _(" \\dP[+] [PATTERN] list partitioned tables\n")); Again, if we include indexes, this should be "partitioned relations". How about adding a couple of regression tests? Thanks, Amit -
Re: ToDo: show size of partitioned table
Pavel Stehule <pavel.stehule@gmail.com> — 2018-10-31T06:30:45Z
st 31. 10. 2018 v 3:27 odesílatel Amit Langote < Langote_Amit_f8@lab.ntt.co.jp> napsal: > On 2018/10/30 20:03, Pavel Stehule wrote: > > út 30. 10. 2018 v 7:52 odesílatel Amit Langote < > > Langote_Amit_f8@lab.ntt.co.jp> napsal: > >> Could one of you please revise the patch to use that function to produce > >> the output of \dP+? > >> > > > > here it is. > > > > It is based on Mathias's patch. Although we can use pg_partition_tree on > > PostgreSQL, we still should to support PostgreSQL 10, 11 where this > > function is not available > > Thanks for updating the patch. Just a couple of comments: > > + is used, a sum of size of related partitions and a description > > I suggest: > > is used, the sum of sizes of related partitions and associated description > > + appendPQExpBufferStr(&buf, "\nWHERE c.relkind IN ('p')\n"); > > I wonder if we should list partitioned indexes ('I') as well, because > their size information is not available with \di+. But maybe, they should > have a separate command. > I though about it too and I prefer separate command. Similar to \di+ > + if (PQntuples(res) == 0 && !pset.quiet) > + { > + if (pattern) > + psql_error("Did not find any relation named \"%s\".\n", > + pattern); > + else > + psql_error("Did not find any relations.\n"); > + } > > I think we should use "partitioned table" instead of "relation" in the > above error messages, because this command is specifically finding > partitioned tables. > > (If we decide to include partitioned indexes as well, then the above error > message should say "partitioned relation") > > + fprintf(output, _(" \\dP[+] [PATTERN] list partitioned > tables\n")); > > Again, if we include indexes, this should be "partitioned relations". > > How about adding a couple of regression tests? > I am not sure. Has not sense run this test over empty database, and some bigger database can increase running. More the size can be platform depend. Regards Pavel > > Thanks, > Amit > > -
Re: ToDo: show size of partitioned table
Amit Langote <langote_amit_f8@lab.ntt.co.jp> — 2018-10-31T06:34:02Z
On 2018/10/31 15:30, Pavel Stehule wrote: > st 31. 10. 2018 v 3:27 odesílatel Amit Langote < > Langote_Amit_f8@lab.ntt.co.jp> napsal: >> + appendPQExpBufferStr(&buf, "\nWHERE c.relkind IN ('p')\n"); >> >> I wonder if we should list partitioned indexes ('I') as well, because >> their size information is not available with \di+. But maybe, they should >> have a separate command. >> > > I though about it too and I prefer separate command. Similar to \di+ Okay, maybe \dI+. > I am not sure. Has not sense run this test over empty database, and some > bigger database can increase running. > > More the size can be platform depend. Okay, sure. Thanks, Amit -
Re: ToDo: show size of partitioned table
Michael Paquier <michael@paquier.xyz> — 2018-10-31T07:38:07Z
On Wed, Oct 31, 2018 at 03:34:02PM +0900, Amit Langote wrote: > On 2018/10/31 15:30, Pavel Stehule wrote: >> I am not sure. Has not sense run this test over empty database, and some >> bigger database can increase running. >> >> More the size can be platform depend. > > Okay, sure. Well, that would mostly depend on the relation page size which is defined at compilation, no? The plans of some queries are unstable depending on the page size so they would fail, but let's limit the damage if possible. -- Michael
-
Re: ToDo: show size of partitioned table
Pavel Stehule <pavel.stehule@gmail.com> — 2018-10-31T07:41:45Z
st 31. 10. 2018 v 8:38 odesílatel Michael Paquier <michael@paquier.xyz> napsal: > On Wed, Oct 31, 2018 at 03:34:02PM +0900, Amit Langote wrote: > > On 2018/10/31 15:30, Pavel Stehule wrote: > >> I am not sure. Has not sense run this test over empty database, and some > >> bigger database can increase running. > >> > >> More the size can be platform depend. > > > > Okay, sure. > > Well, that would mostly depend on the relation page size which is > defined at compilation, no? The plans of some queries are unstable > depending on the page size so they would fail, but let's limit the > damage if possible. > I am not sure - I remember one private test that we did on our patches, and this tests fails sometimes on 32bits. So I afraid about stability. Regards Pavel -- > Michael >
-
Re: ToDo: show size of partitioned table
Michael Paquier <michael@paquier.xyz> — 2018-10-31T08:03:39Z
On Wed, Oct 31, 2018 at 08:41:45AM +0100, Pavel Stehule wrote: > I am not sure - I remember one private test that we did on our patches, and > this tests fails sometimes on 32bits. So I afraid about stability. That could be possible as well. I think that you are right to be afraid of such things, and keeping the tests portable a maximum is always a good thing I think. -- Michael
-
Re: ToDo: show size of partitioned table
Pavel Stehule <pavel.stehule@gmail.com> — 2018-10-31T17:19:01Z
st 31. 10. 2018 v 7:34 odesílatel Amit Langote < Langote_Amit_f8@lab.ntt.co.jp> napsal: > On 2018/10/31 15:30, Pavel Stehule wrote: > > st 31. 10. 2018 v 3:27 odesílatel Amit Langote < > > Langote_Amit_f8@lab.ntt.co.jp> napsal: > >> + appendPQExpBufferStr(&buf, "\nWHERE c.relkind IN ('p')\n"); > >> > >> I wonder if we should list partitioned indexes ('I') as well, because > >> their size information is not available with \di+. But maybe, they > should > >> have a separate command. > >> > > > > I though about it too and I prefer separate command. Similar to \di+ > > Okay, maybe \dI+. > > what about combination \dPt+ for partitined tables and size \dPi+ for indexes on partitioned tables and size \dP+ for total partition size (tables + indexes) What do you think about it? Regards Pavel > > I am not sure. Has not sense run this test over empty database, and some > > bigger database can increase running. > > > > More the size can be platform depend. > > Okay, sure. > > Thanks, > Amit > > -
Re: ToDo: show size of partitioned table
Amit Langote <langote_amit_f8@lab.ntt.co.jp> — 2018-11-02T05:16:56Z
Hi, On 2018/11/01 2:19, Pavel Stehule wrote: > st 31. 10. 2018 v 7:34 odesílatel Amit Langote < > Langote_Amit_f8@lab.ntt.co.jp> napsal: >> On 2018/10/31 15:30, Pavel Stehule wrote: >>> st 31. 10. 2018 v 3:27 odesílatel Amit Langote < >>> Langote_Amit_f8@lab.ntt.co.jp> napsal: >>>> + appendPQExpBufferStr(&buf, "\nWHERE c.relkind IN ('p')\n"); >>>> >>>> I wonder if we should list partitioned indexes ('I') as well, because >>>> their size information is not available with \di+. But maybe, they >> should >>>> have a separate command. >>>> >>> >>> I though about it too and I prefer separate command. Similar to \di+ >> >> Okay, maybe \dI+. >> >> > what about combination > > \dPt+ for partitined tables and size > \dPi+ for indexes on partitioned tables and size > \dP+ for total partition size (tables + indexes) +1 Thanks, Amit -
Re: ToDo: show size of partitioned table
Pavel Stehule <pavel.stehule@gmail.com> — 2018-11-03T19:58:56Z
pá 2. 11. 2018 v 6:17 odesílatel Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> napsal: > Hi, > > On 2018/11/01 2:19, Pavel Stehule wrote: > > st 31. 10. 2018 v 7:34 odesílatel Amit Langote < > > Langote_Amit_f8@lab.ntt.co.jp> napsal: > >> On 2018/10/31 15:30, Pavel Stehule wrote: > >>> st 31. 10. 2018 v 3:27 odesílatel Amit Langote < > >>> Langote_Amit_f8@lab.ntt.co.jp> napsal: > >>>> + appendPQExpBufferStr(&buf, "\nWHERE c.relkind IN ('p')\n"); > >>>> > >>>> I wonder if we should list partitioned indexes ('I') as well, because > >>>> their size information is not available with \di+. But maybe, they > >> should > >>>> have a separate command. > >>>> > >>> > >>> I though about it too and I prefer separate command. Similar to \di+ > >> > >> Okay, maybe \dI+. > >> > >> > > what about combination > > > > \dPt+ for partitined tables and size > > \dPi+ for indexes on partitioned tables and size > > \dP+ for total partition size (tables + indexes) > > +1 > here is a patch postgres=# \dt+ List of relations ┌────────┬───────────┬───────┬───────┬─────────┬─────────────┐ │ Schema │ Name │ Type │ Owner │ Size │ Description │ ╞════════╪═══════════╪═══════╪═══════╪═════════╪═════════════╡ │ public │ data │ table │ pavel │ 0 bytes │ │ │ public │ data_2016 │ table │ pavel │ 18 MB │ │ │ public │ data_2017 │ table │ pavel │ 17 MB │ │ └────────┴───────────┴───────┴───────┴─────────┴─────────────┘ (3 rows) postgres=# \di+ List of relations ┌────────┬────────────────────────┬───────┬───────┬───────────┬─────────┬─────────────┐ │ Schema │ Name │ Type │ Owner │ Table │ Size │ Description │ ╞════════╪════════════════════════╪═══════╪═══════╪═══════════╪═════════╪═════════════╡ │ public │ data_2016_inserted_idx │ index │ pavel │ data_2016 │ 11 MB │ │ │ public │ data_2016_value_idx │ index │ pavel │ data_2016 │ 15 MB │ │ │ public │ data_2017_inserted_idx │ index │ pavel │ data_2017 │ 10 MB │ │ │ public │ data_2017_value_idx │ index │ pavel │ data_2017 │ 14 MB │ │ │ public │ data_inserted_idx │ index │ pavel │ data │ 0 bytes │ │ │ public │ data_value_idx │ index │ pavel │ data │ 0 bytes │ │ └────────┴────────────────────────┴───────┴───────┴───────────┴─────────┴─────────────┘ (6 rows) postgres=# \dP+ List of partitioned relations ┌────────┬──────┬───────┬───────┬─────────────┐ │ Schema │ Name │ Owner │ Size │ Description │ ╞════════╪══════╪═══════╪═══════╪═════════════╡ │ public │ data │ pavel │ 84 MB │ │ └────────┴──────┴───────┴───────┴─────────────┘ (1 row) postgres=# \dPt+ List of partitioned tables ┌────────┬──────┬───────┬───────┬─────────────┐ │ Schema │ Name │ Owner │ Size │ Description │ ╞════════╪══════╪═══════╪═══════╪═════════════╡ │ public │ data │ pavel │ 35 MB │ │ └────────┴──────┴───────┴───────┴─────────────┘ (1 row) postgres=# \dPi+ List of partitioned indexes ┌────────┬───────────────────┬───────┬───────┬───────┬─────────────┐ │ Schema │ Name │ Owner │ Table │ Size │ Description │ ╞════════╪═══════════════════╪═══════╪═══════╪═══════╪═════════════╡ │ public │ data_inserted_idx │ pavel │ data │ 21 MB │ │ │ public │ data_value_idx │ pavel │ data │ 28 MB │ │ └────────┴───────────────────┴───────┴───────┴───────┴─────────────┘ (2 rows) > Thanks, > Amit > > -
Re: ToDo: show size of partitioned table
Amit Langote <langote_amit_f8@lab.ntt.co.jp> — 2018-11-05T06:19:46Z
On 2018/11/04 4:58, Pavel Stehule wrote: > here is a patch Thank you, Pavel. Here are some comments. I mentioned it during the last review, but maybe you missed it due to the other discussion. + the pattern are listed. If the form <literal>\dP+</literal> + is used, a sum of size of related partitions and a description + are also displayed. + the pattern are listed. If the form <literal>\dPi+</literal> + is used, a sum of size of related indexes and a description + are also displayed. + the pattern are listed. If the form <literal>\dPt+</literal> + is used, a sum of size of related indexes and a description + are also displayed. I suggest: "is used, the sum of sizes of related partitions / index partitions / table partitions and associated description are also displayed." Note that I also fixed the typo (indexes -> tables) in the last one. Also, I wonder if we should mention in the description of \dP+ that the displayed size considers the sizes of both the tables and indexes on the individual partitions, because in the code, I see pg_total_relation_size being used. So, the text should be something like: "is used, the sum of size of related partitions (including the table and indexes, if any) and associated description are also displayed." Code itself looks to me to be in good shape, except you seem to have also missed Michael's comment upthread: + /* PostgreSQL 11 has pg_partition_tree function */ /* PostgreSQL 12 has pg_partition_tree function */ Thanks again. Regards, Amit
-
Re: ToDo: show size of partitioned table
Pavel Stehule <pavel.stehule@gmail.com> — 2018-11-05T10:43:16Z
po 5. 11. 2018 v 7:20 odesílatel Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> napsal: > On 2018/11/04 4:58, Pavel Stehule wrote: > > here is a patch > > Thank you, Pavel. > > Here are some comments. > > I mentioned it during the last review, but maybe you missed it due to the > other discussion. > > + the pattern are listed. If the form <literal>\dP+</literal> > + is used, a sum of size of related partitions and a description > + are also displayed. > > + the pattern are listed. If the form <literal>\dPi+</literal> > + is used, a sum of size of related indexes and a description > + are also displayed. > > + the pattern are listed. If the form <literal>\dPt+</literal> > + is used, a sum of size of related indexes and a description > + are also displayed. > > I suggest: > > "is used, the sum of sizes of related partitions / index partitions / > table partitions and associated description are also displayed." > > Note that I also fixed the typo (indexes -> tables) in the last one. > > Also, I wonder if we should mention in the description of \dP+ that the > displayed size considers the sizes of both the tables and indexes on the > individual partitions, because in the code, I see pg_total_relation_size > being used. So, the text should be something like: > > "is used, the sum of size of related partitions (including the table and > indexes, if any) and associated description are also displayed." > > should be fixed now > Code itself looks to me to be in good shape, except you seem to have also > missed Michael's comment upthread: > > + /* PostgreSQL 11 has pg_partition_tree function */ > > /* PostgreSQL 12 has pg_partition_tree function */ > should be fixed too > Thanks again. > > Regards, > Amit > > Updated patch attached Regards Pavel
-
Re: ToDo: show size of partitioned table
Alvaro Herrera <alvherre@2ndquadrant.com> — 2018-11-19T02:17:37Z
On 2018-Jul-23, Pavel Stehule wrote: > p.s. Another patch can be replacement of relation type from "table" to > "partitioned table" > > postgres=# \dt+ > List of relations > +--------+------------+-------------------+-------+---------+-------------+ > | Schema | Name | Type | Owner | Size | Description | > +--------+------------+-------------------+-------+---------+-------------+ > | public | data | partitioned table | pavel | 0 bytes | | > | public | data_2016 | table | pavel | 15 MB | | > | public | data_2017 | table | pavel | 15 MB | | > | public | data_other | table | pavel | 11 MB | | > +--------+------------+-------------------+-------+---------+-------------+ > (4 rows) I think this is a clear improvement. The term "table" was introduced for this case by f0e44751d7 ("Implement table partitioning.") and now the author of that commit supports this change. I used the term "index" for partitioned indexes originally because I was copying the existing term, but now I too think they should say "partitioned indexes" instead, because they are different enough objects from plain indexes. To be certain I'm not going against some old decision, I digged up Amit's old patches. Turns out he submitted psql's describe.c using the term "partitioned table" on August 10th [1] and then based on a discussion where Robert suggested calling these new objects "partition roots" instead to avoid confusion, it was changed to "table" in the next submission on August 26th [2]. It seems the right call to have used the term "table" in many places (rather than "partition roots"), but at least in psql's \dt it seems extremely useful to show the type as "partitioned table" instead, because it is one place where the distinction is clearly useful. In this thread there have been no contrary votes, so I'm pushing this part soon. [1] https://postgr.es/m/ad16e2f5-fc7c-cc2d-333a-88d4aa446f96@lab.ntt.co.jp [2] https://postgr.es/m/169708f6-6e5a-18d1-707b-1b323e4a6baf@lab.ntt.co.jp -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -
Re: ToDo: show size of partitioned table
Amit Langote <langote_amit_f8@lab.ntt.co.jp> — 2018-11-19T02:37:46Z
On 2018/11/19 11:17, Alvaro Herrera wrote: > On 2018-Jul-23, Pavel Stehule wrote: > >> p.s. Another patch can be replacement of relation type from "table" to >> "partitioned table" >> >> postgres=# \dt+ >> List of relations >> +--------+------------+-------------------+-------+---------+-------------+ >> | Schema | Name | Type | Owner | Size | Description | >> +--------+------------+-------------------+-------+---------+-------------+ >> | public | data | partitioned table | pavel | 0 bytes | | >> | public | data_2016 | table | pavel | 15 MB | | >> | public | data_2017 | table | pavel | 15 MB | | >> | public | data_other | table | pavel | 11 MB | | >> +--------+------------+-------------------+-------+---------+-------------+ >> (4 rows) > > I think this is a clear improvement. The term "table" was introduced > for this case by f0e44751d7 ("Implement table partitioning.") and now > the author of that commit supports this change. I used the term "index" > for partitioned indexes originally because I was copying the existing > term, but now I too think they should say "partitioned indexes" instead, > because they are different enough objects from plain indexes. > > To be certain I'm not going against some old decision, I digged up > Amit's old patches. Turns out he submitted psql's describe.c using the > term "partitioned table" on August 10th [1] and then based on a > discussion where Robert suggested calling these new objects "partition > roots" instead to avoid confusion, it was changed to "table" in the next > submission on August 26th [2]. It seems the right call to have used the > term "table" in many places (rather than "partition roots"), but at > least in psql's \dt it seems extremely useful to show the type as > "partitioned table" instead, because it is one place where the > distinction is clearly useful. > > In this thread there have been no contrary votes, so I'm pushing this > part soon. > > [1] https://postgr.es/m/ad16e2f5-fc7c-cc2d-333a-88d4aa446f96@lab.ntt.co.jp > [2] https://postgr.es/m/169708f6-6e5a-18d1-707b-1b323e4a6baf@lab.ntt.co.jp Yeah, I agree that showing "partitioned table" for partitioned tables in this case is helpful. Earlier on this thread [1], I had expressed a slight concern about the consistency of mentioning "partitioned" in various outputs, because many error messages say "table" even if the table is partitioned. But now I think that it's orthogonal. We should show "partitioned" where it is helpful. Thanks, Amit [1] https://www.postgresql.org/message-id/5474c8b6-04e7-1afc-97b6-adb7471c2c71%40lab.ntt.co.jp -
Re: ToDo: show size of partitioned table
Michael Paquier <michael@paquier.xyz> — 2018-11-19T02:42:16Z
On Sun, Nov 18, 2018 at 11:17:37PM -0300, Alvaro Herrera wrote: > To be certain I'm not going against some old decision, I digged up > Amit's old patches. Turns out he submitted psql's describe.c using the > term "partitioned table" on August 10th [1] and then based on a > discussion where Robert suggested calling these new objects "partition > roots" instead to avoid confusion, it was changed to "table" in the next > submission on August 26th [2]. It seems the right call to have used the > term "table" in many places (rather than "partition roots"), but at > least in psql's \dt it seems extremely useful to show the type as > "partitioned table" instead, because it is one place where the > distinction is clearly useful. +1. > In this thread there have been no contrary votes, so I'm pushing this > part soon. > > [1] https://postgr.es/m/ad16e2f5-fc7c-cc2d-333a-88d4aa446f96@lab.ntt.co.jp > [2] https://postgr.es/m/169708f6-6e5a-18d1-707b-1b323e4a6baf@lab.ntt.co.jp Sorry for degressing, but could you also update \di at the same time so as it shows "partitioned index"? listTables() should be switched to use partitioned tables and partitioned indexes, and permissionsList() has a reference to partitioned tables. While on it, this gives the attached.. -- Michael
-
Re: ToDo: show size of partitioned table
Pavel Stehule <pavel.stehule@gmail.com> — 2018-11-19T08:33:58Z
po 19. 11. 2018 v 3:42 odesílatel Michael Paquier <michael@paquier.xyz> napsal: > On Sun, Nov 18, 2018 at 11:17:37PM -0300, Alvaro Herrera wrote: > > To be certain I'm not going against some old decision, I digged up > > Amit's old patches. Turns out he submitted psql's describe.c using the > > term "partitioned table" on August 10th [1] and then based on a > > discussion where Robert suggested calling these new objects "partition > > roots" instead to avoid confusion, it was changed to "table" in the next > > submission on August 26th [2]. It seems the right call to have used the > > term "table" in many places (rather than "partition roots"), but at > > least in psql's \dt it seems extremely useful to show the type as > > "partitioned table" instead, because it is one place where the > > distinction is clearly useful. > > +1. > > > In this thread there have been no contrary votes, so I'm pushing this > > part soon. > > > > [1] > https://postgr.es/m/ad16e2f5-fc7c-cc2d-333a-88d4aa446f96@lab.ntt.co.jp > > [2] > https://postgr.es/m/169708f6-6e5a-18d1-707b-1b323e4a6baf@lab.ntt.co.jp > > Sorry for degressing, but could you also update \di at the same time so > as it shows "partitioned index"? listTables() should be switched to use > partitioned tables and partitioned indexes, and permissionsList() has a > reference to partitioned tables. While on it, this gives the attached.. > It has sense Pavel -- > Michael >
-
Re: ToDo: show size of partitioned table
Michael Paquier <michael@paquier.xyz> — 2018-11-20T01:25:22Z
On Mon, Nov 19, 2018 at 09:33:58AM +0100, Pavel Stehule wrote: > po 19. 11. 2018 v 3:42 odesílatel Michael Paquier <michael@paquier.xyz> > napsal: >> Sorry for degressing, but could you also update \di at the same time so >> as it shows "partitioned index"? listTables() should be switched to use >> partitioned tables and partitioned indexes, and permissionsList() has a >> reference to partitioned tables. While on it, this gives the attached.. >> > > It has sense For the archive's sake, d56e0fde has been committed for this purpose. -- Michael
-
Re: ToDo: show size of partitioned table
Michael Paquier <michael@paquier.xyz> — 2018-11-20T07:50:31Z
On Mon, Nov 05, 2018 at 11:43:16AM +0100, Pavel Stehule wrote: > should be fixed now. Here are some notes on the last version. + " FROM pg_inherits i\n" Missing schema qualification. + case 'P': + if (cmd[2] == 'i') + success = listPartitions(pattern, show_verbose, true, false); + else if (cmd[2] == 't') + success = listPartitions(pattern, show_verbose, false, true); + else if (cmd[2] == '+' || cmd[2] == '\0') + success = listPartitions(pattern, show_verbose, false, false); + else + status = PSQL_CMD_UNKNOWN; + break; The style is heavy. Perhaps it would be cleaner to have a switch/case.. Not a big deal visibly. show_indexes is true only if the subcommand is 'i'. show_tables is true only if the subcommand is 't'. Using "\dP" with a pattern matching a partitioned index should show a partitioned index, no? As far as I know, a partitioned relation can be either an index or a table. Testing the feature, \dP shows all partitioned relations, still does not show the relationship when multiple levels are used. Could it make sense to also show the direct parent of a partitioned table when verbose mode is used? Could it be possible to have tests for \dP, \dPi and \dPt with matching patterns? You could just place that in one of the existing tests where there are partitioned tables and indexes. -- Michael
-
Re: ToDo: show size of partitioned table
Amit Langote <langote_amit_f8@lab.ntt.co.jp> — 2018-11-20T08:14:03Z
On 2018/11/20 16:50, Michael Paquier wrote: > Testing the feature, \dP shows all partitioned relations, still does not > show the relationship when multiple levels are used. Could it make > sense to also show the direct parent of a partitioned table when > verbose mode is used? Yeah. I think it would make sense for \dP output to have an additional column(s) for partitioning-specific information if we're building a special command for partitioning anyway. Thanks, Amit
-
Re: ToDo: show size of partitioned table
Pavel Stehule <pavel.stehule@gmail.com> — 2018-11-21T15:36:05Z
út 20. 11. 2018 v 9:14 odesílatel Amit Langote < Langote_Amit_f8@lab.ntt.co.jp> napsal: > On 2018/11/20 16:50, Michael Paquier wrote: > > Testing the feature, \dP shows all partitioned relations, still does not > > show the relationship when multiple levels are used. Could it make > > sense to also show the direct parent of a partitioned table when > > verbose mode is used? > > Yeah. I think it would make sense for \dP output to have an additional > column(s) for partitioning-specific information if we're building a > special command for partitioning anyway. > I can imagine to count columns, or levels columns Regards Pavel > > Thanks, > Amit > >
-
Re: ToDo: show size of partitioned table
Pavel Stehule <pavel.stehule@gmail.com> — 2018-11-21T16:09:39Z
út 20. 11. 2018 v 8:50 odesílatel Michael Paquier <michael@paquier.xyz> napsal: > On Mon, Nov 05, 2018 at 11:43:16AM +0100, Pavel Stehule wrote: > > should be fixed now. > > Here are some notes on the last version. > > + " FROM pg_inherits i\n" > Missing schema qualification. > > fixed > + case 'P': > + if (cmd[2] == 'i') > + success = listPartitions(pattern, show_verbose, > true, false); > + else if (cmd[2] == 't') > + success = listPartitions(pattern, show_verbose, > false, true); > + else if (cmd[2] == '+' || cmd[2] == '\0') > + success = listPartitions(pattern, show_verbose, > false, false); > + else > + status = PSQL_CMD_UNKNOWN; > + break; > The style is heavy. Perhaps it would be cleaner to have a > switch/case.. Not a big deal visibly. show_indexes is true only if the > subcommand is 'i'. show_tables is true only if the subcommand is 't'. > > Using "\dP" with a pattern matching a partitioned index should show a > partitioned index, no? As far as I know, a partitioned relation can be > either an index or a table. > I don't think \dP shows uses pg_total_relation_size as size function, and then we should to display just tables, but with total size. I don't see a sense to show indexes and tables too, more when we show total relation size - see description for total relation size "total disk space usage for the specified table and associated indexes" > > Testing the feature, \dP shows all partitioned relations, still does not > show the relationship when multiple levels are used. Could it make > sense to also show the direct parent of a partitioned table when > verbose mode is used? > it is expected - you got one number for one partitioned table. I agree, so can be interesting to see agregated sizes per partitioning hierarchy - but in this moment I cannot to imagine form of the result. any table can have different number of levels - so you can get different number of values. > > Could it be possible to have tests for \dP, \dPi and \dPt with matching > patterns? You could just place that in one of the existing tests where > there are partitioned tables and indexes. > I did it see assigned patch, please. Regards Pavel > -- > Michael >
-
Re: ToDo: show size of partitioned table
Alvaro Herrera <alvherre@2ndquadrant.com> — 2018-11-21T16:21:36Z
Hmm, these tests are not going to work, because they have "pavel" in the expected output. -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
-
Re: ToDo: show size of partitioned table
Pavel Stehule <pavel.stehule@gmail.com> — 2018-11-21T16:37:33Z
st 21. 11. 2018 v 17:21 odesílatel Alvaro Herrera <alvherre@2ndquadrant.com> napsal: > Hmm, these tests are not going to work, because they have "pavel" in the > expected output. > I was blind, thank you for check fixed Regards Pavel > > -- > Álvaro Herrera https://www.2ndQuadrant.com/ > PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services >
-
Re: ToDo: show size of partitioned table
Michael Paquier <michael@paquier.xyz> — 2018-11-22T00:51:09Z
On Wed, Nov 21, 2018 at 05:37:33PM +0100, Pavel Stehule wrote: > st 21. 11. 2018 v 17:21 odesílatel Alvaro Herrera <alvherre@2ndquadrant.com> > napsal: >> Hmm, these tests are not going to work, because they have "pavel" in the >> expected output. > > I was blind, thank you for check +create table testtable_apple(logdate date); +create table testtable_orange(logdate date); +create index testtable_apple_index on testtable_apple(logdate); +create index testtable_orange_index on testtable_orange(logdate); There are already a bunch of partition relations with multiple levels created as part of the regression tests, so instead of creating more of those, I would suggest to test \dP and \dPt in create_table.sql, and \dPi in indexing.sql (please make sure to add tests for \dP with partitioned indexes as well). I think that you should really add the direct parent of a partition in at least the verbose output, now for multiple partition levels things are confusing in my opinion. For example with such a schema: CREATE TABLE parent_tab (id int) PARTITION BY RANGE (id); CREATE INDEX parent_index ON parent_tab (id); CREATE TABLE child_0_10 PARTITION OF parent_tab FOR VALUES FROM (0) TO (10); CREATE TABLE child_10_20 PARTITION OF parent_tab FOR VALUES FROM (10) TO (20); CREATE TABLE child_20_30 PARTITION OF parent_tab FOR VALUES FROM (20) TO (30); INSERT INTO parent_tab VALUES (generate_series(0,29)); CREATE TABLE child_30_40 PARTITION OF parent_tab FOR VALUES FROM (30) TO (40) PARTITION BY RANGE(id); CREATE TABLE child_30_35 PARTITION OF child_30_40 FOR VALUES FROM (30) TO (35); CREATE TABLE child_35_40 PARTITION OF child_30_40 FOR VALUES FROM (35) TO (40); INSERT INTO parent_tab VALUES (generate_series(30,39)); Then with \dP+ I got that: =# \dP+ List of partitioned relations Schema | Name | Owner | Size | Description --------+-------------+--------+--------+------------- public | child_30_40 | ioltas | 48 kB | public | parent_tab | ioltas | 120 kB | (2 rows) Showing the parent partition looks like a pretty important to me as I would expect multi-level partitions to be a frequent case (perhaps it should show up as well in the non-verbose output?). The field should be NULL if the relation is the top of the tree. Again, with the previous schema: =# \dPi *idx List of partitioned indexes Schema | Name | Owner | Table --------+--------------------+--------+------------- public | child_30_40_id_idx | ioltas | child_30_40 (1 row) =# \dP *idx Did not find any partitioned relations named "*idx" I would have expected in the second case to have the partitioned *relations* showing up in the output, and a relation can be an index as well if the pattern matches. Could you please address those problems first? The basic shape of the patch with the three new sub-commands is fine I think, so we can go ahead with that, but the two problems reported are blockers in my opinion. -- Michael -
Re: ToDo: show size of partitioned table
Pavel Stehule <pavel.stehule@gmail.com> — 2018-11-22T11:42:14Z
čt 22. 11. 2018 v 1:51 odesílatel Michael Paquier <michael@paquier.xyz> napsal: > On Wed, Nov 21, 2018 at 05:37:33PM +0100, Pavel Stehule wrote: > > st 21. 11. 2018 v 17:21 odesílatel Alvaro Herrera < > alvherre@2ndquadrant.com> > > napsal: > >> Hmm, these tests are not going to work, because they have "pavel" in the > >> expected output. > > > > I was blind, thank you for check > > +create table testtable_apple(logdate date); > +create table testtable_orange(logdate date); > +create index testtable_apple_index on testtable_apple(logdate); > +create index testtable_orange_index on testtable_orange(logdate); > There are already a bunch of partition relations with multiple levels > created as part of the regression tests, so instead of creating more of > those, I would suggest to test \dP and \dPt in create_table.sql, and > \dPi in indexing.sql (please make sure to add tests for \dP with > partitioned indexes as well). > > I think that you should really add the direct parent of a partition in > at least the verbose output, now for multiple partition levels things > are confusing in my opinion. For example with such a schema: > CREATE TABLE parent_tab (id int) PARTITION BY RANGE (id); > CREATE INDEX parent_index ON parent_tab (id); > CREATE TABLE child_0_10 PARTITION OF parent_tab > FOR VALUES FROM (0) TO (10); > CREATE TABLE child_10_20 PARTITION OF parent_tab > FOR VALUES FROM (10) TO (20); > CREATE TABLE child_20_30 PARTITION OF parent_tab > FOR VALUES FROM (20) TO (30); > INSERT INTO parent_tab VALUES (generate_series(0,29)); > CREATE TABLE child_30_40 PARTITION OF parent_tab > FOR VALUES FROM (30) TO (40) > PARTITION BY RANGE(id); > CREATE TABLE child_30_35 PARTITION OF child_30_40 > FOR VALUES FROM (30) TO (35); > CREATE TABLE child_35_40 PARTITION OF child_30_40 > FOR VALUES FROM (35) TO (40); > INSERT INTO parent_tab VALUES (generate_series(30,39)); > > Then with \dP+ I got that: > =# \dP+ > List of partitioned relations > Schema | Name | Owner | Size | Description > --------+-------------+--------+--------+------------- > public | child_30_40 | ioltas | 48 kB | > public | parent_tab | ioltas | 120 kB | > (2 rows) > Showing the parent partition looks like a pretty important to me as I > would expect multi-level partitions to be a frequent case (perhaps it > should show up as well in the non-verbose output?). The field should be > NULL if the relation is the top of the tree. > > it looks like bug for me much more. your example - on my comp List of relations +--------+-------------+-------------------+-------+------------+-------------+ | Schema | Name | Type | Owner | Size | Description | +--------+-------------+-------------------+-------+------------+-------------+ | public | child_0_10 | table | pavel | 8192 bytes | | | public | child_10_20 | table | pavel | 8192 bytes | | | public | child_20_30 | table | pavel | 8192 bytes | | | public | child_30_35 | table | pavel | 8192 bytes | | | public | child_30_40 | partitioned table | pavel | 0 bytes | | | public | child_35_40 | table | pavel | 8192 bytes | | | public | parent_tab | partitioned table | pavel | 0 bytes | | +--------+-------------+-------------------+-------+------------+-------------+ (7 rows) there is about 5x 8KB data .. 40KB But in views I got List of partitioned tables +--------+-------------+-------+-------+-------------+ | Schema | Name | Owner | Size | Description | +--------+-------------+-------+-------+-------------+ | public | child_30_40 | pavel | 16 kB | | | public | parent_tab | pavel | 40 kB | | +--------+-------------+-------+-------+-------------+ (2 rows) there is 16KB more, what is really messy. I think so most correct is removing child_30_40 from the report. test=# SELECT n.nspname as "Schema", c.relname as "Name", pg_catalog.pg_get_userbyid(c.relowner) as "Owner", (SELECT pg_catalog.pg_size_pretty(sum(pg_catalog.pg_table_size(relid))) FROM pg_catalog.pg_partition_tree(c.oid)) AS "Size", pg_catalog.obj_description(c.oid, 'pg_class') as "Description" FROM pg_catalog.pg_class c LEFT JOIN pg_catalog.pg_namespace n ON n.oid = c.relnamespace WHERE c.relkind IN ('p') and not c.relispartition AND n.nspname <> 'pg_catalog' AND n.nspname <> 'information_schema' AND n.nspname !~ '^pg_toast' AND pg_catalog.pg_table_is_visible(c.oid) ORDER BY 1,2; +--------+------------+-------+-------+-------------+ | Schema | Name | Owner | Size | Description | +--------+------------+-------+-------+-------------+ | public | parent_tab | pavel | 40 kB | | +--------+------------+-------+-------+-------------+ (1 row) I afraid of unreadable result if we allow overlap in report. I think so can be strange if some disk space will be reported 2x or more times in one report. Unfortunately It means so some information will be hidden. In this moment I prefer readability and simple meaning. I am not strong in this topics. Another possibility is show parent (this should be displayed every time, without it it is messy). This query is much more complex, but the result is more informative SELECT n.nspname as "Schema", c.relname as "Name", n2.nspname as "Parent schema", c2.relname as "Parent name", pg_catalog.pg_get_userbyid(c.relowner) as "Owner", s.max as "Hiearchy deep", s.size as "Size", pg_catalog.obj_description(c.oid, 'pg_class') as "Description" FROM pg_catalog.pg_class c LEFT JOIN pg_catalog.pg_namespace n ON n.oid = c.relnamespace LEFT JOIN pg_catalog.pg_inherits i ON c.oid = i.inhrelid LEFT JOIN pg_catalog.pg_class c2 ON c2.oid = i.inhparent LEFT JOIN pg_catalog.pg_namespace n2 ON n2.oid = c2.relnamespace, LATERAL (SELECT max(level), pg_catalog.pg_size_pretty(sum(pg_catalog.pg_table_size(relid))) as size FROM pg_catalog.pg_partition_tree(c.oid)) s WHERE c.relkind IN ('p') AND n.nspname <> 'pg_catalog' AND n.nspname <> 'information_schema' AND n.nspname !~ '^pg_toast' AND pg_catalog.pg_table_is_visible(c.oid) ORDER BY 1,2; +--------+-------------+---------------+-------------+-------+---------------+-------+-------------+ | Schema | Name | Parent schema | Parent name | Owner | Hiearchy deep | Size | Description | +--------+-------------+---------------+-------------+-------+---------------+-------+-------------+ | public | child_30_40 | public | parent_tab | pavel | 1 | 16 kB | | | public | parent_tab | | | pavel | 2 | 40 kB | | +--------+-------------+---------------+-------------+-------+---------------+-------+-------------+ (2 rows) Still I prefer to not show nested partitioned tables for simplicity, readability reasons. Displaying nested objects in one table doesn't look like good idea for me. But I am ready to accept different common opinion. Still do you think so variant with parent should be preferred? > Again, with the previous schema: > =# \dPi *idx > List of partitioned indexes > Schema | Name | Owner | Table > --------+--------------------+--------+------------- > public | child_30_40_id_idx | ioltas | child_30_40 > (1 row) > =# \dP *idx > Did not find any partitioned relations named "*idx" > I would have expected in the second case to have the partitioned > *relations* showing up in the output, and a relation can be an index as > well if the pattern matches. > I think so it is correct - I don't would to see the index here, because index size is calculated by total_relation_size already. Here my position is strong. \dP for me doesn't mean "tables or indexes" - it means "partition tables with total relation size". I don't see any sense to show tables and indexes in one report. Regards Pavel > Could you please address those problems first? The basic shape of the > patch with the three new sub-commands is fine I think, so we can go > ahead with that, but the two problems reported are blockers in my > opinion. > -- > Michael > -
Re: ToDo: show size of partitioned table
Michael Paquier <michael@paquier.xyz> — 2018-11-22T14:29:11Z
On Thu, Nov 22, 2018 at 12:42:14PM +0100, Pavel Stehule wrote: > Here my position is strong. \dP for me doesn't mean "tables or > indexes" - it means "partition tables with total relation size". I > don't see any sense to show tables and indexes in one report. Please let me disagree on that point. \dP, \dPt and \dPi are commands able to show information about respectively partitioned relations, partitioned tables and partitioned indexes, which is not something only related to the size of those partitions. Showing only the level of a relation in its hierarchy may be useful, but that's confusing for the user without knowing its direct parent or its top-most parent. For multiple levels, the direct parent without the number in the hierarchy seems enough to me. I may be of course wrong in designing those concepts. -- Michael
-
Re: ToDo: show size of partitioned table
Pavel Stehule <pavel.stehule@gmail.com> — 2018-11-22T14:47:11Z
čt 22. 11. 2018 v 15:29 odesílatel Michael Paquier <michael@paquier.xyz> napsal: > On Thu, Nov 22, 2018 at 12:42:14PM +0100, Pavel Stehule wrote: > > Here my position is strong. \dP for me doesn't mean "tables or > > indexes" - it means "partition tables with total relation size". I > > don't see any sense to show tables and indexes in one report. > > Please let me disagree on that point. \dP, \dPt and \dPi are commands > able to show information about respectively partitioned relations, > partitioned tables and partitioned indexes, which is not something only > related to the size of those partitions. Showing only the level of a > relation in its hierarchy may be useful, but that's confusing for the > user without knowing its direct parent or its top-most parent. For > multiple levels, the direct parent without the number in the hierarchy > seems enough to me. I may be of course wrong in designing those > concepts. > There are open two points: 1. display hierarchy of partitioned structures. 2. what should be displayed by \dP command. @1 I agree so this information can be interesting and useful. But I have a problem with consistency of this report. When result is table, then I think so we can introduce, and should to introduce some new special report for command - maybe \dPh that can show hiearchy of one partitioned table (the table name should be required) I think so can be much more readable to have special report like \dPh parent_tab parent_tab -> direct partitions 24kB -> child_30_40 -> direct partitions 16kB This is some what i can read, and I see (very naturally) the hierarchy of partitions and the relations between I have not feel well when I see in one report numbers 40 and 16, I see much more comfortable when I see 24 and 16, but for this I need a different perspective What do you think about it? > -- > Michael > -
Re: ToDo: show size of partitioned table
Michael Paquier <michael@paquier.xyz> — 2018-11-29T06:58:51Z
On Thu, Nov 22, 2018 at 03:47:11PM +0100, Pavel Stehule wrote: > I have not feel well when I see in one report numbers 40 and 16, I see much > more comfortable when I see 24 and 16, but for this I need a different > perspective > > What do you think about it? Maybe, my thought is that this would be a separate patch, and that we had better keep it simple for now because that's complicated enough :) So I would rather have a wrapper on top of pg_partition_tree which is useful for the user with matching patterns, which shows roughly: - the size of the whole partition tree from the root. - its direct parent if any. Potentially in the verbose output. This depends on how much users have multi-level partitions. My bet would be not that much, but more input from others is welcome. - perhaps its level in the hierarchy, if integrated most likely as part of the verbose output. Then with the set of commands, have a mapping close to how \d treats indexes and relations: - \dp shows information about partitioned tables or indexes, if no pattern is defined tables show up. If a partitioned index pattern shows up, then index information is displayed. - \dpi and \dpt for respectively partitioned indexes and tables. -- Michael
-
Re: ToDo: show size of partitioned table
Pavel Stehule <pavel.stehule@gmail.com> — 2018-11-30T19:06:47Z
čt 29. 11. 2018 v 7:58 odesílatel Michael Paquier <michael@paquier.xyz> napsal: > On Thu, Nov 22, 2018 at 03:47:11PM +0100, Pavel Stehule wrote: > > I have not feel well when I see in one report numbers 40 and 16, I see > much > > more comfortable when I see 24 and 16, but for this I need a different > > perspective > > > > What do you think about it? > > Maybe, my thought is that this would be a separate patch, and that we > had better keep it simple for now because that's complicated enough :) > any time, the behave should be consistent. I agree, so there can be another patch, that reports more about partitioned table for \d+ > So I would rather have a wrapper on top of pg_partition_tree which is > useful for the user with matching patterns, which shows roughly: > - the size of the whole partition tree from the root. > - its direct parent if any. Potentially in the verbose output. This > depends on how much users have multi-level partitions. My bet would be > not that much, but more input from others is welcome. > - perhaps its level in the hierarchy, if integrated most likely as part > of the verbose output. > > Then with the set of commands, have a mapping close to how \d treats > indexes and relations: > - \dp shows information about partitioned tables or indexes, if no pattern > is defined tables show up. If a partitioned index pattern shows up, > then index information is displayed. > ok - \dpi and \dpt for respectively partitioned indexes and tables. > -- > Michael >
-
Re: ToDo: show size of partitioned table
Pavel Stehule <pavel.stehule@gmail.com> — 2018-12-17T08:48:07Z
Hi pá 30. 11. 2018 v 20:06 odesílatel Pavel Stehule <pavel.stehule@gmail.com> napsal: > > > čt 29. 11. 2018 v 7:58 odesílatel Michael Paquier <michael@paquier.xyz> > napsal: > >> On Thu, Nov 22, 2018 at 03:47:11PM +0100, Pavel Stehule wrote: >> > I have not feel well when I see in one report numbers 40 and 16, I see >> much >> > more comfortable when I see 24 and 16, but for this I need a different >> > perspective >> > >> > What do you think about it? >> >> Maybe, my thought is that this would be a separate patch, and that we >> had better keep it simple for now because that's complicated enough :) >> > > any time, the behave should be consistent. > > I agree, so there can be another patch, that reports more about > partitioned table for \d+ > > >> So I would rather have a wrapper on top of pg_partition_tree which is >> useful for the user with matching patterns, which shows roughly: >> - the size of the whole partition tree from the root. >> - its direct parent if any. Potentially in the verbose output. This >> depends on how much users have multi-level partitions. My bet would be >> not that much, but more input from others is welcome. >> - perhaps its level in the hierarchy, if integrated most likely as part >> of the verbose output. >> >> Then with the set of commands, have a mapping close to how \d treats >> indexes and relations: >> - \dp shows information about partitioned tables or indexes, if no pattern >> is defined tables show up. If a partitioned index pattern shows up, >> then index information is displayed. >> > > ok > > - \dpi and \dpt for respectively partitioned indexes and tables. >> > new update of this patch changes: 1. only root partitioned tables are displayed - you can see quickly total allocated space. It is not duplicated due nested partitions. 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 2. \dP without pattern shows root partitioned tables + total relation size. When pattern is defined, then shows tables and indexes + table size postgres=# \dP+ List of partitioned relations ┌────────┬────────────┬───────┬────────┬─────────────┐ │ Schema │ Name │ Owner │ Size │ Description │ ╞════════╪════════════╪═══════╪════════╪═════════════╡ │ public │ parent_tab │ pavel │ 120 kB │ │ └────────┴────────────┴───────┴────────┴─────────────┘ (1 row) postgres=# \dP+ * List of partitioned relations or indexes ┌────────┬──────────────┬───────┬───────────────────┬────────────┬───────┬─────────────┐ │ Schema │ Name │ Owner │ Type │ Table │ Size │ Description │ ╞════════╪══════════════╪═══════╪═══════════════════╪════════════╪═══════╪═════════════╡ │ public │ parent_index │ pavel │ partitioned index │ parent_tab │ 80 kB │ │ │ public │ parent_tab │ pavel │ partitioned table │ │ 40 kB │ │ └────────┴──────────────┴───────┴───────────────────┴────────────┴───────┴─────────────┘ (2 rows) postgres=# \dP+ *index List of partitioned relations or indexes ┌────────┬──────────────┬───────┬───────────────────┬────────────┬───────┬─────────────┐ │ Schema │ Name │ Owner │ Type │ Table │ Size │ Description │ ╞════════╪══════════════╪═══════╪═══════════════════╪════════════╪═══════╪═════════════╡ │ public │ parent_index │ pavel │ partitioned index │ parent_tab │ 80 kB │ │ └────────┴──────────────┴───────┴───────────────────┴────────────┴───────┴─────────────┘ (1 row) Regards Pavel > -- >> Michael >> > -
Re: ToDo: show size of partitioned table
Amit Langote <langote_amit_f8@lab.ntt.co.jp> — 2018-12-18T07:49:07Z
Hi, Thank you for updating the patch. On 2018/12/17 17:48, Pavel Stehule wrote: > new update of this patch Documentation portion of this patch still contains some typos that I mentioned before here: https://www.postgresql.org/message-id/1c83bb5c-47cd-d796-226c-e95795b05551%40lab.ntt.co.jp + .. If the form <literal>\dP+</literal> + is used, the sum of size of related partitions (including the + table and indexes, if any) and a description + are also displayed. + ... If the form <literal>\dPi+</literal> + is used, the sum of size of related indexes and a description + are also displayed. + ... If the form <literal>\dPt+</literal> + is used, the sum of size of related tables and a description + are also displayed. In all of the three hunks: the sum of size of -> the sum of "sizes" of and a description -> and associated description > changes: > > 1. only root partitioned tables are displayed - you can see quickly total > allocated space. It is not duplicated due nested partitions. +1 If one wants to see a non-root partitioned table's details, they can use \dP+ <pattern>. > 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? > 2. \dP without pattern shows root partitioned tables + total relation size. > When pattern is defined, then shows tables and indexes + table size > > postgres=# \dP+ > List of partitioned relations > ┌────────┬────────────┬───────┬────────┬─────────────┐ > │ Schema │ Name │ Owner │ Size │ Description │ > ╞════════╪════════════╪═══════╪════════╪═════════════╡ > │ public │ parent_tab │ pavel │ 120 kB │ │ > └────────┴────────────┴───────┴────────┴─────────────┘ > (1 row) > > postgres=# \dP+ * > List of partitioned relations or indexes > ┌────────┬──────────────┬───────┬───────────────────┬────────────┬───────┬─────────────┐ > │ Schema │ Name │ Owner │ Type │ Table │ Size │ > Description │ > ╞════════╪══════════════╪═══════╪═══════════════════╪════════════╪═══════╪═════════════╡ > │ public │ parent_index │ pavel │ partitioned index │ parent_tab │ 80 kB > │ │ > │ public │ parent_tab │ pavel │ partitioned table │ │ 40 kB > │ │ > └────────┴──────────────┴───────┴───────────────────┴────────────┴───────┴─────────────┘ > (2 rows) > > postgres=# \dP+ *index > List of partitioned relations or indexes > ┌────────┬──────────────┬───────┬───────────────────┬────────────┬───────┬─────────────┐ > │ Schema │ Name │ Owner │ Type │ Table │ Size │ > Description │ > ╞════════╪══════════════╪═══════╪═══════════════════╪════════════╪═══════╪═════════════╡ > │ public │ parent_index │ pavel │ partitioned index │ parent_tab │ 80 kB > │ │ > └────────┴──────────────┴───────┴───────────────────┴────────────┴───────┴─────────────┘ > (1 row) Looking at the patch: + if (pattern) + /* translator: objects_name is "indexes", "tables" or "relations" */ + psql_error("Did not find any partitioned %s named \"%s\".\n", + objects_name, + pattern); + else + /* translator: object_name is "index", "table" or "relation" */ + psql_error("Did not find any partitioned %s.\n", + object_name); It seems that objects_name and object_name need to be swapped between the if and else blocks, and so do /* translator: ... */ comments. if (pattern) /* translator: object_name is "index", "table" or "relation" */ psql_error(..., object_name); else /* translator: objects_name is "indexes", "tables" or "relations" */ psql_error(..., objects_name); That is, it should say, "Did not find any partitioned index/table/relation named "foo" and "Did not find any partitioned indexes/tables/relations". Thanks, Amit -
Re: ToDo: show size of partitioned table
Pavel Stehule <pavel.stehule@gmail.com> — 2018-12-19T06:38:16Z
út 18. 12. 2018 v 8:49 odesílatel Amit Langote < Langote_Amit_f8@lab.ntt.co.jp> napsal: > Hi, > > Thank you for updating the patch. > > On 2018/12/17 17:48, Pavel Stehule wrote: > > new update of this patch > > Documentation portion of this patch still contains some typos that I > mentioned before here: > > > https://www.postgresql.org/message-id/1c83bb5c-47cd-d796-226c-e95795b05551%40lab.ntt.co.jp > > + .. If the form <literal>\dP+</literal> > + is used, the sum of size of related partitions (including the > + table and indexes, if any) and a description > + are also displayed. > > + ... If the form <literal>\dPi+</literal> > + is used, the sum of size of related indexes and a description > + are also displayed. > > + ... If the form <literal>\dPt+</literal> > + is used, the sum of size of related tables and a description > + are also displayed. > > In all of the three hunks: > > the sum of size of -> the sum of "sizes" of > > and a description -> and associated description > fixed > > > changes: > > > > 1. only root partitioned tables are displayed - you can see quickly total > > allocated space. It is not duplicated due nested partitions. > > +1 > > If one wants to see a non-root partitioned table's details, they can use > \dP+ <pattern>. > > > 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 postgres=# \dP+ List of partitioned relations ┌────────┬────────────┬───────┬────────┬─────────────┐ │ Schema │ Name │ Owner │ Size │ Description │ ╞════════╪════════════╪═══════╪════════╪═════════════╡ │ public │ parent_tab │ pavel │ 120 kB │ │ └────────┴────────────┴───────┴────────┴─────────────┘ (1 row) postgres=# \dPn+ List of partitioned relations ┌────────┬─────────────┬───────┬─────────────┬───────┬─────────────┐ │ Schema │ Name │ Owner │ Parent name │ Size │ Description │ ╞════════╪═════════════╪═══════╪═════════════╪═══════╪═════════════╡ │ public │ child_30_40 │ pavel │ parent_tab │ 48 kB │ │ │ public │ parent_tab │ pavel │ │ 72 kB │ │ └────────┴─────────────┴───────┴─────────────┴───────┴─────────────┘ (2 rows) postgres=# \dPn+ * List of partitioned relations or indexes ┌────────┬────────────────────┬───────┬───────────────────┬──────────────┬─────────────┬───────┬─────────────┐ │ Schema │ Name │ Owner │ Type │ Parent name │ On table │ Size │ Description │ ╞════════╪════════════════════╪═══════╪═══════════════════╪══════════════╪═════════════╪═══════╪═════════════╡ │ public │ child_30_40 │ pavel │ partitioned table │ parent_tab │ │ 16 kB │ │ │ public │ child_30_40_id_idx │ pavel │ partitioned index │ parent_index │ child_30_40 │ 32 kB │ │ │ public │ parent_index │ pavel │ partitioned index │ │ parent_tab │ 48 kB │ │ │ public │ parent_tab │ pavel │ partitioned table │ │ │ 24 kB │ │ └────────┴────────────────────┴───────┴───────────────────┴──────────────┴─────────────┴───────┴─────────────┘ (4 rows) > > 2. \dP without pattern shows root partitioned tables + total relation > size. > > When pattern is defined, then shows tables and indexes + table size > > > > postgres=# \dP+ > > List of partitioned relations > > ┌────────┬────────────┬───────┬────────┬─────────────┐ > > │ Schema │ Name │ Owner │ Size │ Description │ > > ╞════════╪════════════╪═══════╪════════╪═════════════╡ > > │ public │ parent_tab │ pavel │ 120 kB │ │ > > └────────┴────────────┴───────┴────────┴─────────────┘ > > (1 row) > > > > postgres=# \dP+ * > > List of partitioned relations or indexes > > > ┌────────┬──────────────┬───────┬───────────────────┬────────────┬───────┬─────────────┐ > > │ Schema │ Name │ Owner │ Type │ Table │ Size > │ > > Description │ > > > ╞════════╪══════════════╪═══════╪═══════════════════╪════════════╪═══════╪═════════════╡ > > │ public │ parent_index │ pavel │ partitioned index │ parent_tab │ 80 kB > > │ │ > > │ public │ parent_tab │ pavel │ partitioned table │ │ 40 kB > > │ │ > > > └────────┴──────────────┴───────┴───────────────────┴────────────┴───────┴─────────────┘ > > (2 rows) > > > > postgres=# \dP+ *index > > List of partitioned relations or indexes > > > ┌────────┬──────────────┬───────┬───────────────────┬────────────┬───────┬─────────────┐ > > │ Schema │ Name │ Owner │ Type │ Table │ Size > │ > > Description │ > > > ╞════════╪══════════════╪═══════╪═══════════════════╪════════════╪═══════╪═════════════╡ > > │ public │ parent_index │ pavel │ partitioned index │ parent_tab │ 80 kB > > │ │ > > > └────────┴──────────────┴───────┴───────────────────┴────────────┴───────┴─────────────┘ > > (1 row) > > Looking at the patch: > > + if (pattern) > + /* translator: objects_name is "indexes", "tables" > or "relations" */ > + psql_error("Did not find any partitioned %s named > \"%s\".\n", > + objects_name, > + pattern); > + else > + /* translator: object_name is "index", "table" or > "relation" */ > + psql_error("Did not find any partitioned %s.\n", > + object_name); > > It seems that objects_name and object_name need to be swapped between the > if and else blocks, and so do /* translator: ... */ comments. > > if (pattern) > /* translator: object_name is "index", "table" or "relation" */ > psql_error(..., object_name); > else > /* translator: objects_name is "indexes", "tables" or "relations" */ > psql_error(..., objects_name); > > That is, it should say, "Did not find any partitioned index/table/relation > named "foo" and "Did not find any partitioned indexes/tables/relations". > fixed I am sending updated patch Regards Pavel > > Thanks, > Amit > > -
Re: ToDo: show size of partitioned table
Michael Paquier <michael@paquier.xyz> — 2019-02-04T01:44:24Z
On Wed, Dec 19, 2018 at 07:38:16AM +0100, Pavel Stehule wrote: > I am sending updated patch. Moved to next CF. -- Michael
-
Re: ToDo: show size of partitioned table
Amit Langote <langote_amit_f8@lab.ntt.co.jp> — 2019-02-07T08:50:51Z
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 -
Re: ToDo: show size of partitioned table
Pavel Stehule <pavel.stehule@gmail.com> — 2019-02-07T09:08:35Z
čt 7. 2. 2019 v 9:51 odesílatel Amit Langote <Langote_Amit_f8@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 > > -
Re: ToDo: show size of partitioned table
Amit Langote <langote_amit_f8@lab.ntt.co.jp> — 2019-02-07T10:24:55Z
Hi, On 2019/02/07 18:08, Pavel Stehule wrote: > čt 7. 2. 2019 v 9:51 odesílatel Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> >> \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 (correcting words of my previous email: ... of level 1.) > 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. Just to recap, the originally proposed feature is to show the size of a partitioned table by summing the sizes of *all* of its (actually leaf) partitions, which \dP[t|i]+ gives us. As you mentioned, a limitation of the feature as initially proposed is that it only shows partitioned tables that are roots of their respective partition trees. That is, there is no way to see the sizes of the intermediate partitioned tables using any of psql's \d commands. So, you introduced the "n" modifier, whereby \dP[t|i]n+ now shows *also* the intermediate partitioned tables with their sizes. But it only considers the directly attached partitions of each partitioned table that's shown. So, only those partitioned tables that have leaf partitions directly attached them are shown with non-0 size (if leaf partitions are non-empty) and others with size 0 (root partitioned tables in most cases where nested partitioned tables are involved). But I think that means the "n" modifier is changing the behavior of the base command (\dP+) which is meant to show the total size of *all* partitions under a given partitioned table. Maybe, the "n" modifier should only result in including the nested/intermediate partitioned tables and nothing more than that. I see your point that all these tables are appearing in the display as one flat list and so the sizes of same leaf partitions may be multiply counted, but it's not totally a flat representation given that you have added "Parent name" column. We could document that the size of a nested partitioned table shown in the display is also counted in the size of its parent partitioned table. That I think may be easier to understand than that the size of each partitioned table shown in the display only considers the sizes of leaf partitions that are directly attached to it. Thoughts? Any more opinions on this? Thanks, Amit
-
Re: ToDo: show size of partitioned table
Pavel Stehule <pavel.stehule@gmail.com> — 2019-02-07T10:54:46Z
čt 7. 2. 2019 v 11:25 odesílatel Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> napsal: > Hi, > > On 2019/02/07 18:08, Pavel Stehule wrote: > > čt 7. 2. 2019 v 9:51 odesílatel Amit Langote < > Langote_Amit_f8@lab.ntt.co.jp> > >> \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 > > (correcting words of my previous email: ... of level 1.) > > > 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. > > Just to recap, the originally proposed feature is to show the size of a > partitioned table by summing the sizes of *all* of its (actually leaf) > partitions, which \dP[t|i]+ gives us. As you mentioned, a limitation of > the feature as initially proposed is that it only shows partitioned tables > that are roots of their respective partition trees. That is, there is no > way to see the sizes of the intermediate partitioned tables using any of > psql's \d commands. So, you introduced the "n" modifier, whereby > \dP[t|i]n+ now shows *also* the intermediate partitioned tables with their > sizes. But it only considers the directly attached partitions of each > partitioned table that's shown. So, only those partitioned tables that > have leaf partitions directly attached them are shown with non-0 size (if > leaf partitions are non-empty) and others with size 0 (root partitioned > tables in most cases where nested partitioned tables are involved). But I > think that means the "n" modifier is changing the behavior of the base > command (\dP+) which is meant to show the total size of *all* partitions > under a given partitioned table. Maybe, the "n" modifier should only > result in including the nested/intermediate partitioned tables and nothing > more than that. > It was a Michael's request to see all hierarchy, and I think so it has some benefits > I see your point that all these tables are appearing in the display as one > flat list and so the sizes of same leaf partitions may be multiply > counted, but it's not totally a flat representation given that you have > added "Parent name" column. We could document that the size of a nested > partitioned table shown in the display is also counted in the size of its > parent partitioned table. That I think may be easier to understand than > that the size of each partitioned table shown in the display only > considers the sizes of leaf partitions that are directly attached to it. > > Personally I don't agree - a) who read a documentation, b) it is really violation of some relation principles. It is clean, if we have only one table, but if we see a report with more tables, than multiple size calculation can be messy. It is not problem if you have clean schema like P1, P2, tables (when tree is balanced). But when some tables can be assigned to P1 and some to P2 (tree is not balanced) then it is not clean what is size of directly attached tables and what is size of subpartitions. So it is better don't sum apples and oranges. \dPn shows all subroots and related minimal size. I think so this is very clear definition. Your example postgres=# \dPtn+ List of partitioned tables ┌────────┬────────┬───────┬─────────────┬────────────┬─────────────┐ │ Schema │ Name │ Owner │ Parent name │ Size │ Description │ ╞════════╪════════╪═══════╪═════════════╪════════════╪═════════════╡ │ public │ p │ pavel │ │ 8192 bytes │ │ │ public │ p_1 │ pavel │ p │ 8192 bytes │ │ │ public │ p_1_bc │ pavel │ p_1 │ 8192 bytes │ │ └────────┴────────┴───────┴─────────────┴────────────┴─────────────┘ (3 rows) I hope so the interpretation is clean .. there are three partitioned tables (two are subpartitioned tables). Any partitioned table has assigned 8KB of data. We can introduce new column "size with sub partitions" where these numbers can be counted together. But for term "size" I expect valid rule S1+S2+.. SN = total size. It is acceptable for you? > Thoughts? Any more opinions on this? > > > Thanks, > Amit > > -
Re: ToDo: show size of partitioned table
Alvaro Herrera <alvherre@2ndquadrant.com> — 2019-02-07T15:03:17Z
On 2019-Feb-07, Pavel Stehule wrote: > Your example > > postgres=# \dPtn+ > List of partitioned tables > ┌────────┬────────┬───────┬─────────────┬────────────┬─────────────┐ > │ Schema │ Name │ Owner │ Parent name │ Size │ Description │ > ╞════════╪════════╪═══════╪═════════════╪════════════╪═════════════╡ > │ public │ p │ pavel │ │ 8192 bytes │ │ > │ public │ p_1 │ pavel │ p │ 8192 bytes │ │ > │ public │ p_1_bc │ pavel │ p_1 │ 8192 bytes │ │ > └────────┴────────┴───────┴─────────────┴────────────┴─────────────┘ > (3 rows) > > I hope so the interpretation is clean .. there are three partitioned tables > (two are subpartitioned tables). Any partitioned table has assigned 8KB of > data. > > We can introduce new column "size with sub partitions" where these numbers > can be counted together. But for term "size" I expect valid rule S1+S2+.. > SN = total size. Right now, a partitioned table has no size of its own; its only storage is in its children. But that might change in the future, for example if we implement global indexes. Then it will be useful to show these sizes separately. I suggest that we should have one column for the aggregated size of its children, and another column for the "local" size. So currently the local size would always be zero for partitioned tables. The other column (maybe "Children Size") would be the sum of the sizes of all its partitions. So I think this should be: List of partitioned tables ┌────────┬────────┬───────┬─────────────┬────────────┬───────────────┬──────────── │ Schema │ Name │ Owner │ Parent name │ Size │ Children Size │ Description ╞════════╪════════╪═══════╪═════════════╪════════════╪═══════════════╡ │ public │ p │ pavel │ │ │ 8192 bytes │ │ public │ p_1 │ pavel │ p │ │ 8192 bytes │ │ public │ p_1_bc │ pavel │ p_1 │ 8192 bytes │ │ └────────┴────────┴───────┴─────────────┴────────────┴───────────────┴───────────── -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -
Re: ToDo: show size of partitioned table
Pavel Stehule <pavel.stehule@gmail.com> — 2019-02-07T17:43:25Z
čt 7. 2. 2019 v 16:03 odesílatel Alvaro Herrera <alvherre@2ndquadrant.com> napsal: > On 2019-Feb-07, Pavel Stehule wrote: > > > Your example > > > > postgres=# \dPtn+ > > List of partitioned tables > > ┌────────┬────────┬───────┬─────────────┬────────────┬─────────────┐ > > │ Schema │ Name │ Owner │ Parent name │ Size │ Description │ > > ╞════════╪════════╪═══════╪═════════════╪════════════╪═════════════╡ > > │ public │ p │ pavel │ │ 8192 bytes │ │ > > │ public │ p_1 │ pavel │ p │ 8192 bytes │ │ > > │ public │ p_1_bc │ pavel │ p_1 │ 8192 bytes │ │ > > └────────┴────────┴───────┴─────────────┴────────────┴─────────────┘ > > (3 rows) > > > > I hope so the interpretation is clean .. there are three partitioned > tables > > (two are subpartitioned tables). Any partitioned table has assigned 8KB > of > > data. > > > > We can introduce new column "size with sub partitions" where these > numbers > > can be counted together. But for term "size" I expect valid rule S1+S2+.. > > SN = total size. > > Right now, a partitioned table has no size of its own; its only storage > is in its children. But that might change in the future, for example if > we implement global indexes. Then it will be useful to show these sizes > separately. > > I suggest that we should have one column for the aggregated size of its > children, and another column for the "local" size. So currently the > local size would always be zero for partitioned tables. The other > column (maybe "Children Size") would be the sum of the sizes of all its > partitions. > > So I think this should be: > So this is third proposals :-) Can I recapitulate it? 1. My proposal - the "size" = sum(partitions on level1), "size with nested partitioned tables" = sum(partitions on all levels) 2. Amit's proposal - the "size" = sum(partions on all levels) 3. Alvaro's proposal - the "size" = empty now, "children size" = sum(partitions on level1) Every proposal is valid, and has some sense and display some information from some perspective. So first we should not to use one word term "size" in this context, because there is any agreement. Can we find some terminology for two *sizes*? 1. size of all partitions with nesting level = 1 2. size of all partitions without nesting level check. maybe there can be third variant - size of all partitions with nesting level > 1 I am not sure so "children size" is good term because this structure is tree, and the deep is not specified. Maybe "Immediate partitions size" for @1, "total partitions size" for @2, and "indirect partitions size" for @3. Then we can display @1, @2 or @1 or @3. I hope so I mentioned all possible variants. Comments, notes? Pavel > > List of partitioned tables > > ┌────────┬────────┬───────┬─────────────┬────────────┬───────────────┬──────────── > │ Schema │ Name │ Owner │ Parent name │ Size │ Children Size │ > Description > ╞════════╪════════╪═══════╪═════════════╪════════════╪═══════════════╡ > │ public │ p │ pavel │ │ │ 8192 bytes │ > │ public │ p_1 │ pavel │ p │ │ 8192 bytes │ > │ public │ p_1_bc │ pavel │ p_1 │ 8192 bytes │ │ > > └────────┴────────┴───────┴─────────────┴────────────┴───────────────┴───────────── > > > > -- > Álvaro Herrera https://www.2ndQuadrant.com/ > PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services >
-
Re: ToDo: show size of partitioned table
Pavel Stehule <pavel.stehule@gmail.com> — 2019-02-08T08:26:46Z
čt 7. 2. 2019 v 9:51 odesílatel Amit Langote <Langote_Amit_f8@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: > > 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 > > -
Re: ToDo: show size of partitioned table
Amit Langote <langote_amit_f8@lab.ntt.co.jp> — 2019-02-15T06:50:11Z
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
-
Re: ToDo: show size of partitioned table
Pavel Stehule <pavel.stehule@gmail.com> — 2019-02-16T21:52:35Z
pá 15. 2. 2019 v 7:50 odesílatel Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> napsal: > 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. > We can do it in future. Now, I don't think so is good to show 0 always. The psql reports (like this) can be enhanced or changed in future without problems, so we don't need to design all now. > > 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. > I like your changes. I merged all - updated patch is attached Thank you very much Regards Pavel > Thanks, > Amit >
-
Re: ToDo: show size of partitioned table
Justin Pryzby <pryzby@telsasoft.com> — 2019-02-20T23:56:33Z
On Sat, Feb 16, 2019 at 10:52:35PM +0100, Pavel Stehule wrote: > I like your changes. I merged all - updated patch is attached I applied and tested your v10 patch. Find attached some light modifications. Thanks, Justin
-
Re: ToDo: show size of partitioned table
Pavel Stehule <pavel.stehule@gmail.com> — 2019-02-21T16:41:07Z
čt 21. 2. 2019 v 0:56 odesílatel Justin Pryzby <pryzby@telsasoft.com> napsal: > On Sat, Feb 16, 2019 at 10:52:35PM +0100, Pavel Stehule wrote: > > I like your changes. I merged all - updated patch is attached > > I applied and tested your v10 patch. > > Find attached some light modifications. > I have not any objection - I cannot to evaluate. It is question for some native speaker. I am not sure if we use labels in form "some: detail" somewhere. Regards Pavel > > Thanks, > Justin >
-
Re: ToDo: show size of partitioned table
Amit Langote <langote_amit_f8@lab.ntt.co.jp> — 2019-03-13T07:01:54Z
On 2019/02/22 1:41, Pavel Stehule wrote: > čt 21. 2. 2019 v 0:56 odesílatel Justin Pryzby <pryzby@telsasoft.com> > napsal: > >> On Sat, Feb 16, 2019 at 10:52:35PM +0100, Pavel Stehule wrote: >>> I like your changes. I merged all - updated patch is attached >> >> I applied and tested your v10 patch. >> >> Find attached some light modifications. >> > > I have not any objection - I cannot to evaluate. It is question for some > native speaker. Not a native speaker either, but I like Justin's changes. Although I noticed that he missed changing one sentence to look like other similar sentences. What Justin did: - indexes) and associated description are also displayed. + indexes) is displayed, as is the relation's description. </para> What I think he meant to do: - indexes) and associated description are also displayed. + indexes) is also displayed, along with the associated description. </para> > I am not sure if we use labels in form "some: detail" somewhere. I haven't seen column names like that either. How about: - gettext_noop("Direct partitions size")); + gettext_noop("Leaf partition size")); - gettext_noop("Total partitions size")); + gettext_noop("Total size")); - gettext_noop("Partitions size")); + gettext_noop("Total size")); I've attached v11 of the patch, which merges most of Justin's changes and some of my own on top -- documentation and partition size column names. Thanks, Amit -
Re: ToDo: show size of partitioned table
Pavel Stehule <pavel.stehule@gmail.com> — 2019-03-13T17:11:01Z
st 13. 3. 2019 v 8:02 odesílatel Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> napsal: > On 2019/02/22 1:41, Pavel Stehule wrote: > > čt 21. 2. 2019 v 0:56 odesílatel Justin Pryzby <pryzby@telsasoft.com> > > napsal: > > > >> On Sat, Feb 16, 2019 at 10:52:35PM +0100, Pavel Stehule wrote: > >>> I like your changes. I merged all - updated patch is attached > >> > >> I applied and tested your v10 patch. > >> > >> Find attached some light modifications. > >> > > > > I have not any objection - I cannot to evaluate. It is question for some > > native speaker. > > Not a native speaker either, but I like Justin's changes. Although I > noticed that he missed changing one sentence to look like other similar > sentences. > > What Justin did: > > - indexes) and associated description are also displayed. > + indexes) is displayed, as is the relation's description. > </para> > > What I think he meant to do: > > - indexes) and associated description are also displayed. > + indexes) is also displayed, along with the associated description. > </para> > > > > I am not sure if we use labels in form "some: detail" somewhere. > > I haven't seen column names like that either. How about: > > - gettext_noop("Direct partitions size")); > + gettext_noop("Leaf partition size")); > > - gettext_noop("Total partitions size")); > + gettext_noop("Total size")); > > - gettext_noop("Partitions size")); > + gettext_noop("Total size")); > +1 Pavel > I've attached v11 of the patch, which merges most of Justin's changes and > some of my own on top -- documentation and partition size column names. > > Thanks, > Amit > -
Re: ToDo: show size of partitioned table
Amit Langote <langote_amit_f8@lab.ntt.co.jp> — 2019-03-14T02:19:16Z
On 2019/03/14 2:11, Pavel Stehule wrote: > st 13. 3. 2019 v 8:02 odesílatel Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> > napsal: >> Not a native speaker either, but I like Justin's changes. Although I >> noticed that he missed changing one sentence to look like other similar >> sentences. >> >> What Justin did: >> >> - indexes) and associated description are also displayed. >> + indexes) is displayed, as is the relation's description. >> </para> >> >> What I think he meant to do: >> >> - indexes) and associated description are also displayed. >> + indexes) is also displayed, along with the associated description. >> </para> >> >> >>> I am not sure if we use labels in form "some: detail" somewhere. >> >> I haven't seen column names like that either. How about: >> >> - gettext_noop("Direct partitions size")); >> + gettext_noop("Leaf partition size")); >> >> - gettext_noop("Total partitions size")); >> + gettext_noop("Total size")); >> >> - gettext_noop("Partitions size")); >> + gettext_noop("Total size")); >> > > +1 > > Pavel > > >> I've attached v11 of the patch, which merges most of Justin's changes and >> some of my own on top -- documentation and partition size column names. Maybe, we should set this ready for committer then? Thanks, Amit -
Re: Re: ToDo: show size of partitioned table
David Steele <david@pgmasters.net> — 2019-03-21T17:23:45Z
On 3/14/19 6:19 AM, Amit Langote wrote: > On 2019/03/14 2:11, Pavel Stehule wrote: >> >>> I've attached v11 of the patch, which merges most of Justin's changes and >>> some of my own on top -- documentation and partition size column names. > > Maybe, we should set this ready for committer then? There don't appear to be any objections. Perhaps you should do that? Regards, -- -David david@pgmasters.net
-
Re: ToDo: show size of partitioned table
Amit Langote <langote_amit_f8@lab.ntt.co.jp> — 2019-03-22T00:21:56Z
On 2019/03/22 2:23, David Steele wrote: > On 3/14/19 6:19 AM, Amit Langote wrote: >> On 2019/03/14 2:11, Pavel Stehule wrote: >>> >>>> I've attached v11 of the patch, which merges most of Justin's changes and >>>> some of my own on top -- documentation and partition size column names. >> >> Maybe, we should set this ready for committer then? > > There don't appear to be any objections. Perhaps you should do that? OK, done. Thanks, Amit
-
Re: ToDo: show size of partitioned table
Peter Eisentraut <peter.eisentraut@2ndquadrant.com> — 2019-03-28T09:12:18Z
On 2019-03-22 01:21, Amit Langote wrote: > On 2019/03/22 2:23, David Steele wrote: >> On 3/14/19 6:19 AM, Amit Langote wrote: >>> On 2019/03/14 2:11, Pavel Stehule wrote: >>>> >>>>> I've attached v11 of the patch, which merges most of Justin's changes and >>>>> some of my own on top -- documentation and partition size column names. >>> >>> Maybe, we should set this ready for committer then? >> >> There don't appear to be any objections. Perhaps you should do that? > > OK, done. What is the purpose of this patch (hint: commit message)? The email subject is "show size of partitioned table", which seems reasonable, but looking briefly at a patch, it adds new psql commands to display partitioned tables only. I don't understand the purpose of that. (Moreover, who is the author of this patch? Is the commit fest entry accurate?) -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
-
Re: ToDo: show size of partitioned table
Pavel Stehule <pavel.stehule@gmail.com> — 2019-03-28T09:29:52Z
čt 28. 3. 2019 v 10:12 odesílatel Peter Eisentraut < peter.eisentraut@2ndquadrant.com> napsal: > On 2019-03-22 01:21, Amit Langote wrote: > > On 2019/03/22 2:23, David Steele wrote: > >> On 3/14/19 6:19 AM, Amit Langote wrote: > >>> On 2019/03/14 2:11, Pavel Stehule wrote: > >>>> > >>>>> I've attached v11 of the patch, which merges most of Justin's > changes and > >>>>> some of my own on top -- documentation and partition size column > names. > >>> > >>> Maybe, we should set this ready for committer then? > >> > >> There don't appear to be any objections. Perhaps you should do that? > > > > OK, done. > > What is the purpose of this patch (hint: commit message)? The email > subject is "show size of partitioned table", which seems reasonable, but > looking briefly at a patch, it adds new psql commands to display > partitioned tables only. I don't understand the purpose of that. > > When I started with this patch, main goal was displaying total size of partitioned tables (sum of size of partitions). Now, this it can show more - size of indexes, data, - it can aggregate per partitions levels. > (Moreover, who is the author of this patch? Is the commit fest entry > accurate?) > I am a author, but lot of peoples did modifications and comments. Regards Pavel > > -- > Peter Eisentraut http://www.2ndQuadrant.com/ > PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services >
-
Re: ToDo: show size of partitioned table
Amit Langote <langote_amit_f8@lab.ntt.co.jp> — 2019-03-28T09:31:44Z
Hi, On 2019/03/28 18:12, Peter Eisentraut wrote: > On 2019-03-22 01:21, Amit Langote wrote: >> On 2019/03/22 2:23, David Steele wrote: >>> On 3/14/19 6:19 AM, Amit Langote wrote: >>>> On 2019/03/14 2:11, Pavel Stehule wrote: >>>>> >>>>>> I've attached v11 of the patch, which merges most of Justin's changes and >>>>>> some of my own on top -- documentation and partition size column names. >>>> >>>> Maybe, we should set this ready for committer then? >>> >>> There don't appear to be any objections. Perhaps you should do that? >> >> OK, done. > > What is the purpose of this patch (hint: commit message)? The email > subject is "show size of partitioned table", which seems reasonable, but > looking briefly at a patch, it adds new psql commands to display > partitioned tables only. I don't understand the purpose of that. > > (Moreover, who is the author of this patch? Is the commit fest entry > accurate?) It's mainly Pavel's patch, which I've occasionally posted updated versions of, when it appeared to me that Pavel might be busy with other stuff. As mentioned somewhere at the top of this thread, the main reason behind proposing a new command \dP is that some people didn't much like the idea that \d+ command itself be modified to gather all partitions and add their sizes to be shown as the size of the partitioned tables. More features got added later on to account for partitioned indexes, showing sub-partitioned tables in addition to root tables that are shown by default, etc. Maybe, Pavel can say more... Thanks, Amit
-
Re: ToDo: show size of partitioned table
Amit Langote <langote_amit_f8@lab.ntt.co.jp> — 2019-03-28T09:33:23Z
On 2019/03/28 18:31, Amit Langote wrote: > Hi, > > On 2019/03/28 18:12, Peter Eisentraut wrote: >> On 2019-03-22 01:21, Amit Langote wrote: >>> On 2019/03/22 2:23, David Steele wrote: >>>> On 3/14/19 6:19 AM, Amit Langote wrote: >>>>> On 2019/03/14 2:11, Pavel Stehule wrote: >>>>>> >>>>>>> I've attached v11 of the patch, which merges most of Justin's changes and >>>>>>> some of my own on top -- documentation and partition size column names. >>>>> >>>>> Maybe, we should set this ready for committer then? >>>> >>>> There don't appear to be any objections. Perhaps you should do that? >>> >>> OK, done. >> >> What is the purpose of this patch (hint: commit message)? The email >> subject is "show size of partitioned table", which seems reasonable, but >> looking briefly at a patch, it adds new psql commands to display >> partitioned tables only. I don't understand the purpose of that. >> >> (Moreover, who is the author of this patch? Is the commit fest entry >> accurate?) > > It's mainly Pavel's patch, which I've occasionally posted updated versions > of, when it appeared to me that Pavel might be busy with other stuff. > > As mentioned somewhere at the top of this thread, the main reason behind > proposing a new command \dP is that some people didn't much like the idea > that \d+ command itself be modified to gather all partitions and add their > sizes to be shown as the size of the partitioned tables. More features > got added later on to account for partitioned indexes, showing > sub-partitioned tables in addition to root tables that are shown by > default, etc. > > Maybe, Pavel can say more... Ah, his message got in before mine. Thanks, Amit
-
Re: ToDo: show size of partitioned table
Alvaro Herrera <alvherre@2ndquadrant.com> — 2019-04-02T19:15:31Z
On 2019-Mar-13, Amit Langote wrote: > + if (show_indexes) > + { > + objects_name = gettext_noop("indexes"); ... > + if (pattern) > + /* translator: objects_name is "index", "table" */ > + psql_error("Did not find any partitioned %s named \"%s\".\n", > + object_name, > + pattern); ... > + /* translator: objects_name is "indexes", "tables" */ > + appendPQExpBuffer(&title, _("List of partitioned %s"), objects_name); > + This stuff doesn't work from a translation standpoint; sentence building is not allowed. You have to do stuff like: if (tables) { errmsg = gettext_noop("Could not find any partitioned table"); tabletitle = gettext_noop("List of partitioned tables"); } ... if (pattern) /* translator: objects_name is "index", "table" */ psql_error(gettext(errmsg), object_name, pattern); ... appendPQExpBuffer(&title, gettext(tabletitle)); and so on. -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -
Re: ToDo: show size of partitioned table
Alvaro Herrera <alvherre@2ndquadrant.com> — 2019-04-02T19:25:06Z
Something is not right: alvherre=# \dP t List of partitioned tables or indexes Esquema | Nombre | Dueño | Tipo | On table ---------+--------+----------+-------------------+-------------------------------------------------------------- public | t | alvherre | partitioned table | Project-Id-Version: psql (PostgreSQL) 10 + | | | | Report-Msgid-Bugs-To: pgsql-bugs@postgresql.org + | | | | PO-Revision-Date: 2017-07-10 12:14-0400 + | | | | Last-Translator: Álvaro Herrera <alvherre@alvh.no-ip.org> + | | | | Language-Team: PgSQL Español <pgsql-es-ayuda@postgresql.org>+ | | | | Language: es + | | | | MIME-Version: 1.0 + | | | | Content-Type: text/plain; charset=UTF-8 + | | | | Content-Transfer-Encoding: 8bit + | | | | Plural-Forms: nplurals=2; plural=n != 1; + | | | | (1 fila) -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -
Re: ToDo: show size of partitioned table
Alvaro Herrera <alvherre@2ndquadrant.com> — 2019-04-02T19:42:55Z
On 2019-Apr-02, Alvaro Herrera wrote: > Something is not right: Another thing that was not right is that translated_columns was being marked static, and modified in the function; so if you called \dP twice where one called for translation and the second not, the second time we'd also translating that column. Attached contains fixes for those three issues. I'm gonna go over the docs now. -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
-
Re: ToDo: show size of partitioned table
Amit Langote <langote_amit_f8@lab.ntt.co.jp> — 2019-04-03T01:03:29Z
Hi Alvaro, On 2019/04/03 4:42, Alvaro Herrera wrote: > On 2019-Apr-02, Alvaro Herrera wrote: > >> Something is not right: > > Another thing that was not right is that translated_columns was being > marked static, and modified in the function; so if you called \dP twice > where one called for translation and the second not, the second time > we'd also translating that column. > > Attached contains fixes for those three issues. Thanks. I'm getting an error while applying v12: patching file src/bin/psql/po/es.po Hunk #1 FAILED at 12. 1 out of 2 hunks FAILED -- saving rejects to file src/bin/psql/po/es.po.rej Did you accidentally add it to the patch? Thanks, Amit
-
Re: ToDo: show size of partitioned table
Alvaro Herrera <alvherre@2ndquadrant.com> — 2019-04-03T01:49:25Z
Hi Amit On 2019-Apr-03, Amit Langote wrote: > Thanks. I'm getting an error while applying v12: > > patching file src/bin/psql/po/es.po > Hunk #1 FAILED at 12. > 1 out of 2 hunks FAILED -- saving rejects to file src/bin/psql/po/es.po.rej Uh, I certainly did not intend to send the es.po part of the patch, I was just testing that the translation worked as intended. That's what I get for doing stuff till the last minute ... -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
-
Re: ToDo: show size of partitioned table
Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> — 2019-04-03T03:02:09Z
At Tue, 2 Apr 2019 22:49:25 -0300, Alvaro Herrera <alvherre@2ndquadrant.com> wrote in <20190403014925.GA9976@alvherre.pgsql> > Hi Amit > > On 2019-Apr-03, Amit Langote wrote: > > > Thanks. I'm getting an error while applying v12: > > > > patching file src/bin/psql/po/es.po > > Hunk #1 FAILED at 12. > > 1 out of 2 hunks FAILED -- saving rejects to file src/bin/psql/po/es.po.rej > > Uh, I certainly did not intend to send the es.po part of the patch, I > was just testing that the translation worked as intended. That's what I > get for doing stuff till the last minute ... \dPn doesn't show children because the are of 'r' relkind. And \dPn * doesn't show type for children. create table pa (a int, b int) partition by range (a); create table c1 partition of pa for values from (0) to (10); create table c2 partition of pa for values from (10) to (20); create index on pa(a); postgres=# \dPn * List of partitioned tables and indexes Schema | Name | Owner | Type | Parent name | On table --------+----------+----------+-------------------+-------------+---------- public | pa | horiguti | partitioned table | | public | pa_a_idx | horiguti | partitioned index | | pa (2 rows) With the attached on top of the latest patch, we get the following result. postgres=# \dPn * List of partitioned tables and indexes Schema | Name | Owner | Type | Parent name | On table --------+----------+----------+-------------------+-------------+---------- public | c1 | horiguti | partition table | pa | public | c1_a_idx | horiguti | partition index | pa_a_idx | c1 public | c2 | horiguti | partition table | pa | public | c2_a_idx | horiguti | partition index | pa_a_idx | c2 public | pa | horiguti | partitioned table | | public | pa_a_idx | horiguti | partitioned index | | pa (6 rows) regards. -- Kyotaro Horiguchi NTT Open Source Software Center -
Re: ToDo: show size of partitioned table
Amit Langote <langote_amit_f8@lab.ntt.co.jp> — 2019-04-03T03:55:06Z
Horiguchi-san, Thanks for taking a look. On 2019/04/03 12:02, Kyotaro HORIGUCHI wrote: > \dPn doesn't show children because the are of 'r' relkind. And > \dPn * doesn't show type for children. > > create table pa (a int, b int) partition by range (a); > create table c1 partition of pa for values from (0) to (10); > create table c2 partition of pa for values from (10) to (20); > create index on pa(a); > > postgres=# \dPn * > List of partitioned tables and indexes > Schema | Name | Owner | Type | Parent name | On table > --------+----------+----------+-------------------+-------------+---------- > public | pa | horiguti | partitioned table | | > public | pa_a_idx | horiguti | partitioned index | | pa > (2 rows) > > > With the attached on top of the latest patch, we get the following result. > > postgres=# \dPn * > List of partitioned tables and indexes > Schema | Name | Owner | Type | Parent name | On table > --------+----------+----------+-------------------+-------------+---------- > public | c1 | horiguti | partition table | pa | > public | c1_a_idx | horiguti | partition index | pa_a_idx | c1 > public | c2 | horiguti | partition table | pa | > public | c2_a_idx | horiguti | partition index | pa_a_idx | c2 > public | pa | horiguti | partitioned table | | > public | pa_a_idx | horiguti | partitioned index | | pa > (6 rows) I think it's intentional that leaf partitions are not shown, because the patch is meant to allow showing only the "partitioned" members of partition trees. Regular \d[t|i][+] shows normal tables ('r', 'i', etc.) including their size, but it's useless for partitioned tables ('P', 'I', etc.) as far as showing the size is concerned, so the patch. Even \dPn is meant to only show partitions that are themselves partitioned; note the "P" in the command. Thanks, Amit -
Re: ToDo: show size of partitioned table
Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> — 2019-04-03T04:19:30Z
Hello. At Wed, 3 Apr 2019 12:55:06 +0900, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote in <ee892049-0fe4-afe6-cbbf-31cf44fa8522@lab.ntt.co.jp> > On 2019/04/03 12:02, Kyotaro HORIGUCHI wrote: > > \dPn doesn't show children because the are of 'r' relkind. And > > \dPn * doesn't show type for children. ... > I think it's intentional that leaf partitions are not shown, because the > patch is meant to allow showing only the "partitioned" members of > partition trees. Regular \d[t|i][+] shows normal tables ('r', 'i', etc.) > including their size, but it's useless for partitioned tables ('P', 'I', > etc.) as far as showing the size is concerned, so the patch. Even \dPn is > meant to only show partitions that are themselves partitioned; note the > "P" in the command. + If the modifier <literal>n</literal> (<quote>nested</quote>) is used, + then non-root partitioned tables are included, and a column is shown + displaying the parent of each partitioned relation. Ah. I see. "non-root *partitined* tables". I misread the phrase. Sorry for the noise. regards. -- Kyotaro Horiguchi NTT Open Source Software Center -
Re: ToDo: show size of partitioned table
Alvaro Herrera <alvherre@2ndquadrant.com> — 2019-04-03T12:01:45Z
On 2019-Apr-03, Kyotaro HORIGUCHI wrote: > Hello. > > At Wed, 3 Apr 2019 12:55:06 +0900, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote in <ee892049-0fe4-afe6-cbbf-31cf44fa8522@lab.ntt.co.jp> > > On 2019/04/03 12:02, Kyotaro HORIGUCHI wrote: > > > \dPn doesn't show children because the are of 'r' relkind. And > > > \dPn * doesn't show type for children. > ... > > I think it's intentional that leaf partitions are not shown, because the > > patch is meant to allow showing only the "partitioned" members of > > partition trees. Regular \d[t|i][+] shows normal tables ('r', 'i', etc.) > > including their size, but it's useless for partitioned tables ('P', 'I', > > etc.) as far as showing the size is concerned, so the patch. Even \dPn is > > meant to only show partitions that are themselves partitioned; note the > > "P" in the command. > > + If the modifier <literal>n</literal> (<quote>nested</quote>) is used, > + then non-root partitioned tables are included, and a column is shown > + displaying the parent of each partitioned relation. > > Ah. I see. "non-root *partitined* tables". I misread the > phrase. Sorry for the noise. Well, is this decision an excellent one, from a UI perspective? I was surprised by this too and think this should be reconsidered. I would opt for having \d NOT show partitions at all, myself. When you have many partitions (and we're now making the system scale into the thousands for a single partitioned table), they clutter the output making it unusable. So, rather than think as \dP as "the way to show the aggregate size of a partitioned table or index", I'd think it as "the way to show detailed info about a partitioned table or index". Which includes things like displaying its list of partitions and the size of each. -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -
Re: ToDo: show size of partitioned table
Pavel Stehule <pavel.stehule@gmail.com> — 2019-04-03T13:49:43Z
st 3. 4. 2019 v 14:01 odesílatel Alvaro Herrera <alvherre@2ndquadrant.com> napsal: > On 2019-Apr-03, Kyotaro HORIGUCHI wrote: > > > Hello. > > > > At Wed, 3 Apr 2019 12:55:06 +0900, Amit Langote < > Langote_Amit_f8@lab.ntt.co.jp> wrote in < > ee892049-0fe4-afe6-cbbf-31cf44fa8522@lab.ntt.co.jp> > > > On 2019/04/03 12:02, Kyotaro HORIGUCHI wrote: > > > > \dPn doesn't show children because the are of 'r' relkind. And > > > > \dPn * doesn't show type for children. > > ... > > > I think it's intentional that leaf partitions are not shown, because > the > > > patch is meant to allow showing only the "partitioned" members of > > > partition trees. Regular \d[t|i][+] shows normal tables ('r', 'i', > etc.) > > > including their size, but it's useless for partitioned tables ('P', > 'I', > > > etc.) as far as showing the size is concerned, so the patch. Even > \dPn is > > > meant to only show partitions that are themselves partitioned; note the > > > "P" in the command. > > > > + If the modifier <literal>n</literal> (<quote>nested</quote>) is > used, > > + then non-root partitioned tables are included, and a column is > shown > > + displaying the parent of each partitioned relation. > > > > Ah. I see. "non-root *partitined* tables". I misread the > > phrase. Sorry for the noise. > I don't need to see root partitioned tables, because this table is empty. > Well, is this decision an excellent one, from a UI perspective? I was > surprised by this too and think this should be reconsidered. > > I would opt for having \d NOT show partitions at all, myself. When you > have many partitions (and we're now making the system scale into the > thousands for a single partitioned table), they clutter the output > making it unusable. So, rather than think as \dP as "the way to show > the aggregate size of a partitioned table or index", I'd think it as > "the way to show detailed info about a partitioned table or index". > Which includes things like displaying its list of partitions and the > size of each. > I think so there can be more sensible perspectives how to watch partitioned data, and how and where you would to see details. For example - if you remove detail from \d command, then detail should be displayed in some other command. Now, the detail is in \d and then I need some aggregation - and it is \dP(*). For me it is not important if is looking from left to right or from right to left if I see data that I need. Personally, I have not strong opinion what should be best way - but I am thinking so some aggregated information over all partitions is necessary. Currently \d is complex enough, so I designed new command. Any time, this will be complex task, because we should to display tree structure in a table. > > -- > Álvaro Herrera https://www.2ndQuadrant.com/ > PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services > -
Re: ToDo: show size of partitioned table
Amit Langote <langote_amit_f8@lab.ntt.co.jp> — 2019-04-04T10:41:53Z
Hi Alvaro, On 2019/04/03 21:01, Alvaro Herrera wrote: > On 2019-Apr-03, Kyotaro HORIGUCHI wrote: > >> Hello. >> >> At Wed, 3 Apr 2019 12:55:06 +0900, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote in <ee892049-0fe4-afe6-cbbf-31cf44fa8522@lab.ntt.co.jp> >>> On 2019/04/03 12:02, Kyotaro HORIGUCHI wrote: >>>> \dPn doesn't show children because the are of 'r' relkind. And >>>> \dPn * doesn't show type for children. >> ... >>> I think it's intentional that leaf partitions are not shown, because the >>> patch is meant to allow showing only the "partitioned" members of >>> partition trees. Regular \d[t|i][+] shows normal tables ('r', 'i', etc.) >>> including their size, but it's useless for partitioned tables ('P', 'I', >>> etc.) as far as showing the size is concerned, so the patch. Even \dPn is >>> meant to only show partitions that are themselves partitioned; note the >>> "P" in the command. >> >> + If the modifier <literal>n</literal> (<quote>nested</quote>) is used, >> + then non-root partitioned tables are included, and a column is shown >> + displaying the parent of each partitioned relation. >> >> Ah. I see. "non-root *partitined* tables". I misread the >> phrase. Sorry for the noise. > > Well, is this decision an excellent one, from a UI perspective? I was > surprised by this too and think this should be reconsidered. > > I would opt for having \d NOT show partitions at all, myself. Hmm, in the previous long discussions on this topic, people were opposed [1] to an idea like this, but maybe they are no longer. Note that the main opposition came to the idea that \d+ would perform simple pg_relation_size for regular tables and something else for partitioned tables, such as, aggregation query over pg_partition_tree. > When you > have many partitions (and we're now making the system scale into the > thousands for a single partitioned table), they clutter the output > making it unusable. So, rather than think as \dP as "the way to show > the aggregate size of a partitioned table or index", I'd think it as > "the way to show detailed info about a partitioned table or index". > Which includes things like displaying its list of partitions and the > size of each. That might be a good idea, but maybe there isn't time left to revise the proposed patch like that for 12? Thanks, Amit [1] https://www.postgresql.org/message-id/flat/495cec7e-f8d9-7e13-4807-90dbf4eec4ea@lab.ntt.co.jp -
Re: ToDo: show size of partitioned table
Alvaro Herrera <alvherre@2ndquadrant.com> — 2019-04-05T23:28:11Z
On 2019-Apr-04, Amit Langote wrote: > > When you > > have many partitions (and we're now making the system scale into the > > thousands for a single partitioned table), they clutter the output > > making it unusable. So, rather than think as \dP as "the way to show > > the aggregate size of a partitioned table or index", I'd think it as > > "the way to show detailed info about a partitioned table or index". > > Which includes things like displaying its list of partitions and the > > size of each. > > That might be a good idea, but maybe there isn't time left to revise the > proposed patch like that for 12? Well, the problem with designing psql commands incrementallyj is that people are angry if you try to change what they show in a future release. We only have one shot to get it right, pretty much. Looking at the current proposal, I think I like \dPn+ very much -- it shows the aggregated size of all partitioned tables. But I think \dP (without the +) is almost completely useless. I'm not really sure about having to add the "n" flag, but I suspect in practical use it's okay. Also, I think \dPn+ shows partitioned tables, but \dPtn+ shows exactly the same, so why do we have the "t" flag at all? We have \dPin+ which shows aggregate size, but the only way to list both tables and indexes is to add a pattern. I think this design was confusingly inspired by the "list" vs. "describe" schizoid dichotomy of \d, without actually getting any useful functionality in return. IMO \dP should be "list partitioned relations with their sizes", and \dPt "list partitioned tables with their sizes", and \dPi "list partitioned indexes with their sizes". If no pattern is given, list them all, otherwise only list those that match the pattern. -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
-
Re: ToDo: show size of partitioned table
Alvaro Herrera <alvherre@2ndquadrant.com> — 2019-04-06T04:36:23Z
On 2019-Apr-05, Alvaro Herrera wrote: > Looking at the current proposal, I think I like \dPn+ very much -- it > shows the aggregated size of all partitioned tables. But I think \dP > (without the +) is almost completely useless. I'm not really sure about > having to add the "n" flag, but I suspect in practical use it's okay. > > Also, I think \dPn+ shows partitioned tables, but \dPtn+ shows exactly > the same, so why do we have the "t" flag at all? We have \dPin+ which > shows aggregate size, but the only way to list both tables and indexes > is to add a pattern. I think this design was confusingly inspired by > the "list" vs. "describe" schizoid dichotomy of \d, without actually > getting any useful functionality in return. > > IMO \dP should be "list partitioned relations with their sizes", and > \dPt "list partitioned tables with their sizes", and \dPi "list > partitioned indexes with their sizes". If no pattern is given, list > them all, otherwise only list those that match the pattern. After thinking more about this, I'm having second thoughts about the + thing. I'm now thinking that requiring the + for computing sizes is actually a good thing, because if we change it to show all sizes inconditionally, the command becomes unusable for users with lots of large partitioned tables. So the submitted patch is okay in that front. Maybe the only behavior change I'd do to the submitted patch is to have \dP show both tables and indexes, while \dPt shows only tables and \dPi shows only indexes. Maybe have \dPti show both tables and indexes? ( identical to \dP) That would be consistent with \d itself. Also, compare describeFunctions, which allows multiple type specifiers to be given. I'm not in love with the way it handles the "n", "t" and "i" specifiers. I think we should allow them in any order, not just if the "n" is in cmd[2]. -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
-
Re: ToDo: show size of partitioned table
Pavel Stehule <pavel.stehule@gmail.com> — 2019-04-06T19:24:36Z
so 6. 4. 2019 v 6:36 odesílatel Alvaro Herrera <alvherre@2ndquadrant.com> napsal: > On 2019-Apr-05, Alvaro Herrera wrote: > > > Looking at the current proposal, I think I like \dPn+ very much -- it > > shows the aggregated size of all partitioned tables. But I think \dP > > (without the +) is almost completely useless. I'm not really sure about > > having to add the "n" flag, but I suspect in practical use it's okay. > > > > Also, I think \dPn+ shows partitioned tables, but \dPtn+ shows exactly > > the same, so why do we have the "t" flag at all? We have \dPin+ which > > shows aggregate size, but the only way to list both tables and indexes > > is to add a pattern. I think this design was confusingly inspired by > > the "list" vs. "describe" schizoid dichotomy of \d, without actually > > getting any useful functionality in return. > > > > IMO \dP should be "list partitioned relations with their sizes", and > > \dPt "list partitioned tables with their sizes", and \dPi "list > > partitioned indexes with their sizes". If no pattern is given, list > > them all, otherwise only list those that match the pattern. > > After thinking more about this, I'm having second thoughts about the + > thing. I'm now thinking that requiring the + for computing sizes is > actually a good thing, because if we change it to show all sizes > inconditionally, the command becomes unusable for users with lots of > large partitioned tables. So the submitted patch is okay in that front. > I remember, there can be another reason - the size can be different on some platforms, and display size by default can break regress tests. > > Maybe the only behavior change I'd do to the submitted patch is to have > \dP show both tables and indexes, while \dPt shows only tables and \dPi > shows only indexes. Maybe have \dPti show both tables and indexes? ( > identical to \dP) That would be consistent with \d itself. Also, > compare describeFunctions, which allows multiple type specifiers to be > given. > One my idea (maybe not great) was using total relation size, when user doesn't specify if would to see tables or indexes. Because we support \dPi (index size) and \dPt (table size), then I though so for \dP there is a good possibility to use total relation size. So now \dP is not same as possible \dPti by design. But I can imagine \dPti as union of \dPt and \dPi Currently \dP supports both, without pattern then it show tables with possible total relation size, with pattern it shows tables and indexes with related sizes - so behave is very similar to \dPti (but it is driven by pattern usage). > > I'm not in love with the way it handles the "n", "t" and "i" specifiers. > I think we should allow them in any order, not just if the "n" is in > cmd[2]. > It is good idea. > > -- > Álvaro Herrera https://www.2ndQuadrant.com/ > PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services >
-
Re: ToDo: show size of partitioned table
Alvaro Herrera <alvherre@2ndquadrant.com> — 2019-04-07T12:15:06Z
So how about the attached version? -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
-
Re: ToDo: show size of partitioned table
Justin Pryzby <pryzby@telsasoft.com> — 2019-04-07T15:27:08Z
On Sun, Apr 07, 2019 at 08:15:06AM -0400, Alvaro Herrera wrote: > So how about the attached version? +1 I found a few issues. \dP+ didn't work. Fix attached. +static const SchemaQuery Query_for_list_of_partitioned_relations = { + .catname = "pg_catalog.pg_class c", + .selcondition = "c.relkind = " CppAsString2(RELKIND_PARTITIONED_TABLE), => Should it be called Query_for_list_of_partitioned_tables ? Or should c.relkind match indices, too ? On Sat, Apr 06, 2019 at 01:36:23AM -0300, Alvaro Herrera wrote: > Maybe the only behavior change I'd do to the submitted patch is to have > \dP show both tables and indexes, while \dPt shows only tables and \dPi > shows only indexes. Maybe have \dPti show both tables and indexes? ( > identical to \dP) That would be consistent with \d itself. I think there's an issue with showing indices. You said that \dP should be same as \dPti, no? Right now, indices are not shown in \dP, unless a pattern is given. I see you add that behavior in the regression tests; is that really what's intended ? Also, right now adding a pattern affects how sizes are computed, I don't see why that's desirable or, if so, how to resolve that inconsistency, or how to document it. Justin -
Re: ToDo: show size of partitioned table
Pavel Stehule <pavel.stehule@gmail.com> — 2019-04-07T15:47:53Z
ne 7. 4. 2019 v 17:27 odesílatel Justin Pryzby <pryzby@telsasoft.com> napsal: > On Sun, Apr 07, 2019 at 08:15:06AM -0400, Alvaro Herrera wrote: > > So how about the attached version? > > +1 > > I found a few issues. > > \dP+ didn't work. Fix attached. > > +static const SchemaQuery Query_for_list_of_partitioned_relations = { > > + .catname = "pg_catalog.pg_class c", > > + .selcondition = "c.relkind = " > CppAsString2(RELKIND_PARTITIONED_TABLE), > > > => Should it be called Query_for_list_of_partitioned_tables ? Or should > c.relkind match indices, too ? > > On Sat, Apr 06, 2019 at 01:36:23AM -0300, Alvaro Herrera wrote: > > Maybe the only behavior change I'd do to the submitted patch is to have > > \dP show both tables and indexes, while \dPt shows only tables and \dPi > > shows only indexes. Maybe have \dPti show both tables and indexes? ( > > identical to \dP) That would be consistent with \d itself. > > I think there's an issue with showing indices. You said that \dP should be > same as \dPti, no? Right now, indices are not shown in \dP, unless a > pattern > is given. I see you add that behavior in the regression tests; is that > really > what's intended ? Also, right now adding a pattern affects how sizes are > computed, I don't see why that's desirable or, if so, how to resolve that > inconsistency, or how to document it. > That depends. If there are not pattern, then \dP show only tables, but with total relation size (so size of indexes are nested). It is different than \dPti, but I think so it is useful - when you don't specify object type, then usually you would to see a tables, but with total size. I don't see a benefit from \dP == \dPti. When there are a pattern (that can choose some index, then, indexes are displayed and \dP == \dPti. I think so Alvaro's version is correct, and I prefer it. Regards Pavel > Justin > -
Re: ToDo: show size of partitioned table
Pavel Stehule <pavel.stehule@gmail.com> — 2019-04-07T15:50:44Z
ne 7. 4. 2019 v 14:15 odesílatel Alvaro Herrera <alvherre@2ndquadrant.com> napsal: > So how about the attached version? > +1 Pavel > > -- > Álvaro Herrera https://www.2ndQuadrant.com/ > PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services >
-
Re: ToDo: show size of partitioned table
Alvaro Herrera <alvherre@2ndquadrant.com> — 2019-04-07T16:07:50Z
On 2019-Apr-07, Pavel Stehule wrote: > ne 7. 4. 2019 v 17:27 odesílatel Justin Pryzby <pryzby@telsasoft.com> > napsal: > > I think there's an issue with showing indices. You said that \dP should be > > same as \dPti, no? Right now, indices are not shown in \dP, unless a > > pattern is given. I see you add that behavior in the regression > > tests; is that really what's intended ? Also, right now adding a > > pattern affects how sizes are computed, I don't see why that's > > desirable or, if so, how to resolve that inconsistency, or how to > > document it. > > That depends. If there are not pattern, then \dP show only tables, but with > total relation size (so size of indexes are nested). It is different than > \dPti, but I think so it is useful - when you don't specify object type, > then usually you would to see a tables, but with total size. > > I don't see a benefit from \dP == \dPti. When there are a pattern (that can > choose some index, then, indexes are displayed and \dP == \dPti. Well, I think Justin has it right --- \dP should be just like \df, which means to list "everything". If you add the "t" or the "i", that means to list only those kinds of things (just like adding one of a, n, p, t, w does for \df). You can add both, and then it list both kinds, just like \dfanptw list the same things that \df does. That's also what I changed the docs to say, but I failed to update the code correctly, and didn't verify the expected output closely either. So I'm due to resubmit this ... -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
-
Re: ToDo: show size of partitioned table
Pavel Stehule <pavel.stehule@gmail.com> — 2019-04-07T16:11:29Z
ne 7. 4. 2019 v 18:07 odesílatel Alvaro Herrera <alvherre@2ndquadrant.com> napsal: > On 2019-Apr-07, Pavel Stehule wrote: > > > ne 7. 4. 2019 v 17:27 odesílatel Justin Pryzby <pryzby@telsasoft.com> > > napsal: > > > > I think there's an issue with showing indices. You said that \dP > should be > > > same as \dPti, no? Right now, indices are not shown in \dP, unless a > > > pattern is given. I see you add that behavior in the regression > > > tests; is that really what's intended ? Also, right now adding a > > > pattern affects how sizes are computed, I don't see why that's > > > desirable or, if so, how to resolve that inconsistency, or how to > > > document it. > > > > That depends. If there are not pattern, then \dP show only tables, but > with > > total relation size (so size of indexes are nested). It is different than > > \dPti, but I think so it is useful - when you don't specify object type, > > then usually you would to see a tables, but with total size. > > > > I don't see a benefit from \dP == \dPti. When there are a pattern (that > can > > choose some index, then, indexes are displayed and \dP == \dPti. > > Well, I think Justin has it right --- \dP should be just like \df, which > means to list "everything". If you add the "t" or the "i", that means > to list only those kinds of things (just like adding one of a, n, p, t, > w does for \df). You can add both, and then it list both kinds, just > like \dfanptw list the same things that \df does. > > That's also what I changed the docs to say, but I failed to update the > code correctly, and didn't verify the expected output closely either. > So I'm due to resubmit this ... > ok Pavel > -- > Álvaro Herrera https://www.2ndQuadrant.com/ > PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services >
-
Re: ToDo: show size of partitioned table
Alvaro Herrera <alvherre@2ndquadrant.com> — 2019-04-07T17:16:08Z
Here. -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
-
Re: ToDo: show size of partitioned table
Pavel Stehule <pavel.stehule@gmail.com> — 2019-04-07T17:52:33Z
ne 7. 4. 2019 v 19:16 odesílatel Alvaro Herrera <alvherre@2ndquadrant.com> napsal: > Here. > + 1 Pavel > > -- > Álvaro Herrera https://www.2ndQuadrant.com/ > PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services >
-
Re: ToDo: show size of partitioned table
Alvaro Herrera <alvherre@2ndquadrant.com> — 2019-04-07T18:27:35Z
On 2019-Apr-07, Pavel Stehule wrote: > ne 7. 4. 2019 v 19:16 odesílatel Alvaro Herrera <alvherre@2ndquadrant.com> > napsal: > > > Here. > > + 1 BTW I'm not sure if you noticed, but I removed the error message "no partitioned relations found" when the result was empty. This was mimicking \d behavior (which was justified on historical grounds), and \drds behavior (which was justified on pattern ordering grounds); but I see nothing that justifies it for \dP, so let's make it behave like all the other commands and display an empty table. And there's an additional change to make. In the regression database, this returns an empty table: regression=# \dPi regress_indexing.pk5_pkey List of partitioned indexes Esquema | Nombre | Dueño | On table ---------+--------+-------+---------- (0 filas) but the index does exist, and it is a partitioned one. So it should be displayed. In fact, if I add the "n" flag, it shows: regression=# \dPin regress_indexing.pk5_pkey List of partitioned indexes Esquema | Nombre | Dueño | Parent name | On table ------------------+----------+----------+--------------------------+---------------------- regress_indexing | pk5_pkey | alvherre | regress_indexing.pk_pkey | regress_indexing.pk5 (1 fila) I think the fact that \dPi doesn't show it is broken, and this fixes it: @@ -3946,7 +3946,8 @@ listPartitionedTables(const char *reltypes, const char *pattern, bool verbose) appendPQExpBufferStr(&buf, "''"); /* dummy */ appendPQExpBufferStr(&buf, ")\n"); - appendPQExpBufferStr(&buf, !showNested ? " AND NOT c.relispartition\n" : "\n"); + appendPQExpBufferStr(&buf, !showNested && !pattern ? + " AND NOT c.relispartition\n" : ""); if (!pattern) appendPQExpBufferStr(&buf, " AND n.nspname <> 'pg_catalog'\n" In order for this to display sanely, I added the "Parent name" column if either the "n" flag is shown or a pattern is given (previously it only appeared in the former case). Note that this changes the expected output in the test; now every test that uses a pattern displays *all* partitioned relations that match the pattern, not just top-level ones. I'm about +0.2 convinced that this is desirable, but my first example above tilts the scales to changing it as described. I noticed this while testing after messing with the tab completion as suggested by Justin: we already had Query_for_list_of_partitioned_tables (which you could have used), but since \dP works for both indexes and tables, I instead modified your new Query_for_list_of_partitioned_relations to list both relation kinds. -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -
Re: ToDo: show size of partitioned table
Pavel Stehule <pavel.stehule@gmail.com> — 2019-04-07T18:53:21Z
ne 7. 4. 2019 v 20:27 odesílatel Alvaro Herrera <alvherre@2ndquadrant.com> napsal: > On 2019-Apr-07, Pavel Stehule wrote: > > > ne 7. 4. 2019 v 19:16 odesílatel Alvaro Herrera < > alvherre@2ndquadrant.com> > > napsal: > > > > > Here. > > > > + 1 > > BTW I'm not sure if you noticed, but I removed the error message "no > partitioned relations found" when the result was empty. This was > mimicking \d behavior (which was justified on historical grounds), and > \drds behavior (which was justified on pattern ordering grounds); but I > see nothing that justifies it for \dP, so let's make it behave like all > the other commands and display an empty table. > > And there's an additional change to make. In the regression database, > this returns an empty table: > > regression=# \dPi regress_indexing.pk5_pkey > List of partitioned indexes > Esquema | Nombre | Dueño | On table > ---------+--------+-------+---------- > (0 filas) > > but the index does exist, and it is a partitioned one. So it should be > displayed. In fact, if I add the "n" flag, it shows: > > regression=# \dPin regress_indexing.pk5_pkey > List of partitioned indexes > Esquema | Nombre | Dueño | Parent name | > On table > > ------------------+----------+----------+--------------------------+---------------------- > regress_indexing | pk5_pkey | alvherre | regress_indexing.pk_pkey | > regress_indexing.pk5 > (1 fila) > > I think the fact that \dPi doesn't show it is broken, and this fixes it: > > @@ -3946,7 +3946,8 @@ listPartitionedTables(const char *reltypes, const > char *pattern, bool verbose) > appendPQExpBufferStr(&buf, "''"); /* dummy */ > appendPQExpBufferStr(&buf, ")\n"); > > - appendPQExpBufferStr(&buf, !showNested ? " AND NOT > c.relispartition\n" : "\n"); > + appendPQExpBufferStr(&buf, !showNested && !pattern ? > + " AND NOT > c.relispartition\n" : ""); > > if (!pattern) > appendPQExpBufferStr(&buf, " AND n.nspname <> > 'pg_catalog'\n" > > In order for this to display sanely, I added the "Parent name" column if > either the "n" flag is shown or a pattern is given (previously it only > appeared in the former case). > I am thinking about it and original behave and this new behave should be expected (and unexpected too). We can go this way - I have not counter-arguments, but yes, it is more consistent with some other commands, pattern disables some other constraints. It should be documented - Using any pattern in this case forces 'n' flag. > Note that this changes the expected output in the test; now every test > that uses a pattern displays *all* partitioned relations that match the > pattern, not just top-level ones. I'm about +0.2 convinced that this is > desirable, but my first example above tilts the scales to changing it as > described. > > I noticed this while testing after messing with the tab completion as > suggested by Justin: we already had Query_for_list_of_partitioned_tables > (which you could have used), but since \dP works for both indexes and > tables, I instead modified your new > Query_for_list_of_partitioned_relations to list both relation kinds. > > has sense. > -- > Álvaro Herrera https://www.2ndQuadrant.com/ > PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services >
-
Re: ToDo: show size of partitioned table
Alvaro Herrera <alvherre@2ndquadrant.com> — 2019-04-07T19:13:36Z
On 2019-Apr-07, Pavel Stehule wrote: > ne 7. 4. 2019 v 20:27 odesílatel Alvaro Herrera <alvherre@2ndquadrant.com> > napsal: > > In order for this to display sanely, I added the "Parent name" column if > > either the "n" flag is shown or a pattern is given (previously it only > > appeared in the former case). > > I am thinking about it and original behave and this new behave should be > expected (and unexpected too). We can go this way - I have not > counter-arguments, but yes, it is more consistent with some other commands, > pattern disables some other constraints. > > It should be documented - Using any pattern in this case forces 'n' flag. Added to the docs, and pushed. I couldn't resist tweaking the ORDER BY clause, too. I think listing all tables first, followed by all indexes, and sorting by parent in each category, is much easier to read. (Maybe this can use additional tweaking, but it's a minor thing anyway -- for example putting together all indexes that correspond to some particular table?) I noticed that \d never seems to use pg_total_relation_size, so toast size is never shown. I did likewise here too and used pg_table_size everywhere. I'm not 100% sure this is the most convenient thing. Maybe we need yet another column, and/or yet another flag ...? Also, I think the new \dP should gain a new flag (maybe "l") to make it list leaf tables/indexes too with their local sizes, and remove those from the standard \d listing. -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
-
Re: ToDo: show size of partitioned table
Justin Pryzby <pryzby@telsasoft.com> — 2019-04-07T21:25:25Z
On Sun, Apr 07, 2019 at 03:13:36PM -0400, Alvaro Herrera wrote: > On 2019-Apr-07, Pavel Stehule wrote: > > > ne 7. 4. 2019 v 20:27 odesílatel Alvaro Herrera <alvherre@2ndquadrant.com> > > napsal: > > > > In order for this to display sanely, I added the "Parent name" column if > > > either the "n" flag is shown or a pattern is given (previously it only > > > appeared in the former case). > > > > I am thinking about it and original behave and this new behave should be > > expected (and unexpected too). We can go this way - I have not > > counter-arguments, but yes, it is more consistent with some other commands, > > pattern disables some other constraints. > > > > It should be documented - Using any pattern in this case forces 'n' flag. > > Added to the docs, and pushed. Thanks for helping it across the finish line. I see you changed from relname to oid::regclass, which is good, thanks. Then, it's unnecessary to join against pg_class again (and I don't know why it was a left join) ? Also docs are wrong re: indices. Justin
-
Re: ToDo: show size of partitioned table
Justin Pryzby <pryzby@telsasoft.com> — 2019-04-07T21:28:29Z
PFA patch intended to have been previously attached... Hmm..maybe it should say | RELATION'S partitions is also displayed, along with the RELATION'S | description.
-
Re: ToDo: show size of partitioned table
Pavel Stehule <pavel.stehule@gmail.com> — 2019-04-08T03:59:10Z
ne 7. 4. 2019 v 21:13 odesílatel Alvaro Herrera <alvherre@2ndquadrant.com> napsal: > On 2019-Apr-07, Pavel Stehule wrote: > > > ne 7. 4. 2019 v 20:27 odesílatel Alvaro Herrera < > alvherre@2ndquadrant.com> > > napsal: > > > > In order for this to display sanely, I added the "Parent name" column > if > > > either the "n" flag is shown or a pattern is given (previously it only > > > appeared in the former case). > > > > I am thinking about it and original behave and this new behave should be > > expected (and unexpected too). We can go this way - I have not > > counter-arguments, but yes, it is more consistent with some other > commands, > > pattern disables some other constraints. > > > > It should be documented - Using any pattern in this case forces 'n' flag. > > Added to the docs, and pushed. > Thank you very much > I couldn't resist tweaking the ORDER BY clause, too. I think listing > all tables first, followed by all indexes, and sorting by parent in each > category, is much easier to read. (Maybe this can use additional > tweaking, but it's a minor thing anyway -- for example putting together > all indexes that correspond to some particular table?) > > I noticed that \d never seems to use pg_total_relation_size, so toast > size is never shown. I did likewise here too and used pg_table_size > everywhere. I'm not 100% sure this is the most convenient thing. Maybe > we need yet another column, and/or yet another flag ...? > I prefer some flag - both raw size and total size has sense, but another column will be less readable > > Also, I think the new \dP should gain a new flag (maybe "l") to make it > list leaf tables/indexes too with their local sizes, and remove those > from the standard \d listing. > +1 Pavel > > -- > Álvaro Herrera https://www.2ndQuadrant.com/ > PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services >
-
Re: ToDo: show size of partitioned table
Amit Langote <langote_amit_f8@lab.ntt.co.jp> — 2019-04-08T06:38:51Z
Hi, On 2019/04/08 12:59, Pavel Stehule wrote: > ne 7. 4. 2019 v 21:13 odesílatel Alvaro Herrera <alvherre@2ndquadrant.com> > napsal: >> Added to the docs, and pushed. >> > > Thank you very much Thank you Alvaro for pushing this to completion. Also, thank you Justin and Pavel for reviewing it till the last minute. Looks pretty good in my testing so far. >> I couldn't resist tweaking the ORDER BY clause, too. I think listing >> all tables first, followed by all indexes, and sorting by parent in each >> category, is much easier to read. (Maybe this can use additional >> tweaking, but it's a minor thing anyway -- for example putting together >> all indexes that correspond to some particular table?) >> >> I noticed that \d never seems to use pg_total_relation_size, so toast >> size is never shown. I did likewise here too and used pg_table_size >> everywhere. I'm not 100% sure this is the most convenient thing. Maybe >> we need yet another column, and/or yet another flag ...? >> > > I prefer some flag - both raw size and total size has sense, but another > column will be less readable I noticed that the size shown in the output of both \dP+ and \dPt+ does include toast relation sizes of (leaf) partitions, because pg_table_size(), which is used by all the queries that listPartitionedTables sends to the server, does: /* * heap size, including FSM and VM */ for (forkNum = 0; forkNum <= MAX_FORKNUM; forkNum++) size += calculate_relation_size(&(rel->rd_node), rel->rd_backend, forkNum); /* * Size of toast relation */ if (OidIsValid(rel->rd_rel->reltoastrelid)) size += calculate_toast_table_size(rel->rd_rel->reltoastrelid); >> Also, I think the new \dP should gain a new flag (maybe "l") to make it >> list leaf tables/indexes too with their local sizes, and remove those >> from the standard \d listing. > > +1 +1 to the idea. Perhaps we'd need to post this in a separate thread to highlight what may be a substantive change to \d's output? Thanks, Amit -
Re: ToDo: show size of partitioned table
Alvaro Herrera <alvherre@2ndquadrant.com> — 2019-04-08T17:37:54Z
On 2019-Apr-08, Amit Langote wrote: > I noticed that the size shown in the output of both \dP+ and \dPt+ does > include toast relation sizes of (leaf) partitions, because > pg_table_size(), which is used by all the queries that > listPartitionedTables sends to the server, does: > > /* > * heap size, including FSM and VM > */ > for (forkNum = 0; forkNum <= MAX_FORKNUM; forkNum++) > size += calculate_relation_size(&(rel->rd_node), rel->rd_backend, > forkNum); > > /* > * Size of toast relation > */ > if (OidIsValid(rel->rd_rel->reltoastrelid)) > size += calculate_toast_table_size(rel->rd_rel->reltoastrelid); Sigh. I must have mixed up whether pg_table_size did include toast size or not. Anyway, I think \dP should report the same thing as \d, which I think it's doing, so we're good there. > >> Also, I think the new \dP should gain a new flag (maybe "l") to make it > >> list leaf tables/indexes too with their local sizes, and remove those > >> from the standard \d listing. > > > > +1 > > +1 to the idea. Perhaps we'd need to post this in a separate thread to > highlight what may be a substantive change to \d's output? Aye aye. Not in a position to work on that right now; I might get to that in a few days if nobody beats me to it. (But then, I might not.) -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
-
Re: ToDo: show size of partitioned table
Amit Langote <langote_amit_f8@lab.ntt.co.jp> — 2019-04-09T06:25:15Z
On 2019/04/09 2:37, Alvaro Herrera wrote: > On 2019-Apr-08, Amit Langote wrote: > >> I noticed that the size shown in the output of both \dP+ and \dPt+ does >> include toast relation sizes of (leaf) partitions, because >> pg_table_size(), which is used by all the queries that >> listPartitionedTables sends to the server, does: >> >> /* >> * heap size, including FSM and VM >> */ >> for (forkNum = 0; forkNum <= MAX_FORKNUM; forkNum++) >> size += calculate_relation_size(&(rel->rd_node), rel->rd_backend, >> forkNum); >> >> /* >> * Size of toast relation >> */ >> if (OidIsValid(rel->rd_rel->reltoastrelid)) >> size += calculate_toast_table_size(rel->rd_rel->reltoastrelid); > > Sigh. I must have mixed up whether pg_table_size did include toast size > or not. Anyway, I think \dP should report the same thing as \d, which I > think it's doing, so we're good there. Yeah. >>>> Also, I think the new \dP should gain a new flag (maybe "l") to make it >>>> list leaf tables/indexes too with their local sizes, and remove those >>>> from the standard \d listing. >>> >>> +1 >> >> +1 to the idea. Perhaps we'd need to post this in a separate thread to >> highlight what may be a substantive change to \d's output? > > Aye aye. > > Not in a position to work on that right now; I might get to that in a > few days if nobody beats me to it. (But then, I might not.) Sure. I guess I also wanted to hear if you were of thinking it as PG 12 material or PG 13. Thanks, Amit
-
Re: ToDo: show size of partitioned table
Justin Pryzby <pryzby@telsasoft.com> — 2019-04-17T14:01:15Z
A reminder about these. Also, I suggest renaming "On Table" to "Table", for consistency with \di. Justin
-
Re: ToDo: show size of partitioned table
Alvaro Herrera <alvherre@2ndquadrant.com> — 2019-04-17T22:53:30Z
On 2019-Apr-17, Justin Pryzby wrote: > A reminder about these. > > Also, I suggest renaming "On Table" to "Table", for consistency with \di. Thanks! Pushed all three. Please feel free to add things like these to wiki.postgresql.org/wiki/Open_Items to serve as reminders for the project as a whole. -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services