ToDo: show size of partitioned table

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

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

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

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]: /messages/by-id/7dfc13c5-d6b7-1ff1-4bef-d75d6d2f76d9@lab.ntt.co.jp
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]/messages/by-id/495cec7e-f8d9-7e13-4807-90dbf4eec4ea@lab.ntt.co.jp.

Thanks,
Amit

[1]: /messages/by-id/7dfc13c5-d6b7-1ff1-4bef-d75d6d2f76d9@lab.ntt.co.jp
/messages/by-id/7dfc13c5-d6b7-1ff1-4bef-d75d6d2f76d9@lab.ntt.co.jp

[2]: /messages/by-id/495cec7e-f8d9-7e13-4807-90dbf4eec4ea@lab.ntt.co.jp
/messages/by-id/495cec7e-f8d9-7e13-4807-90dbf4eec4ea@lab.ntt.co.jp

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

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:

/messages/by-id/7a9c5328-5328-52a3-2a3d-bf1434b4dd1d@lab.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

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:

/messages/by-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

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:

/messages/by-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

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:

/messages/by-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

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

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

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

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)

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

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

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

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

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

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