Re: Handle infinite recursion in logical replication setup

Here are some more review comments for v15-0002 I found while working
through the documented examples. These are all for
doc/src/sgml/logical-replication.sgml. The section numbers (e.g.
31.11.1) are the assigned numbers after HTML rendering.

The general comment is that the sections appeared at first to be
independent of each other, but they are not really - sometimes
existing data and existing pub/sub are assumed to remain. Therefore, I
think some more text needs to be added to clarify the initial state
for each of these sections.

======

1. Section id.

<sect1 id="bidirectional-logical-replication">

IMO this section ID should be renamed
"logical-replication-bidirectional" then it will be more consistent
filenames with all the other logical replication section names.

~~~

2. Section 31.11.2. Adding a new node when there is no data in any of the nodes

2a.
This seems like a continuation of 31.11.1 because pub/sub of
node1,node2 is assumed to exist still.

2b.
The bottom of this section should say after these steps any subsequent
incremental data changes on any node will be replicated to all nodes
(node1, node2, node3)

~~~

3. Section 31.11.3. Adding a new node when data is present in the existing nodes

3a.
This seems to be a continuation of 31.11.1 because pub/sub (and
initial data) of node1/node2 is assumed to exist.

3b.

The bottom of this section should say after these steps that any
initial data of node1/node2 will be seen in node3, and any subsequent
incremental data changes on any node will be replicated to all nodes
(node1, node2, node3)

~~~

4. Section 31.11.4. Adding a new node when data is present in the new node

4a.
This seems to be a continuation of 31.11.1 because pub/sub (and
initial data) of node1/node2 is assumed to exist.

4b.
It is not made very clear up-front if the tables on node1 and node2
are empty or not. Apparently, they are considered NOT empty because
later in the example you are using "force" when you create the
subscription on node3.

4c.
The SQL wrapping here is strangely different from others (put the
subscription name on 1st line)
node3=# CREATE SUBSCRIPTION
node3-# sub_node3_node1 CONNECTION 'dbname=foo host=node1 user=repuser'
node3-# PUBLICATION pub_node1
node3-# WITH (copy_data = force, only_local = on);
CREATE SUBSCRIPTION

4d.
The SQL wrapping here is strangely different from others (put the
subscription name on 1st line)
node3=# CREATE SUBSCRIPTION
node3-# sub_node3_node2 CONNECTION 'dbname=foo host=node2 user=repuser'
node3-# PUBLICATION pub_node2
node3-# WITH (copy_data = off, only_local = on);
CREATE SUBSCRIPTION

4e.
The bottom of this section should say after this that any initial data
of node1/node2 will be seen in node3, and any subsequent incremental
data changes on any node will be replicated to all nodes (node1,
node2, node3)

~~~

5. Section 31.11.5. Generic steps to add a new node to the existing set of nodes

5a
Create subscriptions on the new node pointing to publication on the
first node with only_local option specified as on and copy_data option
specified as "force".

-> that should say "Create a subscription" (singular)

5b.
Create subscriptions on the new node pointing to publications on the
remaining node with only_local option specified as on and copy_data
option specified as off.

-> that should say "on the remaining node" (plural)

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