Re: What to name the current heap after pluggable storage / what to rename? - Mailing list pgsql-hackers

From Haribabu Kommi
Subject Re: What to name the current heap after pluggable storage / what to rename?
Date
Msg-id CAJrrPGeTPu=8eTnQMRtwxz8uTLnGNpSZa6K4JsU6gMd=ULHgeA@mail.gmail.com
Whole thread Raw
In response to Re: What to name the current heap after pluggable storage / what torename?  (Andres Freund <andres@anarazel.de>)
Responses Re: What to name the current heap after pluggable storage / what to rename?  (Robert Haas <robertmhaas@gmail.com>)
List pgsql-hackers

On Tue, Jan 15, 2019 at 1:43 PM Andres Freund <andres@anarazel.de> wrote:
Hi,

On 2019-01-11 12:01:36 -0500, Robert Haas wrote:
> On Thu, Jan 10, 2019 at 7:05 PM Andres Freund <andres@anarazel.de> wrote:
> > ISTM that the first block would best belong into new files like
> > access/rel[ation].h and access/common/rel[ation].h.
>
> +1.
>
> > I think the second
> > set should be renamed to be table_open() (with backward compat
> > redirects, there's way way too many references) and should go into
> > access/table.h access/table/table.c alongside tableam.[ch],
>
> Sounds reasonable.
>
> > but I could
> > also see just putting them into relation.[ch].
>
> I would view that as a less-preferred alternative, but not crazy.

Here's a set of patches. Not commit quality, but enough to discuss.


The first patch, the only really interesting one, splits out
relation_(open|openrv|openrv_extended|close) into access/relation.h and access/common/relation.c
and
heap_(open|openrv|openrv_extended|close) into access/table.h and access/table/table.c

It's worthwhile to note that there's another header named
nodes/relation.h. But there's also utils/rel.h, so I couldn't think of a
another good name.

access/relation.[c|h] name is fine. Or how about access/rel.[c|h],
because nodes/relation.h is related to planner. utils/rel.h is some how
related to relation caches.
 
I'm basically thinking that table.h, even in the post pluggable storage
world, should not contain lower level functionality like dispatching
into table-am (that'll reside in tableam.h). But e.g. a
simple_table_(insert|update|delete) could live there, as well as
potentially some other heap_ functionality strewn around the backend.

I made table.h not include relation.h, which means that a few files
might need both.  I'm not sure that's the right choice, but it seems
easier to extend that later if shows to be painful, than to do the
reverse.

The need of both relation.h and table.h into a single file is because of
use of both relation_open table_open functions. when I checked one
of the file where both headers are included, I found that it simply used
the relation_open function even the type of the relation is known.

I didn't check whether it is possible or not? In case if the kind of the relation
is known at every caller of relation_open, it can be replaced with either
table_open or index_open or composite_type_open functions. So that
there may not need any of the relation functions needs to be exposed.


I've left the following in table.h:
/*
 * heap_ used to be the prefix for these routines, and a lot of code will just
 * continue to work without adaptions after the introduction of pluggable
 * storage, therefore just map these names.
 */
#define heap_open(r, l)                                 table_open(r, l)
#define heap_openrv(r, l)                               table_openrv(r, l)
#define heap_openrv_extended(r, l, m)   table_openrv_extended(r, l, m)
#define heap_close(r, l)                                table_close(r, l)

and I think we should leave that in there for the forseeable future.

Above typedefs are good. They are useful to avoid any third party
extensions.

Regards,
Haribabu Kommi
Fujitsu Australia

pgsql-hackers by date:

Previous
From: "Tsunakawa, Takayuki"
Date:
Subject: RE: Libpq support to connect to standby server as priority
Next
From: Mithun Cy
Date:
Subject: Re: WIP: Avoid creation of the free space map for small tables