Hi,
I went over the latest version of this, here are my notes.
create-am.9:
+ case DO_ACCESS_METHOD:
+ snprintf(buf, bufsize,
+ "");
+ return;
Missing the actual description.
+ appendPQExpBuffer(q, "CREATE ACCESS METHOD %s HANDLER %s;\n",
+ qamname, aminfo->amhandler);
Generates invalid syntax (missing TYPE).
I don't like that you hardcode 'i' everywhere in the code as am type
index. It would be better to define it in pg_am.h and use that.
I was also debating in my head if amtype is correct name as it maps to
relkind so amkind might be better name for the column, otoh then it
won't map to the syntax we have agreed on for CREATE ACCESS METHOD so I
guess there is no ideal name here.
object_type_map record is missing, as is getObjectTypeDescription and
getObjectIdentityParts support
(you can check what I sent as part of sequence access methods where I
fixed these things, otherwise it reuses most of your code)
generic-xlog.9:
Not much to say here, the api makes sense to me, it will have
performance impact but don't see how we can do this generically without
callbacks to extension in any better way.
One thing I don't understand is why there is support for unlogged
relations. Shouldn't that be handled by whoever is using this to
actually not call this at all? It's just call to RelationNeedsWAL()
nothing to hard and hidden magic like not doing anything with WAL for
the unlogged tables is seldomly good idea.
Another small thing is that we put the API explanation comments into .c
file not .h file.
Didn't look at the bloom index too deeply yet.
-- Petr Jelinek http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training &
Services
