xpath changes in the recent back branches

On Thu, Feb 19, 2015 at 5:53 AM, Marko Tiikkaja <marko@joh.to> 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 (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Robert Haas <robertmhaas@gmail.com> writes:

On Thu, Feb 19, 2015 at 5:53 AM, Marko Tiikkaja <marko@joh.to> 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 (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Thu, Feb 19, 2015 at 5:53 AM, Marko Tiikkaja <marko@joh.to> 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

On 3/4/15 5:26 PM, Tom Lane wrote:

Robert Haas <robertmhaas@gmail.com> writes:

On Thu, Feb 19, 2015 at 5:53 AM, Marko Tiikkaja <marko@joh.to> 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 (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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 (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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 (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Wed, Mar 4, 2015 at 4:11 PM, Peter Eisentraut <peter_e@gmx.net> 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 (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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 (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers