Re: [HACKERS] [PATCH 05/14] Add a new relmapper.c function RelationMapFilenodeToOid that acts as a reverse of RelationMapOidToFilenode
On Fri, Nov 16, 2012 at 7:58 PM, Andres Freund 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? If other people have additional comments on the ability to perform a relfileoid->reloid operation for cached maps, of course go ahead. -- Michael Paquier http://michael.otacoo.com
Re: [HACKERS] [PATCH 05/14] Add a new relmapper.c function RelationMapFilenodeToOid that acts as a reverse of RelationMapOidToFilenode
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. Thanks, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH 05/14] Add a new relmapper.c function RelationMapFilenodeToOid that acts as a reverse of RelationMapOidToFilenode
Hi, 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. 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. 3) Both functions are doing similar operations. Could it be possible to wrap them in the same central function? On Thu, Nov 15, 2012 at 10:17 AM, Andres Freund wrote: > --- > src/backend/utils/cache/relmapper.c | 53 > + > src/include/catalog/indexing.h | 4 +-- > src/include/utils/relmapper.h | 2 ++ > 3 files changed, 57 insertions(+), 2 deletions(-) > > > > -- > Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgsql-hackers > > -- Michael Paquier http://michael.otacoo.com
