Re: POC: enable logical decoding when wal_level = 'replica' without a server restart

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\".")

~~

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)

~~

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?

======
Kind Regards,
Peter Smith.
Fujitsu Australia