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

From: Andres Freund <andres(at)anarazel(dot)de>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>, Haribabu Kommi <kommi(dot)haribabu(at)gmail(dot)com>, Alexander Korotkov <a(dot)korotkov(at)postgrespro(dot)ru>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
Subject: Re: What to name the current heap after pluggable storage / what to rename?
Date: 2019-01-15 02:43:19
Message-ID: 20190115024319.vdaz7lq6v7agwisi@alap3.anarazel.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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.

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

Attachment Content-Type Size
v1-0001-WIP-Introduce-access-table.h-access-relation.h.patch text/x-diff 26.5 KB
v1-0002-Replace-heapam.h-includes-with-relation.h-table.h.patch text/x-diff 48.3 KB
v1-0003-Replace-uses-of-heap_open-et-al-with-table_open-e.patch text/x-diff 377.3 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2019-01-15 02:46:49 Re: strange valgrind failures (again)
Previous Message Amit Langote 2019-01-15 02:42:18 Re: Query with high planning time at version 11.1 compared versions 10.5 and 11.0