RE: Fix inappropriate comments in function ReplicationSlotAcquire

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