On Thu, Jan 25, 2024 at 18:39 Amit Kapila <amit.kapila16@gmail.com> wrote:
>
Thanks for your review.
> On Thu, Jan 25, 2024 at 2:57 PM Wei Wang (Fujitsu)
> <wangw.fnst@fujitsu.com> wrote:
> >
> > In the function ReplicationSlotAcquire(), I found there is a missing in the
> > below comments:
> >
> > ```
> > /*
> > * Search for the slot with the specified name if the slot to acquire is
> > * not given. If the slot is not found, we either return -1 or error out.
> > */
> > s = SearchNamedReplicationSlot(name, false);
> > if (s == NULL || !s->in_use)
> > {
> > LWLockRelease(ReplicationSlotControlLock);
> >
> > ereport(ERROR,
> > (errcode(ERRCODE_UNDEFINED_OBJECT),
> > errmsg("replication slot \"%s\" does not
> exist",
> > name)));
> > }
> > ```
> >
> > It seems that when the slot is not found, we will only error out and will not
> > return -1.
> >
>
> You seem to be correct. However, isn't the first part of the comment
> also slightly confusing? In particular, "... if the slot to acquire is
> not given." In this function, I don't see the case where a slot to
> acquire is given.
Yes, agree. I think these two parts have become slightly outdated after the
commit 1632ea4. So also tried to fix the first part of the comment.
Attach the new patch.
Regards,
Wang Wei
