On Tue, Mar 8, 2022 at 10:11 PM Robert Haas <robertmhaas@gmail.com> wrote:
>
> On Fri, Mar 4, 2022 at 12:37 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> > In this version I have fixed both of these issues.
>
> Here's a bit of review for these patches:
>
> - The whole relnode vs. relfilenode thing is really confusing. I
> realize that there is some precedent for calling the number that
> pertains to the file on disk "relnode" and that value when combined
> with the database and tablespace OIDs "relfilenode," but it's
> definitely not the most obvious thing, especially since
> pg_class.relfilenode is a prominent case where we don't even adhere to
> that convention. I'm kind of tempted to think that we should go the
> other way and rename the RelFileNode struct to something like
> RelFileLocator, and then maybe call the new data type RelFileNumber.
> And then we could work toward removing references to "filenode" and
> "relfilenode" in favor of either (rel)filelocator or (rel)filenumber.
> Now the question (even assuming other people like this general
> direction) is how far do we go with it? Renaming pg_class.relfilenode
> itself wouldn't be the worst compatibility break we've ever had, but
> it would definitely cause some pain. I'd be inclined to leave the
> user-visible catalog column alone and just push in this direction for
> internal stuff.
I have worked on this renaming stuff first and once we agree with that
then I will rebase the other patches on top of this and will also work
on the other review comments for those patches.
So basically in this patch
- The "RelFileNode" structure to "RelFileLocator" and also renamed
other internal member as below
typedef struct RelFileLocator
{
Oid spcOid; /* tablespace */
Oid dbOid; /* database */
Oid relNumber; /* relation */
} RelFileLocator;
- All variables and internal functions which are using name as
relfilenode/rnode and referring to this structure are renamed to
relfilelocator/rlocator.
- relNode/relfilenode which are referring to the actual file name on
disk is renamed to relNumber/relfilenumber.
- Based on the new terminology, I have renamed the file names as well, e.g.
relfilenode.h -> relfilelocator.h
relfilenodemap.h -> relfilenumbermap.h
I haven't renamed the exposed catalog variable and exposed function
here is the high level list
- pg_class.relfilenode
- pg_catalog.pg_relation_filenode()
- All test cases variables referring to pg_class.relfilenode.
- exposed option for tool which are w.r.t pg_class relfilenode (e.g.
-f, --filenode=FILENODE)
- exposed functions
pg_catalog.binary_upgrade_set_next_heap_relfilenode() and friends
- pg_filenode.map file name, maybe we can rename this but this is used
by other tools so I left this alone.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com