[patch] Have psql's \d+ indicate foreign partitions

On Mon, Oct 24, 2022 at 09:44:18PM +0900, Ian Lawrence Barwick wrote:

Recently I have been working a lot with partitioned tables which contain a mix
of local and foreign partitions, and find it would be very useful to be able to
easily obtain an overview of which partitions are foreign and where they are
located.

Partitions: parttest_10_0 FOR VALUES WITH (modulus 10, remainder 0),
parttest_10_1 FOR VALUES WITH (modulus 10, remainder 1), server: "fdw_node2",

which is much more informative, albeit a little more cluttered, but

@@ -3445,6 +3451,10 @@ describeOneTableDetails(const char *schemaname,
if (child_relkind == RELKIND_PARTITIONED_TABLE ||
child_relkind == RELKIND_PARTITIONED_INDEX)
appendPQExpBufferStr(&buf, ", PARTITIONED");
+				else if (child_relkind == RELKIND_FOREIGN_TABLE && is_partitioned)
+					appendPQExpBuffer(&buf, ", server: \"%s\"", PQgetvalue(result, i, 4));
+				else if (child_relkind == RELKIND_FOREIGN_TABLE && !is_partitioned)
+					appendPQExpBuffer(&buf, " (server: \"%s\")", PQgetvalue(result, i, 4));
if (strcmp(PQgetvalue(result, i, 2), "t") == 0)
appendPQExpBufferStr(&buf, " (DETACH PENDING)");
if (i < tuples - 1)

To avoid the clutter that you mentioned, I suggest that this should show
that the table *is* foreign, but without the server - if you want to
know the server (or its options), you can run another \d command for
that (or run a SQL query).

That's similar to what's shown if the child is partitioned: a suffix
like ", PARTITIONED", but without show the partition strategy.

I had a patch to allow \d++, and maybe showing the foreign server would
be reasonable for that. But the patch got closed, evidently lack of
interest.

On 2022-Oct-24, Justin Pryzby wrote:

On Mon, Oct 24, 2022 at 09:44:18PM +0900, Ian Lawrence Barwick wrote:

+				else if (child_relkind == RELKIND_FOREIGN_TABLE && is_partitioned)
+					appendPQExpBuffer(&buf, ", server: \"%s\"", PQgetvalue(result, i, 4));

To avoid the clutter that you mentioned, I suggest that this should show
that the table *is* foreign, but without the server - if you want to
know the server (or its options), you can run another \d command for
that (or run a SQL query).

But 'server "%s"' is not much longer than "foreign", and it's not like
your saving any vertical space at all (you're just using space that
would otherwise be empty), so I'm not sure it is better. I would vote
for showing the server.

--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
"You don't solve a bad join with SELECT DISTINCT" #CupsOfFail
https://twitter.com/connor_mc_d/status/1431240081726115845

2022年10月27日(木) 16:12 Alvaro Herrera <alvherre@alvh.no-ip.org>:

On 2022-Oct-24, Justin Pryzby wrote:

On Mon, Oct 24, 2022 at 09:44:18PM +0900, Ian Lawrence Barwick wrote:

+                           else if (child_relkind == RELKIND_FOREIGN_TABLE && is_partitioned)
+                                   appendPQExpBuffer(&buf, ", server: \"%s\"", PQgetvalue(result, i, 4));

To avoid the clutter that you mentioned, I suggest that this should show
that the table *is* foreign, but without the server - if you want to
know the server (or its options), you can run another \d command for
that (or run a SQL query).

But 'server "%s"' is not much longer than "foreign", and it's not like
your saving any vertical space at all (you're just using space that
would otherwise be empty), so I'm not sure it is better. I would vote
for showing the server.

Indeed; my particular use-case is being able to see how the (foreign) tablesare
distributed over one or more foreign servers, so while being able to see whether
it's a foreign table or not helps, it's not all that much more disruptive to
include the identity of the server (unless the server's name is maxing out
NAMEDATALEN, dunno how prevalent that is in the wild, but it's not something
I've ever felt the need to do).

Regards

Ian Barwick

On Mon, Oct 24, 2022 at 09:44:18PM +0900, Ian Lawrence Barwick wrote:

Recently I have been working a lot with partitioned tables which contain a mix
of local and foreign partitions, and find it would be very useful to be able to
easily obtain an overview of which partitions are foreign and where they are
located.

Currently, executing "\d+" on a partitioned table lists the partitions
like this:

Hmm. I am not sure that we should add this much amount of
information, particularly for the server bits. First, worth
mentioning, pg_partition_tree() is very handy when it comes to know
partition information, like:
SELECT relid, relkind
FROM pg_partition_tree('parttest') p, pg_class c
where c.oid = p.relid;

Anyway, saying that, we do something similar for partitioned indexes
and tables with \d+, aka around L3445:
if (child_relkind == RELKIND_PARTITIONED_TABLE ||
child_relkind == RELKIND_PARTITIONED_INDEX)
appendPQExpBufferStr(&buf, ", PARTITIONED");

This is the same, just for a new relkind.
--
Michael

Michael Paquier <michael@paquier.xyz> writes:

On Mon, Oct 24, 2022 at 09:44:18PM +0900, Ian Lawrence Barwick wrote:

Recently I have been working a lot with partitioned tables which contain a mix
of local and foreign partitions, and find it would be very useful to be able to
easily obtain an overview of which partitions are foreign and where they are
located.

Hmm. I am not sure that we should add this much amount of
information, particularly for the server bits.

FWIW, I am also in favor of adding ", FOREIGN" but no more.
My concern is that as submitted, the patch greatly increases
the cost of the underlying query by adding two more catalogs
to the join. I don't think imposing such a cost on everybody
(whether they use foreign partitions or not) is worth that. But
we can add ", FOREIGN" for free since we have the relkind anyway.

regards, tom lane

2022年11月6日(日) 1:39 Tom Lane <tgl@sss.pgh.pa.us>:

Michael Paquier <michael@paquier.xyz> writes:

On Mon, Oct 24, 2022 at 09:44:18PM +0900, Ian Lawrence Barwick wrote:

Recently I have been working a lot with partitioned tables which contain a mix
of local and foreign partitions, and find it would be very useful to be able to
easily obtain an overview of which partitions are foreign and where they are
located.

Hmm. I am not sure that we should add this much amount of
information, particularly for the server bits.

FWIW, I am also in favor of adding ", FOREIGN" but no more.
My concern is that as submitted, the patch greatly increases
the cost of the underlying query by adding two more catalogs
to the join. I don't think imposing such a cost on everybody
(whether they use foreign partitions or not) is worth that. But
we can add ", FOREIGN" for free since we have the relkind anyway.

Fair enough, make sense.

Revised version added per suggestions, which produces output like this:

postgres=# \d+ parttest
Partitioned table "public.parttest"
Column | Type | Collation | Nullable | Default | Storage |
Compression | Stats target | Description
--------+---------+-----------+----------+---------+----------+-------------+--------------+-------------
id | integer | | not null | | plain |
| |
val1 | text | | | | extended |
| |
val2 | text | | | | extended |
| |
Partition key: HASH (id)
Partitions: parttest_10_0 FOR VALUES WITH (modulus 10, remainder 0),
parttest_10_1 FOR VALUES WITH (modulus 10, remainder
1), FOREIGN,
parttest_10_2 FOR VALUES WITH (modulus 10, remainder 2),
parttest_10_3 FOR VALUES WITH (modulus 10, remainder
3), FOREIGN,
parttest_10_4 FOR VALUES WITH (modulus 10, remainder 4),
parttest_10_5 FOR VALUES WITH (modulus 10, remainder
5), FOREIGN,
parttest_10_6 FOR VALUES WITH (modulus 10, remainder 6),
parttest_10_7 FOR VALUES WITH (modulus 10, remainder
7), FOREIGN,
parttest_10_8 FOR VALUES WITH (modulus 10, remainder 8),
parttest_10_9 FOR VALUES WITH (modulus 10, remainder 9), FOREIGN

Regards

Ian Barwick

On Sun, Nov 06, 2022 at 09:23:01PM +0900, Ian Lawrence Barwick wrote:

Fair enough, make sense.

Fine by me and the patch looks OK. I'd like to apply this if there
are no objections.
--
Michael

Michael Paquier <michael@paquier.xyz> writes:

On Sun, Nov 06, 2022 at 09:23:01PM +0900, Ian Lawrence Barwick wrote:

Fair enough, make sense.

Fine by me and the patch looks OK. I'd like to apply this if there
are no objections.

WFM.

regards, tom lane

On Mon, Nov 07, 2022 at 01:43:22AM -0500, Tom Lane wrote:

WFM.

Okay, applied as bd95816, then.
--
Michael

2022年11月8日(火) 14:49 Michael Paquier <michael@paquier.xyz>:

On Mon, Nov 07, 2022 at 01:43:22AM -0500, Tom Lane wrote:

WFM.

Okay, applied as bd95816, then.

Thanks!

CF entry updated accordingly.

Regards

Ian Barwick

On Tue, Nov 08, 2022 at 03:38:22PM +0900, Ian Lawrence Barwick wrote:

CF entry updated accordingly.

Missed this part, thanks..
--
Michael