Re: Logical Replication WIP

Review of 0004-Make-libpqwalreceiver-reentrant.patch:

This looks like a good change.

typo: _PG_walreceirver_conn_init

For libpqrcv_create_slot(), slotname should be const char *.
Similarly, for slotname in libpqrcv_startstreaming*() and conninfo in
libpqrcv_connect(). (the latter two pre-existing)

The connection handle should record in libpqrcv_connect() whether a
connection is a logical or physical replication stream. Then that
parameter doesn't have to be passed around later (or at least some
asserts could double-check it).

In libpqrcv_connect(), the new argument connname is actually just the
application name, for which in later patches the subscription name is
passed in. Does this have a deeper meaning, or should we call the
argument appname to avoid introducing another term?

New function libpqrcv_create_slot(): Hardcoded cmd length (hmm, other
functions do that too), should used StringInfo. ereport instead of
elog. No newline at the end of error message, since PQerrorMessage()
already supplies it. Typo "could not crate". Briefly document return
value.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services