Re: Added schema level support for publication.

On Mon, Sep 13, 2021 at 7:06 PM tanghy(dot)fnst(at)fujitsu(dot)com
<tanghy(dot)fnst(at)fujitsu(dot)com> wrote:
>
> On Sunday, September 12, 2021 11:13 PM vignesh C <vignesh21(at)gmail(dot)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

Modified

> 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:

Modified

> 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:

Modified

> 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.

I felt this is ok, as both single space and double space are used at
various places.

> 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'.

Modified.

> 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"

Modified

I have fixed the comments in the v28 patch attached at [1].
[1] - https://www.postgresql.org/message-id/CALDaNm0OudeDeFN7bSWPro0hgKx%3D1zPgcNFWnvU_G6w3mDPX0Q%40mail.gmail.com

Regards,
Vignesh