On Tue, Apr 26, 2022 at 11:32 AM osumi.takamichi@fujitsu.com
<osumi.takamichi@fujitsu.com> wrote:
>
> On Thursday, April 21, 2022 12:15 PM vignesh C <vignesh21@gmail.com> wrote:
> > Updated patch by changing the syntax to use EXCEPT instead of SKIP.
> Hi
>
>
> This is my review comments on the v2 patch.
>
> (1) gram.y
>
> I think we can make a unified function that merges
> preprocess_alltables_pubobj_list with check_except_in_pubobj_list.
>
> With regard to preprocess_alltables_pubobj_list,
> we don't use the 2nd argument "location" in this function.
Removed location and made a unified function.
> (2) create_publication.sgml
>
> + <para>
> + Create a publication that publishes all changes in all the tables except for
> + the changes of <structname>users</structname> and
> + <structname>departments</structname> table;
>
> This sentence should end ":" not ";".
Modified
> (3) publication.out & publication.sql
>
> +-- fail - can't set except table to schema publication
> +ALTER PUBLICATION testpub_forschema SET EXCEPT TABLE testpub_tbl1;
>
> There is one unnecessary space in the comment.
> Kindly change from "schema publication" to "schema publication".
Modified
> (4) pg_dump.c & describe.c
>
> In your first email of this thread, you explained this feature
> is for PG16. Don't we need additional branch for PG16 ?
>
> @@ -6322,6 +6328,21 @@ describePublications(const char *pattern)
> }
> }
>
> + if (pset.sversion >= 150000)
> + {
>
>
> @@ -4162,7 +4164,7 @@ getPublicationTables(Archive *fout, TableInfo tblinfo[], int numTables)
> /* Collect all publication membership info. */
> if (fout->remoteVersion >= 150000)
> appendPQExpBufferStr(query,
> - "SELECT tableoid, oid, prpubid, prrelid, "
> + "SELECT tableoid, oid, prpubid, prrelid, prexcept,"
>
Modified by adding a comment saying "FIXME: 150000 should be changed
to 160000 later for PG16."
> (5) psql-ref.sgml
>
> + If <literal>+</literal> is appended to the command name, the tables,
> + except tables and schemas associated with each publication are shown as
> + well.
>
> I'm not sure if "except tables" is a good description.
> I suggest "excluded tables". This applies to the entire patch,
> in case if this is reasonable suggestion.
Modified it in most of the places where it was applicable. I felt the
usage was ok in a few places.
Thanks for the comments, the attached v3 patch has the changes for the same.
Regards.
Vignesh
