On 4/8/23 12:01 AM, Andres Freund wrote:
> Hi,
>
> 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, though perhaps "receive" instead of "get?"
Jonathan