At Fri, 29 May 2020 16:21:38 +0900, Michael Paquier <mich...@paquier.xyz> wrote in > On Thu, May 28, 2020 at 06:11:39PM +0900, Kyotaro Horiguchi wrote: > > Mmm. It is not the proper way to use physical replication and it's > > totally accidental that that worked (or even it might be a bug). The > > documentation is saying as the follows, as more-or-less the same for > > all versions since 9.4. > > > > https://www.postgresql.org/docs/13/protocol-replication.html > > + if (am_db_walsender) > + ereport(ERROR, > + (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), > + errmsg("cannot initiate physical > replication on a logical replication connection"))); > > I don't agree with this change. The only restriction that we have in > place now in walsender.c regarding MyDatabaseId not being set is to > prevent the execution of SQL commands. Note that it is possible to > start physical replication even if MyDatabaseId is set in a > replication connection, so you could break cases that have been valid > until now.
It donesn't check MyDatabase, but whether the connection parameter "repliation" is "true" or "database". The documentation is telling that "replication" should be "true" for a connection that is to be used for physical replication, and "replication" should literally be "database" for a connection that is for logical replication. We need to revise the documentation if we are going to allow physical replication on a conection with "replication = database". > I think that we actually should be much more careful with the > initialization of the WAL reader used in the context of a WAL sender > before calling WALRead() and attempting to read a new WAL page. I agree that the initialization can be improved, but the current code is no problem if we don't allow to run both logical and physical replication on a single session. regards. -- Kyotaro Horiguchi NTT Open Source Software Center
