Re: walsender: Assert MyReplicationSlot is set before use - Mailing list pgsql-hackers

> On Feb 3, 2026, at 13:12, Fujii Masao <masao.fujii@gmail.com> wrote:
>
> On Tue, Feb 3, 2026 at 12:38 PM Chao Li <li.evan.chao@gmail.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
“physicalreplication”. 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/