Re: Passing CopyMultiInsertInfo structure to CopyMultiInsertInfoNextFreeSlot()

From: Ashutosh Sharma <ashu(dot)coek88(at)gmail(dot)com>
To: David Rowley <david(dot)rowley(at)2ndquadrant(dot)com>
Cc: Andres Freund <andres(at)anarazel(dot)de>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Michael Paquier <michael(at)paquier(dot)xyz>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, Haribabu Kommi <kommi(dot)haribabu(at)gmail(dot)com>
Subject: Re: Passing CopyMultiInsertInfo structure to CopyMultiInsertInfoNextFreeSlot()
Date: 2019-05-18 01:31:05
Message-ID: CAE9k0PkASG72buVx-h9Q-ZhEjD7v8Nnk0OLk0L_iEYkS9hM=Ww@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Sat, May 18, 2019 at 6:44 AM David Rowley <david(dot)rowley(at)2ndquadrant(dot)com>
wrote:

> On Sat, 18 May 2019 at 12:49, Andres Freund <andres(at)anarazel(dot)de> wrote:
> >
> > On 2019-05-18 06:14:15 +0530, Ashutosh Sharma wrote:
> > > I actually feel that the function name itself is not correct here, it
> > > appears to be confusing and inconsistent considering the kind of work
> that
> > > it is doing. I think, the function name should have been
> CopyMultiInsert
> > > *Buffer*NextFreeSlot() instead of CopyMultiInsert*Info*NextFreeSlot().
> What
> > > do you think, Andres, David, Alvaro ?
> >
> > Unless somebody else presses back hard against doing so *soon*, I'm
> > going to close this open issue. I don't think it's worth spending
> > further time arguing about a few characters.
>
> I'd say if we're not going to bother removing the unused param that
> there's not much point in renaming the function. The proposed name
> might make sense if the function was:
>
> static inline TupleTableSlot *
> CopyMultiInsertBufferNextFreeSlot(CopyMultiInsertBuffer *buffer, Relation
> rel)
>
>
Well, that's what I suggested but seems like Andres is not in favour of it
because he feels that in the future we *should* add a facility to reuse the
slots across the partitions because reusing a free slot is quite cheaper
than creating a new one and in that sense we would in future need to pass
miinfo to *NextFreeSlot function

> then that might be worth a commit, but giving it that name without
> changing the signature to that does not seem like an improvement to
> me.
>
>
That's right, we cannot have this name without changing it's signature.

> I'm personally about +0.1 for making the above change, which is well
> below my threshold for shouting and screaming.
>
>
I think, as Andres pointed out in his earlier reply, we should probably
stop this discussion here, if in future we add the support to reuse the
slots across the partition then probably we will have to undo the changes
that we will be doing here. Anyways, thanks to Andres and David for
clearing my doubts.

> --
> David Rowley http://www.2ndQuadrant.com/
> PostgreSQL Development, 24x7 Support, Training & Services
>

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2019-05-18 01:31:59 Re: itemptr_encode/itemptr_decode
Previous Message Tom Lane 2019-05-18 01:30:07 Re: Passing CopyMultiInsertInfo structure to CopyMultiInsertInfoNextFreeSlot()