Re: patch: function xmltable

From: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
To: Alvaro Herrera <alvherre(at)2ndquadrant(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-04 22:00:17
Message-ID: CAFj8pRDJ-djtq1NbtyACp4wZxcSix7H-hkk+W6r=rG+yMO8y+w@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

2016-12-03 16:03 GMT+01:00 Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>:

> 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?
>

I am not sure if I understand well to your ideas - please, check attached
patch.

Regards

Pavel

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

Attachment Content-Type Size
xmltable-18.patch text/x-patch 174.1 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Pavel Stehule 2016-12-04 22:12:55 Re: patch: function xmltable
Previous Message Peter Geoghegan 2016-12-04 20:44:46 Re: Parallel tuplesort (for parallel B-Tree index creation)