Re: Reworks of CustomScan serialization/deserialization

Поиск
Список
Период
Сортировка
От Petr Jelinek
Тема Re: Reworks of CustomScan serialization/deserialization
Дата
Msg-id 56E0BF2F.1020801@2ndquadrant.com
обсуждение исходный текст
Ответ на Reworks of CustomScan serialization/deserialization  (Kouhei Kaigai <kaigai@ak.jp.nec.com>)
Ответы Re: Reworks of CustomScan serialization/deserialization
Список pgsql-hackers
Hi,

On 29/02/16 13:07, Kouhei Kaigai wrote:
> 
> I'd like to adjust a few of custom-scan interface prior to v9.6 freeze.
> 
> The major point is serialization/deserialization mechanism.
> Now, extension has to give LibraryName and SymbolName to reproduce
> same CustomScanMethods on the background worker process side. Indeed,
> it is sufficient information to pull the table of function pointers.
> 
> On the other hands, we now have different mechanism to wrap private
> information - extensible node. It requires extensions to register its
> ExtensibleNodeMethods identified by name, usually, on _PG_init() time.
> It is also reasonable way to reproduce same objects on background
> worker side.
> 
> However, mixture of two different ways is not good. My preference is
> what extensible-node is doing rather than what custom-scan is currently
> doing.
> The attached patch allows extension to register CustomScanMethods once,
> then readFunc.c can pull this table by CustomName in string form.
> 

Agreed, but this will break compatibility right?

> I'm not 100% certain whether "nodes/custom-apis.h" is the best location,
> but somewhere we can put these declarations rather than the primitive
> header files might be needed.

custom-apis.c does not sound like right name to me, maybe it can be just
custom.c but custom.h might be bit too generic, maybe custom_node.h

I am not sure I like the fact that we have this EXTNODENAME_MAX_LEN and
now the CUSTOM_NAME_MAX_LEN with the same length and also they are both
same lenght as NAMEDATALEN I wonder if this shouldn't be somehow
squished to less defines.

Also in RegisterCustomScanMethods
+    Assert(strlen(methods->CustomName) <= CUSTOM_NAME_MAX_LEN);

Shouldn't this be actually "if" with ereport() considering this is
public API and extensions can pass anything there? (for that matter same
is true for RegisterExtensibleNodeMethods but that's already committed).

Other than that this seems like straight conversion to same basic
template as extensible nodes so I think it's ok.

--  Petr Jelinek                  http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training &
Services



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

Предыдущее
От: Petr Jelinek
Дата:
Сообщение: Re: Proposal: Generic WAL logical messages
Следующее
От: David Steele
Дата:
Сообщение: Re: Improving replay of XLOG_BTREE_VACUUM records