Some partial notes on 0005-Add-logical-replication-workers.patch:
Documentation still says that TRUNCATE is supported.
In catalogs.sgml for pg_subscription column subpublications I'd add a
note that those are publications that live on the remote server.
Otherwise one might think by mistake that it references pg_publication.
The changes in reference.sgml should go into an earlier patch.
Document that table and column names are matched by name. (This seems
obvious, but it's not explained anywhere, AFAICT.)
Document to what extent other relation types are supported (e.g.,
materialized views as source, view or foreign table or temp table as
target). Suggest an updatable view as target if user wants to have
different table names or write into a different table structure.
subscriptioncmds.c: In CreateSubscription(), the
CommandCounterIncrement() call is apparently not needed.
subscriptioncmds.c: Duplicative code for libpqwalreceiver loading and
init, should be refactored.
subscriptioncmds.c: Perhaps combine logicalrep_worker_find() and
logicalrep_worker_stop() into one call that also encapsulates the
required locking.
001_rep_changes.pl: The TAP protocol does not allow direct printing to
stdout. (It needs to be prefixed with # or with spaces or something; I
forget.) In this case, the print calls can just be removed, because the
following is() calls in each case will print the failing value anyway.
In get_subscription_list(), the memory context pointers don't appear to
do anything useful, because everything ends up being CurrentMemoryContext.
pg_stat_get_subscription(NULL) for "all" seems a bit of a weird interface.
pglogical_apply_main not used, should be removed.
In logicalreprel_open(), the error message "cache lookup failed for
remote relation %u" could be clarified. This message could probably
happen if the protocol did not send a Relation message first. (The term
"cache" is perhaps inappropriate for LogicalRepRelMap, because it
implies that the value can be gotten from elsewhere if it's not in the
cache. In this case it's really session state that cannot be recovered
easily.)
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
