On Sun, Oct 26, 2025 at 5:54 PM Peter Smith <[email protected]> wrote: > > Hi Sawada. > > A couple of comments for v22-0001. > > ====== > > 1. > + /* > + * We don't need this warning message when wal_level >= 'replica' since > + * logical decoding is automatically enabled up on a logical slot > + * creation. > + */ > + if (wal_level == WAL_LEVEL_MINIMAL) > ereport(WARNING, > (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), > - errmsg("\"wal_level\" is insufficient to publish logical changes"), > - errhint("Set \"wal_level\" to \"logical\" before creating > subscriptions."))); > + errmsg("logical decoding must be enabled to publish logical changes"), > + errhint("Before creating subscriptions, set \"wal_level\" >= > \"logical\" or create a logical replication slot when \"wal_level\" = > \"replica\"."))); > > 1a. > Sorry, for repeating the same question about the HINT message, but > AFAICT I did not yet notice any reply to directly reject or address my > question. > > Basically, I felt the errhint part that says "...or create a logical > replication slot when wal_level = replica." is overkill, because > creating the subscription will do that anyway. > In other words, we don't need to tell the user this is needed "Before > creating subscriptions". So, I thought the only requirement to be able > to publish a subscription is for wal_level >= replica. > > SUGGESTION > errhint("Before creating subscriptions, set \"wal_level\" >= \"replica\".") >
Agreed with the suggested change. > ~~ > > 1b. > Since wal_level needs to be >= replica, the condition for this WARNING > might be better to be written as: > if (wal_level < WAL_LEVEL_REPLICA) Changed. > > ~~ > > 1c. > Now that the condition for this warning was slightly changed, there > seems to be no test case anymore for this WARNING. Isn't it better to > still have a test case for this? There are other tests covering this path. For instance, we check the WARNING message in 001_rep_changes.pl. Regards, -- Masahiko Sawada Amazon Web Services: https://aws.amazon.com
