Re: Added schema level support for publication.

On Mon, Sep 13, 2021 at 7:06 PM tanghy.fnst@fujitsu.com
<tanghy.fnst@fujitsu.com> wrote:
>
> 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

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