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.
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.
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.
Patches 0002 replaces includes of heapam.h with relation.h / table.h
where appropriate. 0003 renames all in-core references of
heap_(open|openrv|openrv_extended|close) with the table_ variant.
Does this seem basically sensible? Different ideas?
Greetings,
Andres Freund
