Обсуждение: Deduplicate min restart_lsn calculation code

Hi,

It seems like the two functions ReplicationSlotsComputeRequiredLSN and
ReplicationSlotsComputeLogicalRestartLSN more or less does the same
thing which makes me optimize (saving 40 LOC) it as attached. I'm
pretty much okay if it gets rejected on the grounds that it creates a
lot of diff with the older versions and the new API may not look
nicer, still I want to give it a try.

Thoughts?

Regards,
Bharath Rupireddy.

On 2022-Jan-06, Bharath Rupireddy wrote:

> Hi,
> 
> It seems like the two functions ReplicationSlotsComputeRequiredLSN and
> ReplicationSlotsComputeLogicalRestartLSN more or less does the same
> thing which makes me optimize (saving 40 LOC) it as attached. I'm
> pretty much okay if it gets rejected on the grounds that it creates a
> lot of diff with the older versions and the new API may not look
> nicer, still I want to give it a try.
> 
> Thoughts?

Hmm, it seems sensible to me.  But I would not have the second boolean
argument in the new function, and instead have the caller save the
return value in a local variable to do the XLogSetReplicationSlotMinimumLSN
step separately.  Then the new function API is not so strange.

-- 
Álvaro Herrera           39°49'30"S 73°17'W  —  https://www.EnterpriseDB.com/
"Linux transformó mi computadora, de una `máquina para hacer cosas',
en un aparato realmente entretenido, sobre el cual cada día aprendo
algo nuevo" (Jaime Salinas)

On Thu, Jan 6, 2022 at 11:54 PM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
>
> On 2022-Jan-06, Bharath Rupireddy wrote:
>
> > Hi,
> >
> > It seems like the two functions ReplicationSlotsComputeRequiredLSN and
> > ReplicationSlotsComputeLogicalRestartLSN more or less does the same
> > thing which makes me optimize (saving 40 LOC) it as attached. I'm
> > pretty much okay if it gets rejected on the grounds that it creates a
> > lot of diff with the older versions and the new API may not look
> > nicer, still I want to give it a try.
> >
> > Thoughts?
>
> Hmm, it seems sensible to me.  But I would not have the second boolean
> argument in the new function, and instead have the caller save the
> return value in a local variable to do the XLogSetReplicationSlotMinimumLSN
> step separately.  Then the new function API is not so strange.

Thanks for taking a look at it. Here's the v2 patch, please review.

Regards,
Bharath Rupireddy.