Re: replication_slots usability issue

On 2018-11-01 09:34:05 +0900, Michael Paquier wrote:
> HI Andres,
>
> On Wed, Oct 31, 2018 at 03:48:02PM -0700, Andres Freund wrote:
> > And done. Thanks for the report JD.
>
> Shouldn't we also switch the PANIC to a FATAL in
> RestoreSlotFromDisk()?

That has absolutely nothing to do with the issue at hand though, so I
don't think it'd have made much sense to do it at the same time. Nor do
I think it's particularly important.

> I don't mind doing so myself if you agree with the change, only on
> HEAD as you seemed to disagree about changing that on back-branches.

Cool. And yes, I don't think a cosmetic log level adjustment that could
affect people's scripts should be backpatched without need. Even if not
particularly likely to break something.

> Also, from 691d79a which you just committed:
> + ereport(FATAL,
> + (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
> + errmsg("logical replication slots \"%s\" exists, but wal_level < logical",
> + NameStr(cp.slotdata.name)),
> I can see one grammar mistake here, as you refer to only one slot here.
> The error messages should read:
> "logical replication slot \"%s\" exists, but wal_level < logical"
> and:
> "physical replication slot \"%s\" exists, but wal_level < replica"

Darnit. Fixed. Thanks.

Greetings,

Andres Freund