Re: patch: function xmltable

Поиск
Список
Период
Сортировка
От Pavel Stehule
Тема Re: patch: function xmltable
Дата
Msg-id CAFj8pRDLadjfDBS9FHki4xkquU9161XjFA7jbrrsiL244HQ4Gg@mail.gmail.com
обсуждение исходный текст
Ответ на Re: patch: function xmltable  (Alvaro Herrera <alvherre@2ndquadrant.com>)
Ответы Re: patch: function xmltable  (Alvaro Herrera <alvherre@2ndquadrant.com>)
Список pgsql-hackers
Hi

2016-12-02 23:25 GMT+01:00 Alvaro Herrera <alvherre@2ndquadrant.com>:
Here's version 17.  I have made significant changes here.

1. Restructure the execQual code.  Instead of a PG_TRY wrapper, I have
split this code in three pieces; there's the main code with the PG_TRY
wrappers and is mainly in charge of the builderCxt pointer.  In the
previous coding there was a shim that examined builderCxt but was not
responsible for setting it up, which was ugly.  The second part is the
"initializer" which sets the row and column filters and does namespace
processing.  The third part is the "FetchRow" logic.  It seems to me
much cleaner this way.

2. rename the "builder" stuff to use the "routine" terminology.  This is
in line with what we do for other function-pointer-filled structs, such
as FdwRoutine, IndexAmRoutine etc.  I also cleaned up the names a bit
more.

3. Added a magic number to the table builder context struct, so that we
can barf appropriately.  This is in line with PgXmlErrorContext --
mostly for future-proofing.  I didn't test this too hard.  Also, moved
the XmlTableContext struct declaration nearer the top of the file, as is
customary.  (We don't really need it that way, since the functions are
all declared taking void *, but it seems cleaner to me anyway).

4. I added, edited, and fixed a large number of code comments.

This is looking much better now, but it still needs at least the
following changes.

First, we need to fix is the per_rowset_memcxt thingy.  I think the way
it's currently being used is rather ugly; it looks to me like the memory
context does not belong into the XmlTableContext struct at all.
Instead, the executor code should keep the memcxt pointer in a state
struct of its own, and it should be the executor's responsibility to
change to the appropriate context before calling the table builder
functions.  In particular, this means that the table context can no
longer be a void * pointer; it needs to be a struct that's defined by
the executor (probably a primnodes.h one).  The void * pointer is
stashed inside that struct.  Also, the "routine" pointer should not be
part of the void * struct, but of the executor's struct.  So the
execQual code can switch to the memory context, and destroy it
appropriately.

Second, we should make gram.y set a new "function type" value in the
TableExpr it creates, so that the downstream code (transformTableExpr,
ExecInitExpr, ruleutils.c) really knows that the given function is
XmlTableExpr, instead of guessing just because it's the only implemented
case.  Probably this "function type" is an enum (currently with a single
value TableExprTypeXml or something like that) in primnodes.

It has sense - I was not sure about it - because currently it is only one value, you mentioned it.
 

Finally, there's the pending task of renaming and moving
ExecTypeFromTableExpr to some better place.  Not sure that moving it
back to nodeFuncs is a nice idea.  Looks to me like calling it from
ExprTypmod is a rather ugly idea.

The code is related to prim nodes - it is used more times than in executor.

Hmm, ruleutils ... not sure what to think of this one.

it is little bit more complex - but it is related to complexity of XMLTABLE
 

The typedefs.list changes are just used to pgindent the affected code
correctly.  It's not for commit.

The documentation is very precious. Nice

+    /* XXX OK to do this?  looks a bit out of place ... */
+    assign_record_type_typmod(typeInfo);

I am thinking it is ok. It is tupdesc without fixed typid, typname used in returned value - you should to register this tupdesc in typcache.

Regards

Pavel



--
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

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

Предыдущее
От: Fabien COELHO
Дата:
Сообщение: Re: PSQL commands: \quit_if, \quit_unless
Следующее
От: Fabien COELHO
Дата:
Сообщение: Re: pgbench - allow backslash continuations in \set expressions