From: | Alvaro Herrera <alvherre(at)2ndquadrant(dot)com> |
---|---|
To: | Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> |
Cc: | Craig Ringer <craig(at)2ndquadrant(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: patch: function xmltable |
Date: | 2016-12-03 15:03:58 |
Message-ID: | 20161203150358.4npcgzjxcxysqlcd@alvherre.pgsql |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Pavel Stehule wrote:
> 2016-12-02 23:25 GMT+01:00 Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>:
> > 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.
True. This is a minor point.
Are you able to do the memory context change I describe?
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From | Date | Subject | |
---|---|---|---|
Next Message | Dilip Kumar | 2016-12-03 15:30:27 | Re: Proposal: scan key push down to heap [WIP] |
Previous Message | Michael Paquier | 2016-12-03 13:14:15 | Better support for symlinks on Windows... |