Re: [PATCH] Add XMLEXISTS function from the SQL/XML standard

I've been reading the SQL/XML standard and discovered that it defines a
function named XMLEXISTS that does exactly what the todo item
xpath_exists defines. My original patch named the function as per the
todo but I think using the function name from the standard is a better
idea. So this patch is the same as before, but the function is now named
XMLEXISTS instead of xpath_exists.

Regards,

--
Mike Fowler
Registered Linux user: 379787

On Tue, May 25, 2010 16:31, Mike Fowler wrote:
> I've been reading the SQL/XML standard and discovered that it defines a
> function named XMLEXISTS that does exactly what the todo item
> xpath_exists defines. My original patch named the function as per the
> todo but I think using the function name from the standard is a better
> idea. So this patch is the same as before, but the function is now named
> XMLEXISTS instead of xpath_exists.
>

I tried this path (cvs HEAD, applies without error), but get this error:

[...]
utils/adt/xml.o: In function `xmlexists':
/var/data1/pg_stuff/pg_sandbox/pgsql.xmlexists/src/backend/utils/adt/xml.c:3639: undefined
reference to `xmlXPathCompiledEvalToBoolean'
collect2: ld returned 1 exit status
make[2]: *** [postgres] Error 1
make[2]: Leaving directory `/var/data1/pg_stuff/pg_sandbox/pgsql.xmlexists/src/backend'
make[1]: *** [all] Error 2
make[1]: Leaving directory `/var/data1/pg_stuff/pg_sandbox/pgsql.xmlexists/src'
make: *** [all] Error 2

./configure --prefix=/var/data1/pg_stuff/pg_installations/pgsql.xmlexists --with-pgport=6548
--quiet --enable-depend --enable-cassert --enable-debug --with-openssl --with-perl --with-libxml
--with-libxslt

centos 5.4 2.6.18-164.el5 x86_64 GNU/Linux
libxml2.x86_64 2.6.26-2.1.2.8 installed
libxml2-devel.x86_64 2.6.26-2.1.2.8 installed

Erik Rijkers

Erik Rijkers wrote:
> libxml2.x86_64 2.6.26-2.1.2.8 installed
> libxml2-devel.x86_64 2.6.26-2.1.2.8 installed
>

