On Mon, Aug 10, 2015 at 1:12 PM, Petr Jelinek <p...@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...@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.1436740...@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
