Hi, Here are my review comments for patch v2.
======
1. doc/src/sgml/logical-replication.sgml
Candidate indexes must be btree, non-partial, and have at least one
column reference to the published table column at the leftmost column
(i.e. cannot consist of only expressions).
~
There is only one column which can be the leftmost one, so the wording
"At least one ... at the leftmost" seemed a bit strange to me.
Personally, I would phrase it something like below:
SUGGESTION #1
Candidate indexes must be btree, non-partial, and the leftmost index
column must reference a published table column (i.e. the index cannot
consist of only expressions).
SUGGESTION #2 (same as above, but omitting the "only expressions"
part, which I think is implied by the "leftmost" rule anyway)
Candidate indexes must be btree, non-partial, and the leftmost index
column must reference a published table column.
======
2. src/backend/replication/logical/relation.c
* Returns the oid of an index that can be used by the apply worker to scan
* the relation. The index must be btree, non-partial, and have at least
- * one column reference (i.e. cannot consist of only expressions). These
- * limitations help to keep the index scan similar to PK/RI index scans.
+ * one column reference to the remote relation's column at the leftmost column
+ * (i.e. cannot consist of only expressions). These limitations help
to keep the
+ * index scan similar to PK/RI index scans.
This comment text is similar to the docs change, so refer to the same
suggestions as #1 above.
------
Kind Regards,
Peter Smith.
Fujitsu Australia