xpath changes in the recent back branches

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

xpath changes in the recent back branches

Marko Tiikkaja-4
Hi,

Commit 79af9a1d2668c9edc8171f03c39e7fed571eeb98 changed xpath handling
with regard to namespaces, and it seems to be fixing an actual issue.
However, it was also backpatched to all branches despite it breaking for
example code like this:

do $$
declare
_x xml;
begin
_x := (xpath('/x:Foo/x:Bar', xml '<Foo
xmlns="teh:urn"><Bar><Baz>1</Baz><Bat>2</Bat></Bar></Foo>',
array[['x','teh:urn']]))[1];
raise notice '%', xpath('/Bar/Baz/text()', _x);
raise notice '%', xpath('/Bar/Bat/text()', _x);
end
$$;

The problem is that there's no way to write the code like this in such a
way that it would work on both versions.  If I add the namespace, it's
broken on 9.1.14.  Without it it's broken on 9.1.15.

I'm now thinking of adding a workaround which strips namespaces, but
that doesn't seem to be easy to do, even with PL/Perl.  Is there a
better workaround here that I'm not seeing?

I'm not sure how changing behavior like this in a minor release was
considered acceptable.


.m


--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|

Re: xpath changes in the recent back branches

Robert Haas
On Thu, Feb 19, 2015 at 5:53 AM, Marko Tiikkaja <[hidden email]> wrote:

> Commit 79af9a1d2668c9edc8171f03c39e7fed571eeb98 changed xpath handling with
> regard to namespaces, and it seems to be fixing an actual issue. However, it
> was also backpatched to all branches despite it breaking for example code
> like this:
>
> do $$
> declare
> _x xml;
> begin
> _x := (xpath('/x:Foo/x:Bar', xml '<Foo
> xmlns="teh:urn"><Bar><Baz>1</Baz><Bat>2</Bat></Bar></Foo>',
> array[['x','teh:urn']]))[1];
> raise notice '%', xpath('/Bar/Baz/text()', _x);
> raise notice '%', xpath('/Bar/Bat/text()', _x);
> end
> $$;
>
> The problem is that there's no way to write the code like this in such a way
> that it would work on both versions.  If I add the namespace, it's broken on
> 9.1.14.  Without it it's broken on 9.1.15.

That certainly sucks.

> I'm not sure how changing behavior like this in a minor release was
> considered acceptable.

I'm guessing that the fact that it changes behavior in cases like this
wasn't recognized, but I suppose Peter will have to be the one to
comment on that.

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


--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|

Re: xpath changes in the recent back branches

Tom Lane-2
Robert Haas <[hidden email]> writes:
> On Thu, Feb 19, 2015 at 5:53 AM, Marko Tiikkaja <[hidden email]> wrote:
>> I'm not sure how changing behavior like this in a minor release was
>> considered acceptable.

> I'm guessing that the fact that it changes behavior in cases like this
> wasn't recognized, but I suppose Peter will have to be the one to
> comment on that.

It was considered to be a bug fix; more, given the few complaints about
the clearly-broken old behavior, we thought it was a fix that would affect
few people, and them positively.  Sorry you're not happy about it, but
these things are always judgment calls.

                        regards, tom lane


--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|

Re: xpath changes in the recent back branches

Mike Rylander
In reply to this post by Marko Tiikkaja-4
On Thu, Feb 19, 2015 at 5:53 AM, Marko Tiikkaja <[hidden email]> wrote:
Hi,

Commit 79af9a1d2668c9edc8171f03c39e7fed571eeb98 changed xpath handling with regard to namespaces, and it seems to be fixing an actual issue. However, it was also backpatched to all branches despite it breaking for example code like this:

do $$
declare
_x xml;
begin
_x := (xpath('/x:Foo/x:Bar', xml '<Foo xmlns="teh:urn"><Bar><Baz>1</Baz><Bat>2</Bat></Bar></Foo>', array[['x','teh:urn']]))[1];
raise notice '%', xpath('/Bar/Baz/text()', _x);
raise notice '%', xpath('/Bar/Bat/text()', _x);
end
$$;

The problem is that there's no way to write the code like this in such a way that it would work on both versions.  If I add the namespace, it's broken on 9.1.14.  Without it it's broken on 9.1.15.

