Re: patch: function xmltable

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.

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.

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

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

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