On 2012-11-17 19:14:06 +0900, Michael Paquier wrote: > On Fri, Nov 16, 2012 at 7:58 PM, Andres Freund <and...@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 -- Sent via pgsql-hackers mailing list (firstname.lastname@example.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers