Re: bogus: logical replication rows/cols combinations

From: Justin Pryzby <pryzby(at)telsasoft(dot)com>
To: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
Cc: Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, houzj(dot)fnst(at)fujitsu(dot)com, Tomas Vondra <tomas(dot)vondra(at)enterprisedb(dot)com>, Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: bogus: logical replication rows/cols combinations
Date: 2022-05-19 12:07:24
Message-ID: 20220519120724.GO19626@telsasoft.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, May 19, 2022 at 10:33:13AM +0530, Amit Kapila wrote:
> I have committed the first patch after fixing this part. It seems Tom
> is not very happy doing this after beta-1 [1]. The reason we get this
> information via this view (and underlying function) is that it
> simplifies the queries on the subscriber-side as you can see in the
> second patch. The query change is as below:
> [1] - https://www.postgresql.org/message-id/91075.1652929852%40sss.pgh.pa.us

I think Tom's concern is that adding information to a view seems like adding a
feature that hadn't previously been contemplated.
(Catalog changes themselves are not prohibited during the beta period).

> a. Revert the change in view (and underlying function) as done in
> commit 0ff20288e1 and consider the alternate way (using a slightly
> complex query) to fix. Then maybe for PG-16, we can simplify it by
> changing the underlying function and view.

But, ISTM that it makes no sense to do it differently for v15 just to avoid the
appearance of adding a new feature, only to re-do it in 2 weeks for v16...
So (from a passive observer) +0.1 to keep the current patch.

I have some minor language fixes to that patch.

diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index d96c72e5310..82aa84e96e1 100644
--- a/doc/src/sgml/catalogs.sgml
+++ b/doc/src/sgml/catalogs.sgml
@@ -9691,7 +9691,7 @@ SCRAM-SHA-256$<replaceable>&lt;iteration count&gt;</replaceable>:<replaceable>&l

<row>
<entry><link linkend="view-pg-publication-tables"><structname>pg_publication_tables</structname></link></entry>
- <entry>publications and information of their associated tables</entry>
+ <entry>publications and information about their associated tables</entry>
</row>

<row>
@@ -11635,7 +11635,7 @@ SELECT * FROM pg_locks pl LEFT JOIN pg_prepared_xacts ppx

<para>
The view <structname>pg_publication_tables</structname> provides
- information about the mapping between publications and information of
+ information about the mapping between publications and information about
tables they contain. Unlike the underlying catalog
<link linkend="catalog-pg-publication-rel"><structname>pg_publication_rel</structname></link>,
this view expands publications defined as <literal>FOR ALL TABLES</literal>
@@ -11695,7 +11695,7 @@ SELECT * FROM pg_locks pl LEFT JOIN pg_prepared_xacts ppx
</para>
<para>
Names of table columns included in the publication. This contains all
- the columns of the table when the user didn't specify the column list
+ the columns of the table when the user didn't specify a column list
for the table.
</para></entry>
</row>
diff --git a/src/backend/catalog/pg_publication.c b/src/backend/catalog/pg_publication.c
index 8c7fca62de3..2f706f638ce 100644
--- a/src/backend/catalog/pg_publication.c
+++ b/src/backend/catalog/pg_publication.c
@@ -1077,7 +1077,7 @@ get_publication_name(Oid pubid, bool missing_ok)
}

/*
- * Returns information of tables in a publication.
+ * Returns information about tables in a publication.
*/
Datum
pg_get_publication_tables(PG_FUNCTION_ARGS)
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 87aa571a331..86f13293090 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -11673,7 +11673,7 @@
prosrc => 'pg_show_replication_origin_status' },

# publications
-{ oid => '6119', descr => 'get information of tables in a publication',
+{ oid => '6119', descr => 'get information about tables in a publication',
proname => 'pg_get_publication_tables', prorows => '1000', proretset => 't',
provolatile => 's', prorettype => 'record', proargtypes => 'text',
proallargtypes => '{text,oid,int2vector,pg_node_tree}', proargmodes => '{i,o,o,o}',

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Justin Pryzby 2022-05-19 12:19:05 Re: JSON Functions and Operators Docs for v15
Previous Message Ronan Dunklau 2022-05-19 11:47:36 Re: Removing unneeded self joins