Re: DropRelFileLocatorBuffers

On Tue, Jul 12, 2022 at 7:55 AM Kyotaro Horiguchi
<horikyota.ntt@gmail.com> wrote:
>
> At Mon, 11 Jul 2022 13:51:12 -0400, Robert Haas <robertmhaas@gmail.com> wrote in
> > On Fri, Jul 8, 2022 at 1:59 AM Kyotaro Horiguchi
> > <horikyota.ntt@gmail.com> wrote:
> > > The function CreateAndCopyRelationData exists since before b0a55e4329
> > > but renamed since it takes RelFileLocators.
> >
> > I'm not very sold on this. I think that the places where you've
> > replaced RelFileLocator with just RelFile in various functions might
> > be an improvement, but the places where you've replaced Relation with
> > RelFile seem to me to be worse. I don't really see that there's
> > anything wrong with names like CreateAndCopyRelationData or
> > FlushRelationsAllBuffers, and in general I prefer function names that
> > are made up of whole words rather than parts of words.
>
> Fair enough. My first thought was that Relation can represent both
> Relation and "RelFile" but in the patch I choosed to make distinction
> between them by associating respectively to the types of the primary
> parameter (Relation or RelFileLocator).  So I'm fine with Relation
> instead since I see it more intuitive than RelFileLocator in the
> function names.
>
> The attached is that.

I think the naming used in your patch looks better to me. So +1 for the change.

-- 
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com