Re: patch: function xmltable
От | Alvaro Herrera |
---|---|
Тема | Re: patch: function xmltable |
Дата | |
Msg-id | 20161202222550.t5v23evjddgmkfly@alvherre.pgsql обсуждение исходный текст |
Ответ на | Re: patch: function xmltable (Pavel Stehule <pavel.stehule@gmail.com>) |
Ответы |
Re: patch: function xmltable
|
Список | pgsql-hackers |
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
Вложения
В списке pgsql-hackers по дате отправления: