On Sat, Apr 8, 2023 at 9:31 AM Andres Freund <and...@anarazel.de> wrote: > > On 2023-04-08 09:15:05 +0530, Amit Kapila wrote: > > The new approach for invalidation looks clean. BTW, I see minor > > inconsistency in the following two error messages (errmsg): > > Thanks for checking. > > > > if (MyReplicationSlot->data.invalidated == RS_INVAL_WAL) > > ereport(ERROR, > > (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), > > errmsg("can no longer get changes from replication slot \"%s\"", > > NameStr(MyReplicationSlot->data.name)), > > errdetail("This slot has been invalidated because it exceeded the > > maximum reserved size."))); > > > > if (MyReplicationSlot->data.invalidated != RS_INVAL_NONE) > > ereport(ERROR, > > (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), > > errmsg("cannot read from logical replication slot \"%s\"", > > NameStr(MyReplicationSlot->data.name)), > > errdetail("This slot has been invalidated because it was conflicting > > with recovery."))); > > > > Won't it be better to keep the same errmsg in the above two cases? > > Probably - do you have a preference? I think the former is a bit better? >
+1 for the former. -- With Regards, Amit Kapila.