Re: [PATCH 05/14] Add a new relmapper.c function RelationMapFilenodeToOid that acts as a reverse of RelationMapOidToFilenode

On 2012-11-17 19:14:06 +0900, Michael Paquier wrote:
> On Fri, Nov 16, 2012 at 7:58 PM, Andres Freund <andres@2ndquadrant.com>wrote:
> 
> > Hi,
> >
> > On 2012-11-16 13:44:45 +0900, Michael Paquier wrote:
> > > This patch looks OK.
> > >
> > > I got 3 comments:
> > > 1) Why changing the OID of pg_class_tblspc_relfilenode_index from 3171 to
> > > 3455? It does not look necessary.
> >
> > Its a mismerge and should have happened in "Add a new RELFILENODE
> > syscache to fetch a pg_class entry via (reltablespace, relfilenode)" but
> > it seems I squashed the wrong two commits.
> > I had originally used 3171 but that since got used up for lo_tell64...
> >
> > > 2) You should perhaps change the header of RelationMapFilenodeToOid so as
> > > not mentionning it as the opposite operation of RelationMapOidToFilenode
> > > but as an operation that looks for the OID of a relation based on its
> > > relfilenode. Both functions are opposite but independent.
> >
> > I described it as the opposite because RelationMapOidToFilenode is the
> > relmappers stated goal and RelationMapFilenodeToOid is just some
> > side-business.
> >
> > > 3) Both functions are doing similar operations. Could it be possible
> > > to wrap them in the same central function?
> >
> > I don't really see how without making both quite a bit more
> > complicated. The amount of if's needed seems to be too large to me.
> >
> OK thanks for your answers.
> As this patch only adds a new function and is not that much complicated, I
> think there is no problem in committing it. The only thing to remove is the
> diff in indexing.h. Could someone take care of that?

I pushed a rebase to the git repository that fixed it...

Greetings,

Andres Freund

-- 
 Andres Freund	                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services