RE: Added schema level support for publication. - Mailing list pgsql-hackers

On Sunday, September 12, 2021 11:13 PM vignesh C <vignesh21@gmail.com> wrote:
> 
> Thanks for the changes, the suggested changes make the parsing code
> simpler. I have merged the changes to the main patch. Attached v27
> patch has the changes for the same.
> 

Thanks for your new patch. I had a look at the changes related to document and tried
the patch. Here are several comments.

1.
   <para>
    You must own the publication to use <command>ALTER PUBLICATION</command>.
    Adding a table to a publication additionally requires owning that table.
+   The <literal>ADD SCHEMA</literal> and <literal>SET SCHEMA</literal> to a
+   publication requires the invoking user to be a superuser.
    To alter the owner, you must also be a direct or indirect member of the new
    owning role. The new owner must have <literal>CREATE</literal> privilege on
    the database.  Also, the new owner of a <literal>FOR ALL TABLES</literal>
    publication must be a superuser.  However, a superuser can change the
    ownership of a publication regardless of these restrictions.
   </para>

ADD SCHEMA
->
ADD ALL TABLES IN SCHEMA

SET SCHEMA
->
SET ALL TABLES IN SCHEMA

2.
+  <para>
+   ADD some tables and schemas to the publication:
+<programlisting>
+ALTER PUBLICATION production_publication ADD TABLE users, departments, ALL TABLES IN SCHEMA production;
+</programlisting>
+  </para>

ADD some tables and schemas to the publication:
->
Add some tables and schemas to the publication:

3.
+  <para>
+   Drop some schema from the publication:
+<programlisting>
+ALTER PUBLICATION production_quarterly_publication DROP ALL TABLES IN SCHEMA production_july;
+</programlisting>
+  </para>

Drop some schema from the publication:
->
Drop some schemas from the publication:

4.
+   The catalog <structname>pg_publication_namespace</structname> contains the
+   mapping between schemas and publications in the database.  This is a
+   many-to-many mapping.  

There are two Spaces at the end of the paragraph.

5.
Adding a table and the schema where the table belonged to is not supported. But
it didn't report error message when I try to add them in the same statement by
using 'ALTER PUBLICATION'.

For example:
postgres=# create publication pub;
CREATE PUBLICATION
postgres=# alter publication pub add  all tables in schema s1, table s1.tbl;
ALTER PUBLICATION
postgres=# \dRp+
                              Publication pub
  Owner   | All tables | Inserts | Updates | Deletes | Truncates | Via root
----------+------------+---------+---------+---------+-----------+----------
 postgres | f          | t       | t       | t       | t         | f
Tables:
    "s1.tbl"
Tables from schemas:
    "s1"

It didn't check if table 's1.tbl' is member of schema 's1'.


6.
I think if I use 'ALTER PUBLICATION ... SET', both the list of tables and the
list of all tables in schemas should be reset. The publication should only
contain the tables and all tables in schemas which user specified. If user only
specified all tables in schema, and didn't specify tables, the tables which used
to be part of the publication should be dropped, too. But currently, if I didn't
specify tables, the list of tables wouldn't be set to empty. Thoughts?

For example:
postgres=# create publication pub for table s1.tbl;
CREATE PUBLICATION
postgres=# alter publication pub set all tables in schema s2;
ALTER PUBLICATION
postgres=# \dRp+
                              Publication pub
  Owner   | All tables | Inserts | Updates | Deletes | Truncates | Via root
----------+------------+---------+---------+---------+-----------+----------
 postgres | f          | t       | t       | t       | t         | f
Tables:
    "s1.tbl"
Tables from schemas:
    "s2"

Regards
Tang