I'm now thinking of adding a workaround which strips namespaces, but that doesn't seem to be easy to do, even with PL/Perl.  Is there a better workaround here that I'm not seeing?

 
FWIW, I've been working around the bug fixed in that commit for ages by spelling my xpath like this:

  xpath('/*[local-name()="Bar"]/*[local-name()="Baz"]/text()', data)

I've modularized my XML handling functions so the source of 'data' is immaterial -- maybe it's a full document, maybe it's a fragment from a previous xpath() call -- and the referenced commit is going to make correct XPATH much more sane, readable, and maintainable.  I, for one, welcome it wholeheartedly.

HTH,

--Mike
Reply | Threaded
Open this post in threaded view
|

Re: xpath changes in the recent back branches

Marko Tiikkaja-4
In reply to this post by Tom Lane-2
On 3/4/15 5:26 PM, Tom Lane wrote:

> Robert Haas <[hidden email]> writes:
>> On Thu, Feb 19, 2015 at 5:53 AM, Marko Tiikkaja <[hidden email]> wrote:
>>> I'm not sure how changing behavior like this in a minor release was
>>> considered acceptable.
>
>> I'm guessing that the fact that it changes behavior in cases like this
>> wasn't recognized, but I suppose Peter will have to be the one to
>> comment on that.
>
> It was considered to be a bug fix; more, given the few complaints about
> the clearly-broken old behavior, we thought it was a fix that would affect
> few people, and them positively.

Yeah, but these things usually go the other way.  "This has been broken
for 10 years but nobody noticed, so we're not going to fix this" is the
answer I'm more used to seeing.  And frankly, even though I remember
getting the wrong end of that hose a few times, I prefer that to
breaking apps in minor releases.


.m


--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|

Re: xpath changes in the recent back branches

Marko Tiikkaja-4
On 3/4/15 6:19 PM, I wrote:
> On 3/4/15 5:26 PM, Tom Lane wrote:
>> It was considered to be a bug fix; more, given the few complaints about
>> the clearly-broken old behavior, we thought it was a fix that would affect
>> few people, and them positively.
>
> Yeah, but these things usually go the other way.  "This has been broken
> for 10 years but nobody noticed, so we're not going to fix this"

Sorry, that should have said "we're not going to fix this *in the back
branches*".


.m


--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|

Re: xpath changes in the recent back branches

Peter Eisentraut-2
On 3/4/15 12:20 PM, Marko Tiikkaja wrote:

> On 3/4/15 6:19 PM, I wrote:
>> On 3/4/15 5:26 PM, Tom Lane wrote:
>>> It was considered to be a bug fix; more, given the few complaints about
>>> the clearly-broken old behavior, we thought it was a fix that would
>>> affect
>>> few people, and them positively.
>>
>> Yeah, but these things usually go the other way.  "This has been broken
>> for 10 years but nobody noticed, so we're not going to fix this"
>
> Sorry, that should have said "we're not going to fix this *in the back
> branches*".

I tend to agree.  I was not in favor of backpatching, but other people
were in favor of it, and no one spoke up against it.

That said, if I understand your test case correctly, this would
basically be an argument against any semantic corrections in stable
releases, since user code could expect to continue to work with the
previous incorrect value.

You can always test the server version number, and you'll have to for
the next major release anyway.



--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|

Re: xpath changes in the recent back branches

Robert Haas
On Wed, Mar 4, 2015 at 4:11 PM, Peter Eisentraut <[hidden email]> wrote:
> That said, if I understand your test case correctly, this would
> basically be an argument against any semantic corrections in stable
> releases, since user code could expect to continue to work with the
> previous incorrect value.
>
> You can always test the server version number, and you'll have to for
> the next major release anyway.

That's not really the problem, of course.  The problem is you upgrade
and your app unexpectedly breaks.  The complexity of fixing that once
it's happened is not entirely irrelevant, but it's not really the main
problem, either.

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


--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|

Re: xpath changes in the recent back branches

Peter Eisentraut-2
On 3/4/15 5:00 PM, Robert Haas wrote:
>> You can always test the server version number, and you'll have to for
>> the next major release anyway.
>
> That's not really the problem, of course.  The problem is you upgrade
> and your app unexpectedly breaks.  The complexity of fixing that once
> it's happened is not entirely irrelevant, but it's not really the main
> problem, either.

That paragraph was mainly a response to the claim that there is "no way"
to make it work in both old and new versions.




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