[ having now read the relevant threads a bit more carefully ... ]

Florian Pflug <f...@phlo.org> writes:
> On Jul18, 2011, at 22:19 , Tom Lane wrote:
>> Yeah, it's not clear to me either which of those are still reasonable
>> candidates for committing as-is.

> There are actually three XML-related patches from me in the queue.
> I'll try to give an overview of their states - as perceived by me,
> of course. If people want to comment on this, I suggest to do that
> that either on the existing threads from these patches or on new ones,
> but not on this one - lest confusion ensues.

> * Bugfix for XPATH() if text or attribute nodes are selected
>   https://commitfest.postgresql.org/action/patch_view?id=580

> Makes XPATH() return valid XML if text or attribute are selected.

> I'm not aware of any issues with that one, other than Radoslaw's
> unhappiness about the change of behaviour. Since the current behaviour
> is very clearly broken (as in dump, reload, ka-Woom), I'm not prepared
> to accept that as a show-stopper.

I think we ought to apply this one.  I agree that returning invalid XML
is not acceptable.

> * Bugfix for XPATH() if expression returns a scalar value
>   https://commitfest.postgresql.org/action/patch_view?id=565

> Makes XPATH() support XPath expressions which compute a scalar value,
> not a node set (i.e. apply a top-level function such as name()).
> Currently, we return an empty array in that case.

> Peter Eisentraut initially seemed to prefer creating separate functions
> for that (XPATH_STRING, ...). I argued against that, on the grounds that
> that'd make it impossible to evaluate user-supplied XPath expression (since
> you wouldn't know which function to use). I haven't heard back from him
> after that argument.

I find your argument against XPATH_STRING() a bit unconvincing.
The use case for that is not where you are trying to evaluate an
unknown, arbitrary XPath expression; it's where you are evaluating an
expression that you *know* returns string (ie, it has name() or some
other string returning function at top level) and you'd like a string,
thanks, not something that you have to de-escape in order to get a
string.

Of course this use-case could also be addressed by providing a de-escape
function, but I don't really think that de_escape(xpath(...)[1]) is a
particularly friendly or efficient notation.

Now, these statements are not arguments against the patch --- as is,
XPATH() is entirely useless for expressions that return scalars, and
with the patch it at least does something usable.  So I think we should
apply it.  But there is room here for more function(s) to provide more
convenient functionality for the special case of expressions returning
strings.  I'd be inclined to provide xpath_number and xpath_bool too,
although those are less essential since a cast will get the job done.

There's some code-style aspects I don't care for in the submitted patch,
but I'll go clean those up and commit it.

                        regards, tom lane

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to