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