Re: patch: function xmltable

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

> Pavel Stehule wrote:
> > 2016-12-07 18:34 GMT+01:00 Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>:
>
> > > Hmm. Now that I see how this works, by having the GetValue "guess"
> what
> > > is going on and have a special case for it, I actually don't like it
> > > very much. It seems way too magical. I think we should do away with
> > > the "if column is NULL" case in GetValue, and instead inject a column
> > > during transformTableExpr if columns is NIL. This has implications on
> > > ExecInitExpr too, which currently checks for an empty column list -- it
> > > would no longer have to do so.
> >
> > I prefer this way against second described. The implementation should be
> in
> > table builder routines, not in executor.
>
> Well, given the way you have implemented it, I would prefer the original
> too. But your v23 is not what I meant. Essentially what you do in v23
> is to communicate the lack of COLUMNS clause in a different way --
> previously it was "ncolumns = 0", now it's "is_auto_col=true". It's
> still "magic". It's not an improvement.
>

is_auto_col is used primary for asserting. The table builder has
information for decision in parameter path, when path is NULL.

Hard to say, if this info should be assigned to column or to table. In both
locations has sense. But somewhere should be some flag.

>
> What I want to happen is that there is no magic at all; it's up to
> transformExpr to make sure that when COLUMNS is empty, one column
> appears and it must not be a magic column that makes the xml.c code act
> differently, but rather to xml.c it should appear that this is just a
> normal column that happens to return the entire row. If I say "COLUMNS
> foo PATH '/'" I should be able to obtain a similar behavior (except that
> in the current code, if I ask for "COLUMNS foo XML PATH '/'" I don't get
> XML at all but rather weird text where all tags have been stripped out,
> which is very strange. I would expect the tags to be preserved if the
> output type is XML. Maybe the tag-stripping behavior should occur if
> the output type is some type of text.)
>

I am doing this. Just I using NULL for PATH.

>
>
> I still have to figure out how to fix the tupledesc thing. What we have
> now is not good.
>

cannot be moved to nodefunc?

Pavel

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