Horton

RE: Skipping schema changes in publication

Yu Shi (Fujitsu) <shiy.fnst@fujitsu.com>

View thread
From: "shiy.fnst@fujitsu.com" <shiy.fnst@fujitsu.com>
To: vignesh C <vignesh21@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2022-04-14T08:55:01Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Fix miscellaneous issues in EXCEPT publication clause.

  2. Change syntax of EXCEPT TABLE clause in publication commands.

  3. Add support for EXCEPT TABLE in ALTER PUBLICATION.

  4. Allow table exclusions in publications via EXCEPT TABLE.

  5. Add wait_for_subscription_sync for TAP tests.

On Tue, Apr 12, 2022 2:23 PM vignesh C <vignesh21@gmail.com> wrote:
> 
> The patch does not apply on top of HEAD because of the recent commit,
> attached patch is rebased on top of HEAD.
> 

Thanks for your patch. Here are some comments for 0001 patch.

1. doc/src/sgml/catalogs.sgml
@@ -6438,6 +6438,15 @@ SCRAM-SHA-256$<replaceable>&lt;iteration count&gt;</replaceable>:<replaceable>&l
        A null value indicates that all columns are published.
       </para></entry>
      </row>
+
+    <row>
+      <entry role="catalog_table_entry"><para role="column_definition">
+       <structfield>pnskip</structfield> <type>bool</type>
+      </para>
+      <para>
+       True if the schema is skip schema
+      </para></entry>
+     </row>
     </tbody>
    </tgroup>
   </table>

This change is added to pg_publication_rel, I think it should be added to
pg_publication_namespace, right?

2.
postgres=# alter publication p1 add skip all tables in schema s1,s2;
ERROR:  schema "s1" is already member of publication "p1"

This error message seems odd to me, can we improve it? Something like:
schema "s1" is already skipped in publication "p1"

3.
create table tbl (a int primary key);
create schema s1;
create schema s2;
create table s1.tbl (a int);
create publication p1 for all tables skip all tables in schema s1,s2;

postgres=# \dRp+
                               Publication p1
  Owner   | All tables | Inserts | Updates | Deletes | Truncates | Via root
----------+------------+---------+---------+---------+-----------+----------
 postgres | t          | t       | t       | t       | t         | f
Skip tables from schemas:
    "s1"
    "s2"

postgres=# select * from pg_publication_tables;
 pubname | schemaname | tablename
---------+------------+-----------
 p1      | public     | tbl
 p1      | s1         | tbl
(2 rows)

There shouldn't be a record of s1.tbl, since all tables in schema s1 are skipped.

I found that it is caused by the following code:

src/backend/catalog/pg_publication.c
+	foreach(cell, pubschemalist)
+	{
+		PublicationSchInfo *pubsch = (PublicationSchInfo *) lfirst(cell);
+
+		skipschemaidlist = lappend_oid(result, pubsch->oid);
+	}

The first argument to append_oid() seems wrong, should it be:

skipschemaidlist = lappend_oid(skipschemaidlist, pubsch->oid);


4. src/backend/commands/publicationcmds.c

/*
 * Convert the PublicationObjSpecType list into schema oid list and
 * PublicationTable list.
 */
static void
ObjectsInPublicationToOids(List *pubobjspec_list, ParseState *pstate,
						   List **rels, List **schemas)

Should we modify the comment of ObjectsInPublicationToOids()?
"schema oid list" should be "PublicationSchInfo list".

Regards,
Shi yu