Thanks for testing my patch Erik. It turns out I've got libxml2
installed at version 2.7.5. Searching the gnome mailing lists, it turns
out xmlXPathCompiledEvalToBoolean was added (unbelievably) in the very
next version from yours, 2.6.27 (see:
http://mail.gnome.org/archives/xml/2006-October/msg00119.html).

Regards,

--
Mike Fowler
Registered Linux user: 379787

On Tue, May 25, 2010 at 12:04 PM, Mike Fowler <mike(at)mlfowler(dot)com> wrote:
> Erik Rijkers wrote:
>>
>> libxml2.x86_64          2.6.26-2.1.2.8  installed
>> libxml2-devel.x86_64    2.6.26-2.1.2.8  installed
>>
>
> Thanks for testing my patch Erik. It turns out I've got libxml2 installed at
> version 2.7.5. Searching the gnome mailing lists, it turns out
> xmlXPathCompiledEvalToBoolean was added (unbelievably) in the very next
> version from yours, 2.6.27 (see:
> http://mail.gnome.org/archives/xml/2006-October/msg00119.html).

We're unlikely to accept this patch if it changes the minimum version
of libxml2 required to compile PostgreSQL.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company

Robert Haas wrote:
> On Tue, May 25, 2010 at 12:04 PM, Mike Fowler <mike(at)mlfowler(dot)com> wrote:
>
>> Erik Rijkers wrote:
>>
>>> libxml2.x86_64 2.6.26-2.1.2.8 installed
>>> libxml2-devel.x86_64 2.6.26-2.1.2.8 installed
>>>
>>>
>> Thanks for testing my patch Erik. It turns out I've got libxml2 installed at
>> version 2.7.5. Searching the gnome mailing lists, it turns out
>> xmlXPathCompiledEvalToBoolean was added (unbelievably) in the very next
>> version from yours, 2.6.27 (see:
>> http://mail.gnome.org/archives/xml/2006-October/msg00119.html).
>>
>
> We're unlikely to accept this patch if it changes the minimum version
> of libxml2 required to compile PostgreSQL

Why? 2.6.27 is almost 4 years old.

I realise that my patch didn't update configure and configure.in, and
indeed I didn't think of it when I responded to Erik (I'm too used to
the Java world where people manage their own dependencies). I've now
attached the updated patch which ups the check from version 2.6.23 to
2.6.27.

Regards,

--
Mike Fowler
Registered Linux user: 379787

On Tue, May 25, 2010 at 1:09 PM, Mike Fowler <mike(at)mlfowler(dot)com> wrote:
>> We're unlikely to accept this patch if it changes the minimum version
>> of libxml2 required to compile PostgreSQL
>
> Why? 2.6.27 is almost 4 years old.

Because we work hard to minimize our dependencies and make them as
non-onerous as possible.

At a minimum, I think it's fair to say that the burden is on you to
justify what it's worth bumping the version number. If there is some
major speed or performance advantage to using the newer API, maybe
we'll consider it. But if it's just a few extra lines of code to work
around it, then it's better to write those extra lines of code rather
than potentially force users to upgrade packages they're otherwise
happy with.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company

On 05/25/2010 01:09 PM, Mike Fowler wrote:
> Why? 2.6.27 is almost 4 years old.

RHEL 5 ships with 2.6.26. I imagine that supporting it is very
desirable, regardless of its age, since that is unfortunately still the
latest version of RHEL.

-- m. tharp

Robert Haas wrote:
> On Tue, May 25, 2010 at 1:09 PM, Mike Fowler <mike(at)mlfowler(dot)com> wrote:
>
>>> We're unlikely to accept this patch if it changes the minimum version
>>> of libxml2 required to compile PostgreSQL
>>>
>> Why? 2.6.27 is almost 4 years old.
>>
>
> Because we work hard to minimize our dependencies and make them as
> non-onerous as possible.
>
> At a minimum, I think it's fair to say that the burden is on you to
> justify what it's worth bumping the version number. If there is some
> major speed or performance advantage to using the newer API, maybe
> we'll consider it. But if it's just a few extra lines of code to work
> around it, then it's better to write those extra lines of code rather
> than potentially force users to upgrade packages they're otherwise
> happy with.
>
>

The real issue is what's going to be available on most of the platforms
we build on. Unfortunately, 2.6.26 is what's on my CentOS 5.4 boxes, for
example. I'm sure we don't want to make 9.1 not buildable with the
installed libraries on still fairly current RedHat-derived platforms.

cheers

andrew

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> On Tue, May 25, 2010 at 1:09 PM, Mike Fowler <mike(at)mlfowler(dot)com> wrote:
>>> We're unlikely to accept this patch if it changes the minimum version
>>> of libxml2 required to compile PostgreSQL
>>
>> Why? 2.6.27 is almost 4 years old.

> Because we work hard to minimize our dependencies and make them as
> non-onerous as possible.

> At a minimum, I think it's fair to say that the burden is on you to
> justify what it's worth bumping the version number.

Yes. Increasing the minimum required version of some library is a Big
Deal, we don't do it on a whim. And we definitely don't do it just
because it's old.

regards, tom lane

Tom Lane wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>
>> On Tue, May 25, 2010 at 1:09 PM, Mike Fowler <mike(at)mlfowler(dot)com> wrote:
>>
>>>> We're unlikely to accept this patch if it changes the minimum version
>>>> of libxml2 required to compile PostgreSQL
>>>>
>>> Why? 2.6.27 is almost 4 years old.
>> At a minimum, I think it's fair to say that the burden is on you to
>> justify what it's worth bumping the version number.
>>
>
> Yes. Increasing the minimum required version of some library is a Big
> Deal, we don't do it on a whim. And we definitely don't do it just
> because it's old.
>
> regards, tom lane
>
>

OK, I consider myself suitably educated/chastised. I now understand why
a version bump is such a big deal. Your objections are all reasonable, I
suppose I'm just used to living on the bleeding edge of everything.
Consequently I have changed the code to produce the same result in a
different way without using the new function. I've down-graded my
version to 2.6.26 and it all compiles cleanly. Please find attached my
revised patch, and thanks all for your advise.

Regards,

--
Mike Fowler
Registered Linux user: 379787

On tis, 2010-05-25 at 15:31 +0100, Mike Fowler wrote:
> I've been reading the SQL/XML standard and discovered that it defines a
> function named XMLEXISTS that does exactly what the todo item
> xpath_exists defines. My original patch named the function as per the
> todo but I think using the function name from the standard is a better
> idea. So this patch is the same as before, but the function is now named
> XMLEXISTS instead of xpath_exists.

The XMLEXISTS function works with XQuery expressions and doesn't have
the call signature that your patch implements.

Peter Eisentraut wrote:
> On tis, 2010-05-25 at 15:31 +0100, Mike Fowler wrote:
>
>> I've been reading the SQL/XML standard and discovered that it defines a
>> function named XMLEXISTS that does exactly what the todo item
>> xpath_exists defines. My original patch named the function as per the
>> todo but I think using the function name from the standard is a better
>> idea. So this patch is the same as before, but the function is now named
>> XMLEXISTS instead of xpath_exists.
>>
>
> The XMLEXISTS function works with XQuery expressions and doesn't have
> the call signature that your patch implements

Looking at the manuals of Oracle, Derby and DB2 I see how the call
signature differs. I also note that Oracle's implementation is XPath
only, Derby's is partial XQuery and DB2 appears to be full XQuery.

What do people prefer me to do? I see the options as:

1) Change the call signature to match the standard
2) Change the function name back to xpath_exists

Should option one be the more popular there's further choices:

1) Integrate XQuery support to completely match the standard, however
this will require the addition of a new library libxquery
2) Leave the XPath as is, inline with Oracle's implementation
3) Hybrid approach. Since XML is a comple time option, add XQuery as
another. Conditional completion gives the full XQuery support when
available or just the XPath when not

Thoughts?

--
Mike Fowler
Registered Linux user: 379787

