Re: [patch] Have psql's \d+ indicate foreign partitions
On Tue, Nov 08, 2022 at 03:38:22PM +0900, Ian Lawrence Barwick wrote: > CF entry updated accordingly. Missed this part, thanks.. -- Michael signature.asc Description: PGP signature
Re: [patch] Have psql's \d+ indicate foreign partitions
2022年11月8日(火) 14:49 Michael Paquier : > > 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
Re: [patch] Have psql's \d+ indicate foreign partitions
On Mon, Nov 07, 2022 at 01:43:22AM -0500, Tom Lane wrote: > WFM. Okay, applied as bd95816, then. -- Michael signature.asc Description: PGP signature
Re: [patch] Have psql's \d+ indicate foreign partitions
Michael Paquier 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
Re: [patch] Have psql's \d+ indicate foreign partitions
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 signature.asc Description: PGP signature
Re: [patch] Have psql's \d+ indicate foreign partitions
2022年11月6日(日) 1:39 Tom Lane : > > Michael Paquier 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 commit 0b330a67e5941bacb815fa6dfae914c56563f7a9 Author: Ian Barwick Date: Sun Nov 6 21:08:26 2022 +0900 psql: in \d+, indicate foreign partitions Currently with a partitioned table, \d+ lists the partitions and their partition key, but it would be useful to see which ones, if any, are foreign partitions. diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c index c645d66418..2eae519b1d 100644 --- a/src/bin/psql/describe.c +++ b/src/bin/psql/describe.c @@ -3445,6 +3445,8 @@ 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) + appendPQExpBufferStr(&buf, ", FOREIGN"); if (strcmp(PQgetvalue(result, i, 2), "t") == 0) appendPQExpBufferStr(&buf, " (DETACH PENDING)"); if (i < tuples - 1) diff --git a/src/test/regress/expected/foreign_data.out b/src/test/regress/expected/foreign_data.out index 9d7610b948..47bf56adbf 100644 --- a/src/test/regress/expected/foreign_data.out +++ b/src/test/regress/expected/foreign_data.out @@ -1404,7 +1404,7 @@ CREATE FOREIGN TABLE ft2 () INHERITS (fd_pt1) c1 | integer | | not null | | plain| | c2 | text| | | | extended | | c3 | date| | | | plain| | -Child tables: ft2 +Child tables: ft2, FOREIGN \d+ ft2 Foreign table "public.ft2" @@ -1449,7 +1449,7 @@ ALTER FOREIGN TABLE ft2 INHERIT fd_pt1; c1 | integer | | not null | | plain| | c2 | text| | | | extended | | c3 | date| | | | plain| | -Child tables: ft2 +Child tables: ft2, FOREIGN \d+ ft2 Foreign table "public.ft2" @@ -1483,7 +1483,7 @@ Server: s0 FDW options: (delimiter ',', quote '"', "be quoted" 'value') Inherits: fd_pt1 Child tables: ct3, - ft3 + ft3, FOREIGN \d+ ct3 Table "public.ct3" @@ -1522,7 +1522,7 @@ ALTER TABLE fd_pt1 ADD COLUMN c8 integer; c6 | integer | | | | plain| | c7 | integer | | not null | | plain| | c8 | integer |
Re: [patch] Have psql's \d+ indicate foreign partitions
Michael Paquier 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
Re: [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.
>
> 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
signature.asc
Description: PGP signature
Re: [patch] Have psql's \d+ indicate foreign partitions
2022年10月27日(木) 16:12 Alvaro Herrera : > > 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
Re: [patch] Have psql's \d+ indicate foreign partitions
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 HerreraBreisgau, Deutschland — https://www.EnterpriseDB.com/ "You don't solve a bad join with SELECT DISTINCT" #CupsOfFail https://twitter.com/connor_mc_d/status/1431240081726115845
Re: [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.
