Re: What to name the current heap after pluggable storage / what to rename?

On Tue, Jan 15, 2019 at 1:43 PM Andres Freund <andres(at)anarazel(dot)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(at)anarazel(dot)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