On ons, 2010-05-26 at 11:47 +0100, Mike Fowler wrote:
> > The XMLEXISTS function works with XQuery expressions and doesn't have
> > the call signature that your patch implements
>
> Looking at the manuals of Oracle, Derby and DB2 I see how the call
> signature differs. I also note that Oracle's implementation is XPath
> only, Derby's is partial XQuery and DB2 appears to be full XQuery.
>
> What do people prefer me to do? I see the options as:
>
> 1) Change the call signature to match the standard
> 2) Change the function name back to xpath_exists

It would be nice to make XMLEXISTS work as in the standard, seeing how
many others are providing the same interface.

> Should option one be the more popular there's further choices:
>
> 1) Integrate XQuery support to completely match the standard, however
> this will require the addition of a new library libxquery
> 2) Leave the XPath as is, inline with Oracle's implementation
> 3) Hybrid approach. Since XML is a comple time option, add XQuery as
> another. Conditional completion gives the full XQuery support when
> available or just the XPath when not

I think providing XPath is enough, at least for now.

Peter Eisentraut wrote:
> On ons, 2010-05-26 at 11:47 +0100, Mike Fowler wrote:
>
>>> The XMLEXISTS function works with XQuery expressions and doesn't have
>>> the call signature that your patch implements
>>>
>> Looking at the manuals of Oracle, Derby and DB2 I see how the call
>> signature differs. I also note that Oracle's implementation is XPath
>> only, Derby's is partial XQuery and DB2 appears to be full XQuery.
>>
>> What do people prefer me to do? I see the options as:
>>
>> 1) Change the call signature to match the standard
>> 2) Change the function name back to xpath_exists
>>
>
> It would be nice to make XMLEXISTS work as in the standard, seeing how
> many others are providing the same interface.
>
>
>> Should option one be the more popular there's further choices:
>>
>> 1) Integrate XQuery support to completely match the standard, however
>> this will require the addition of a new library libxquery
>> 2) Leave the XPath as is, inline with Oracle's implementation
>> 3) Hybrid approach. Since XML is a comple time option, add XQuery as
>> another. Conditional completion gives the full XQuery support when
>> available or just the XPath when not
>>
>
> I think providing XPath is enough, at least for now

Agreed. I'll get another patch together in the next day or two.

Regards,

--
Mike Fowler
Registered Linux user: 379787

Mike Fowler wrote:
> Tom Lane wrote:
> > Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> >
> >> On Tue, May 25, 2010 at 1:09 PM, Mike Fowler <mike(at)mlfowler(dot)com> wrote:
> >>
> >>>> We're unlikely to accept this patch if it changes the minimum version
> >>>> of libxml2 required to compile PostgreSQL
> >>>>
> >>> Why? 2.6.27 is almost 4 years old.
> >> At a minimum, I think it's fair to say that the burden is on you to
> >> justify what it's worth bumping the version number.
> >>
> >
> > Yes. Increasing the minimum required version of some library is a Big
> > Deal, we don't do it on a whim. And we definitely don't do it just
> > because it's old.
> >
> > regards, tom lane
> >
> >
>
> OK, I consider myself suitably educated/chastised. I now understand why
> a version bump is such a big deal. Your objections are all reasonable, I
> suppose I'm just used to living on the bleeding edge of everything.
> Consequently I have changed the code to produce the same result in a
> different way without using the new function. I've down-graded my
> version to 2.6.26 and it all compiles cleanly. Please find attached my
> revised patch, and thanks all for your advise.

FYI, it is often good to add a comment in the C code about why you
didn't use the new XML function so if the issue comes up again, we know
why, and in 10 years, we can use it. ;-)

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

Quoting Peter Eisentraut <peter_e(at)gmx(dot)net>:

> It would be nice to make XMLEXISTS work as in the standard, seeing how
> many others are providing the same interface.
>
...
>
> I think providing XPath is enough, at least for now.
>

Hi Peter,

From piecing together sections 8.4 (<XML exists predicate>) and 6.18
(<XML query>), I believe the full call signature would be (apologies
if my bracketing isn't right):

XMLEXISTS
(
xquery_expression
[
PASSING {BY REF|BY VALUE}
{ (xml_expression [BY REF|BY VALUE])
| (xml_expression AS identifier)
}
[, ...]
]
)

Seeing as we're only intereseted in XPath for the moment and most of
the syntax above is only really relevant to XQuery, I would suggest
the following as accepatable until full XQuery support comes:

XMLEXISTS
(
xpath_expression
[
PASSING BY REF xml_expression [BY REF]
]
)

There is one drawback in this approach, namely xml namespace support.
I think borrowing the solution used in the xpath() (i.e. passing a
nsarray) is out of the question as it won't be future proofed against
full XQuery support. This is because the declaration of namespaces in
XQuery is actually part of the query itself. I also think it
inappropriate to attempt to parse the xpath_expression looking for
XQuery style namespace declartions. So I think we ignore declared
namespace support for the moment and just get the syntax correct ready
for XQuery support in the hopefully near future.

Do you agree with what I have proposed?

Regards,
--
Mike Fowler