Re: WIP: Access method extendability

Поиск
Список
Период
Сортировка
От Alexander Korotkov
Тема Re: WIP: Access method extendability
Дата
Msg-id CAPpHfdsWEA4NO7=xsb79jvxWEBkC+6FD0QvmBaBo2g80WtpHhg@mail.gmail.com
обсуждение исходный текст
Ответ на Re: WIP: Access method extendability  (Petr Jelinek <petr@2ndquadrant.com>)
Ответы Re: WIP: Access method extendability  (Alvaro Herrera <alvherre@2ndquadrant.com>)
Список pgsql-hackers
Hi!

Next revision of patches is attached.

On Wed, Mar 9, 2016 at 4:56 AM, Petr Jelinek <petr@2ndquadrant.com> wrote:
I went over the latest version of this, here are my notes.

create-am.9:

+               case DO_ACCESS_METHOD:
+                       snprintf(buf, bufsize,
+                                        "");
+                       return;

Fixed. 
 
Missing the actual description.

+       appendPQExpBuffer(q, "CREATE ACCESS METHOD %s HANDLER %s;\n",
+                                         qamname, aminfo->amhandler);

Generates invalid syntax (missing TYPE).

Fixed. 

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.

Fixed. 

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.

I leave it amtype for now. 
 
object_type_map record is missing, as is getObjectTypeDescription and getObjectIdentityParts support
 
Fixed.

(you can check what I sent as part of sequence access methods where I fixed these things, otherwise it reuses most of your code)

Thank you! I used it as cheat sheet.
 
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.

Yep, performance might be much less than with other resource managers. But that seems to be the only way to do this in extension since we don't want extensions to have redo callbacks. Also, this is the basic version. There could be more advances in future. In the previous version I has more effective handling of data shift inside page. But it was removed for the sake of simplicity to increase chance to commit.

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.

Besides formation of xlog record generic xlog also does copying page images and atomic replacement of them inside critical section. For sure, it could be done without generic xlog. But I found it useful for generic xlog to support this since code of extension becomes much more simple and elegant. But I can remove it if you insist.

Another small thing is that we put the API explanation comments into .c file not .h file.

Explanation was moved from .h to .c.

------
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company 
Вложения

В списке pgsql-hackers по дате отправления:

Предыдущее
От: Fabien COELHO
Дата:
Сообщение: Re: pgbench small bug fix
Следующее
От: Eduardo Morras
Дата:
Сообщение: Re: Novice Presentation and Company Project