Re: Passing CopyMultiInsertInfo structure to CopyMultiInsertInfoNextFreeSlot() - Mailing list pgsql-hackers

From Ashutosh Sharma
Subject Re: Passing CopyMultiInsertInfo structure to CopyMultiInsertInfoNextFreeSlot()
Date
Msg-id CAE9k0PkASG72buVx-h9Q-ZhEjD7v8Nnk0OLk0L_iEYkS9hM=Ww@mail.gmail.com
Whole thread Raw
In response to Re: Passing CopyMultiInsertInfo structure to CopyMultiInsertInfoNextFreeSlot()  (David Rowley <david.rowley@2ndquadrant.com>)
List pgsql-hackers
On Sat, May 18, 2019 at 6:44 AM David Rowley <david.rowley@2ndquadrant.com> wrote:
On Sat, 18 May 2019 at 12:49, Andres Freund <andres@anarazel.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

pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: Passing CopyMultiInsertInfo structure to CopyMultiInsertInfoNextFreeSlot()
Next
From: Andres Freund
Date:
Subject: Re: itemptr_encode/itemptr_decode