Re: WIP: Rework access method interface - Mailing list pgsql-hackers

From Alexander Korotkov
Subject Re: WIP: Rework access method interface
Date
Msg-id CAPpHfdtkv0+zayrK3brxHwB+_tgh8zZ_KKjfmzJ4TuyNq1be8Q@mail.gmail.com
Whole thread Raw
In response to Re: WIP: Rework access method interface  (Petr Jelinek <petr@2ndquadrant.com>)
Responses Re: WIP: Rework access method interface  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-hackers
On Mon, Aug 10, 2015 at 1:12 PM, Petr Jelinek <petr@2ndquadrant.com> wrote:
On 2015-08-09 23:56, Alexander Korotkov wrote:
Hacker,

some time before I proposed patches implementing CREATE ACCESS METHOD.
http://www.postgresql.org/message-id/CAPpHfdsXwZmojm6Dx+TJnpYk27kT4o7Ri6X_4OSWcByu1Rm+VA@mail.gmail.com
As I get from comments to my patches and also from Tom's comment about
AM interface in tablesampling thread – AM interface needs reworking. And
AFAICS AM interface rework is essential to have CREATE ACCESS METHOD
command.
http://www.postgresql.org/message-id/5438.1436740611@sss.pgh.pa.us

Cool, I was planning to take a stab at this myself when I have time, so I am glad you posted this.

This is why I'd like to show a WIP patch implementing AM interface
rework. Patch is quite dirty yet, but I think it illustrated the idea
quite clear. AM now have single parameter – handler function. This
handler returns pointer to AmRoutine which have the same data as pg_am
had before. Thus, API is organized very like FDW.


I wonder if it would make sense to call it IndexAmRoutine instead in case we add other AMs (like the sequence am, or maybe column store if that's done as AM) in the future.

Good point!
 
The patch should probably add the am_handler type which should be return type of the am handler function (see fdw_handler and tsm_handler types).

Sounds reasonable!
 
I also think that the CHECK_PROCEDUREs should be in the place of the original GET_REL_PROCEDUREs  (just after relation check). If the interface must exist we may as well check for it at the beginning and not after we did some other work which is useless without the interface.

Ok, good point too.

However, this patch appears to take more work than I expected. It have
to do many changes spread among many files.

Yeah you need to change the definition and I/O handling of every AM function, but that's to be expected.

Yes, this is why I decided to get some feedback on this stage of work.
 
Also, it appears not so easy
to hide amsupport into AmRoutine, because it's needed for relcache. As a
temporary solution it's duplicated in RelationData.

I don't understand this, there is already AmRoutine in RelationData, why the need for additional field for just amsupport?

We need amsupport in load_relcache_init_file() which reads "pg_internal.init". I'm not sure this is correct place to call am_handler. It should work in the case of built-in AM. But if AM is defined in the extension then we wouldn't be able to do catalog lookup for am_handler on this stage of initialization.

Another point is that amsupport and amstrategies are used for regression tests of opclasses and opfamilies. Thus, we probably can keep them in pg_am.

------
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

pgsql-hackers by date:

Previous
From: Andres Freund
Date:
Subject: Re: Summary of plans to avoid the annoyance of Freezing
Next
From: Robert Haas
Date:
Subject: Re: statement_timeout affects query results fetching?