Re: Skipping schema changes in publication

Hi Shlok,

Here are a few ad-hoc comments for patch v27-0002.

======
doc/src/sgml/logical-replication.sgml

1.
<para>
- To add tables to a publication, the user must have ownership rights on the
- table. To add all tables in schema to a publication, the user must be a
- superuser. To create a publication that publishes all tables, all tables in
- schema, or all sequences automatically, the user must be a superuser.
+ To create a publication using <literal>FOR ALL TABLES</literal>,
+ <literal>FOR ALL SEQUENCES or <literal>FOR TABLES IN SCHEMA</literal>, the
+ user must be a superuser. To add <literal>ALL TABLES</literal> or
+ <literal>TABLES IN SCHEMA</literal> to a publication, the user must be a
+ superuser. To add tables to a publication, the user must have ownership
+ rights on the table.
</para>

Typo: Mismatched tags. "<literal>FOR ALL SEQUENCES or <literal>FOR
TABLES IN SCHEMA</literal>"

(The docs in v27-0002 cannot build because of this error)

======
src/backend/commands/publicationcmds.c

CheckPublicationDefValues:

2.
+/*
+ * Check if the publication has default values.
+ *
+ * Returns true if the publication satisfies all the following conditions:
+ * a) Publication is not set with "FOR ALL TABLES"
+ * b) Publication is having default publication parameter values
+ * c) Publication is not associated with schemas
+ * d) Publication is not associated with relations
+ */
+static bool
+CheckPublicationDefValues(HeapTuple tup)

Should a) also say "FOR ALL SEQUENCES"?

~~~

3.
What about checking defaults of other publication parameters, e.g.
publish_via_parttion_root and publish_generated_columns?

======
src/backend/parser/gram.y

4.
* TABLE table_name [, ...]
* TABLES IN SCHEMA schema_name [, ...]
*
+ * ALTER PUBLICATION name ADD ALL TABLES EXCEPT [TABLE] (table_name [, ...])
+ *
* ALTER PUBLICATION name RESET

The EXCEPT clause part should be optional.

======
src/bin/pg_dump/t/002_pg_dump.pl

5.
#
# Either "all_runs" should be set or there should be a "like" list,
# even if it is empty. (This makes the test more self-documenting.)
- if (!defined($tests{$test}->{all_runs})
+ if ( !defined($tests{$test}->{all_runs})

Is this just an accidental whitespace change?

======
Kind Regards,
Peter Smith.
Fujitsu Australia