RE: Skipping schema changes in publication

On Tue, Apr 12, 2022 2:23 PM vignesh C <vignesh21(at)gmail(dot)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