Re: walsender: Assert MyReplicationSlot is set before use

> On Feb 3, 2026, at 13:12, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
>
> On Tue, Feb 3, 2026 at 12:38 PM Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> wrote:
>>> I think we cannot assume the slot type here. A suitable checking might
>>> be: If a physical slot was acquired during logical replication, report an error,
>>> just like we do in StartReplication().
>>
>> Good point. In StartReplication(), we check MyReplicationSlot is not logical, correspondingly, in StartLogicalReplication(), we should check MyReplicationSlot is not physical.
>
> StartLogicalReplication() calls CreateDecodingContext() after
> ReplicationSlotAcquire(), and CreateDecodingContext() seems to
> already perform this check. Isn't that sufficient?
>
> Regards,
>
>
> --
> Fujii Masao

Ah, thank you so much for pointing out that. I didn’t notice CreateDecodingContext() has done the check already:
```
/* shorter lines... */
slot = MyReplicationSlot;

/* first some sanity checks that are unlikely to be violated */
if (slot == NULL)
elog(ERROR, "cannot perform logical decoding without an acquired slot");

/* make sure the passed slot is suitable, these are user facing errors */
if (SlotIsPhysical(slot))
ereport(ERROR,
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
errmsg("cannot use physical replication slot for logical decoding")));
```

So, adding the check in StartLogicalReplication() is redundant. But I noticed the error message misses an “a” before “physical replication”. Looking at the error message in StartReplicaton():
```
ReplicationSlotAcquire(cmd->slotname, true, true);
if (SlotIsLogical(MyReplicationSlot))
ereport(ERROR,
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
errmsg("cannot use a logical replication slot for physical replication")));
```

It has an “a” before “logical replication”. Also I fixed that in CreateDecodingContext().

PFA v3:

* 0001:
- removed the validation of logical slot
- added a comment in StartLogicalReplication() to explain sanity check is deferred
- added an “a” in the error message in CreateDecodingContext()
* 0002: updated the expected error message in the TAP script

With v3 applied, the TAP test still passed.

Best reagards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/