Re: 9.0 release notes done

I have completed the 9.0 release notes:

http://developer.postgresql.org/pgdocs/postgres/release-9-0.html

I kept the 9.0-alpha release notes in the SGML because people might want
to compare them with the release notes I did, and because the
introductory text will be needed for the next alpha. Eventually we will
want to trim the alpha SGML and perhaps place it in a large comment
block for use for 9.1 alphas.

Interestingly the 9.0 release notes contain 201 items, while the 8.4
release notes contained 314 items. Of course we will be adding a few
more 9.0 items before 9.0 final, but not a lot. The only explanation I
can think of is that we were more focused during this release, and there
were fewer minor cleanups. The migration issues section, for example,
was significantly smaller than in 8.4.

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

PG East: http://www.enterprisedb.com/community/nav-pg-east-2010.do

2010/3/20 Bruce Momjian <bruce(at)momjian(dot)us>:
> I have completed the 9.0 release notes:
>
>        http://developer.postgresql.org/pgdocs/postgres/release-9-0.html
>

I wonder if we need note a minor compatibility from extending window
function's frame.

- Change BETWEEN from TYPE_FUNC_NAME_KEYWORD from COL_NAME_KEYWORD

Regards,

--
Hitoshi Harada

On Sat, Mar 20, 2010 at 12:02 AM, Bruce Momjian <bruce(at)momjian(dot)us> wrote:
> I have completed the 9.0 release notes:
>
>        http://developer.postgresql.org/pgdocs/postgres/release-9-0.html
>
> I kept the 9.0-alpha release notes in the SGML because people might want
> to compare them with the release notes I did, and because the
> introductory text will be needed for the next alpha.  Eventually we will
> want to trim the alpha SGML and perhaps place it in a large comment
> block for use for 9.1 alphas.
>
> Interestingly the 9.0 release notes contain 201 items, while the 8.4
> release notes contained 314 items.  Of course we will be adding a few
> more 9.0 items before 9.0 final, but not a lot.  The only explanation I
> can think of is that we were more focused during this release, and there
> were fewer minor cleanups.  The migration issues section, for example,
> was significantly smaller than in 8.4.

Cool. Thanks for getting this done!

I think we need you and Tom and other senior community members to
weigh in a little more overtly on which of the remaining open items
should get fixed prior to 9.0beta. The biggest thing that is holding
us up right now seems to be that we don't really know what we're
waiting for. If we start talking about it, we might collectively make
the wrong decision; but that's surely better than making no decision
and letting things drag out.

...Robert

Hitoshi Harada wrote:
> 2010/3/20 Bruce Momjian <bruce(at)momjian(dot)us>:
> > I have completed the 9.0 release notes:
> >
> > ? ? ? ?http://developer.postgresql.org/pgdocs/postgres/release-9-0.html
> >
>
> I wonder if we need note a minor compatibility from extending window
> function's frame.
>
> - Change BETWEEN from TYPE_FUNC_NAME_KEYWORD from COL_NAME_KEYWORD

I see. The change appears to be from "can be function or type name" to
"cannot be function or type name", according to
misc.c::pg_get_keywords().

What error will they see if they do use an invalid name? Will it be
clear that they just need to rename it?

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

PG East: http://www.enterprisedb.com/community/nav-pg-east-2010.do

Robert Haas wrote:
> > Interestingly the 9.0 release notes contain 201 items, while the 8.4
> > release notes contained 314 items. ?Of course we will be adding a few
> > more 9.0 items before 9.0 final, but not a lot. ?The only explanation I
> > can think of is that we were more focused during this release, and there
> > were fewer minor cleanups. ?The migration issues section, for example,
> > was significantly smaller than in 8.4.
>
> Cool. Thanks for getting this done!
>
> I think we need you and Tom and other senior community members to
> weigh in a little more overtly on which of the remaining open items
> should get fixed prior to 9.0beta. The biggest thing that is holding
> us up right now seems to be that we don't really know what we're
> waiting for. If we start talking about it, we might collectively make
> the wrong decision; but that's surely better than making no decision
> and letting things drag out.

Well, Tom and I have already posted publicly about it. There is nothing
that either us see on the 9.0 "Bugs" open items list that would delay a
beta:

http://wiki.postgresql.org/wiki/PostgreSQL_9.0_Open_Items

The real wildcard is HS and SR. I asked publicly if we thought we could
release a beta while they were known to be incomplete/broken, and
several replied that we could not, so it appears we are waiting on the
group focusing on those features to say they are ready.

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

PG East: http://www.enterprisedb.com/community/nav-pg-east-2010.do

Bruce Momjian wrote:
> I have completed the 9.0 release notes:
>
> http://developer.postgresql.org/pgdocs/postgres/release-9-0.html
>
> Interestingly the 9.0 release notes contain 201 items, while the 8.4
> release notes contained 314 items. Of course we will be adding a few
> more 9.0 items before 9.0 final, but not a lot. The only explanation I
> can think of is that we were more focused during this release, and there
> were fewer minor cleanups. The migration issues section, for example,
> was significantly smaller than in 8.4.

I did some research on release note item counts for the past several
major releases and found 8.4 to be an abberation:

release-7.4.sgml
263
release-8.0.sgml
230
release-8.1.sgml
174
release-8.2.sgml
215
release-8.3.sgml
214
release-8.4.sgml
314
release-9.0.sgml
201

The 9.0 release item count closely matches the item counts from all
previous major releases, except 8.4. I think 8.4 was a
cleanup/restructuring release and that resulted in a high item count.
(The only two major new 8.4 features were window functions and common
table expressions/recursive queries.)

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

PG East: http://www.enterprisedb.com/community/nav-pg-east-2010.do

Bruce Momjian <bruce(at)momjian(dot)us> writes:
> Robert Haas wrote:
>> I think we need you and Tom and other senior community members to
>> weigh in a little more overtly on which of the remaining open items
>> should get fixed prior to 9.0beta.

> Well, Tom and I have already posted publicly about it. There is nothing
> that either us see on the 9.0 "Bugs" open items list that would delay a
> beta:
> http://wiki.postgresql.org/wiki/PostgreSQL_9.0_Open_Items

The traditional gating factor for going to beta is whether we believe
we are done making initdb-forcing catalog changes. Now the availability
of pg_migrator should lessen the pain of an initdb for beta users, so
that argument is maybe weaker than it used to be; but if we suppose that
that's still the standard then:

* I don't see any likelihood of an initdb being forced by fixes for the
non-HS/SR changes in 9.0.

* Most of the foreseeable flux from HS/SR seems to me to be at the level
of WAL entries not system catalogs. (Yesterday's fixes are unlikely to
be the end of that...) In principle we could support a WAL content
change without initdb, by instructing beta users to do a clean database
shutdown and then run pg_resetxlog when upgrading. In practice that
might be a bit shaky --- I don't remember if pg_resetxlog can get all
its info from pg_control without having to look into the old WAL
segments. It might be worth spending a bit of time to test that
procedure and see if we need to clean anything up.

* The only catalog change I can see coming from HS/SR is possible
additions of new inquiry/control functions. We have several proposals
for such on the table. Getting those in, if we're going to, is
therefore a "must fix for beta" item.

regards, tom lane

Bruce Momjian wrote:
> Well, Tom and I have already posted publicly about it. There is nothing
> that either us see on the 9.0 "Bugs" open items list that would delay a
> beta:
>
> http://wiki.postgresql.org/wiki/PostgreSQL_9.0_Open_Items
>

I have just been looking at the xmlconcat bug on that list. I can't
think of any better solution than parsing the resulting string to make
sure it is well-formed before we return, with lines something like this:

xmlDocPtr doc;
xmltype *str = stringinfo_to_xmltype(&buf);
doc = xml_parse(str, xmloption, true, GetDatabaseEncoding());
xmlFreeDoc(doc);

That's surely going to affect the performance of xmlconcat, not sure how
much.

Does anyone have a better suggestion?

cheers

andrew

On 3/19/10 9:02 PM, Bruce Momjian wrote:
> I have completed the 9.0 release notes:
>
> http://developer.postgresql.org/pgdocs/postgres/release-9-0.html
>
> I kept the 9.0-alpha release notes in the SGML because people might want
> to compare them with the release notes I did, and because the
> introductory text will be needed for the next alpha. Eventually we will
> want to trim the alpha SGML and perhaps place it in a large comment
> block for use for 9.1 alphas.

I'm going to paste these into the wiki so that I can edit them for
language and style.

--
-- Josh Berkus
PostgreSQL Experts Inc.
http://www.pgexperts.com

Tom, Bruce,

I'd favor a beta sooner rather than later even if some stuff is still in
flux. This particular release needs as much testing as possible, and
10x as many people will try a beta as an alpha.

--
-- Josh Berkus
PostgreSQL Experts Inc.
http://www.pgexperts.com

Josh Berkus <josh(at)agliodbs(dot)com> writes:
> I'd favor a beta sooner rather than later even if some stuff is still in
> flux. This particular release needs as much testing as possible, and
> 10x as many people will try a beta as an alpha.

Well, the reason they are willing to try a beta is that it's supposed to
be more stable than an alpha. If we pull back on our stability
commitments (like "no further initdbs expected") we'll just discourage
people from trying betas in future.

regards, tom lane

Josh Berkus wrote:
> On 3/19/10 9:02 PM, Bruce Momjian wrote:
> > I have completed the 9.0 release notes:
> >
> > http://developer.postgresql.org/pgdocs/postgres/release-9-0.html
> >
> > I kept the 9.0-alpha release notes in the SGML because people might want
> > to compare them with the release notes I did, and because the
> > introductory text will be needed for the next alpha. Eventually we will
> > want to trim the alpha SGML and perhaps place it in a large comment
> > block for use for 9.1 alphas.
>
> I'm going to paste these into the wiki so that I can edit them for
> language and style.

I am unclear how you expect to merge this back into SGML from a wiki.
Also consider the SGML will be modified regularly from now on.

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

PG East: http://www.enterprisedb.com/community/nav-pg-east-2010.do

Tom Lane wrote:
> Josh Berkus <josh(at)agliodbs(dot)com> writes:
> > I'd favor a beta sooner rather than later even if some stuff is still in
> > flux. This particular release needs as much testing as possible, and
> > 10x as many people will try a beta as an alpha.
>
> Well, the reason they are willing to try a beta is that it's supposed to
> be more stable than an alpha. If we pull back on our stability
> commitments (like "no further initdbs expected") we'll just discourage
> people from trying betas in future.

Agreed. If you want beta earlier, we are going to need to close the
HS/SR issues sooner.

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

PG East: http://www.enterprisedb.com/community/nav-pg-east-2010.do

2010/3/20 Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>:
> Josh Berkus <josh(at)agliodbs(dot)com> writes:
>> I'd favor a beta sooner rather than later even if some stuff is still in
>> flux.  This particular release needs as much testing as possible, and
>> 10x as many people will try a beta as an alpha.
>
> Well, the reason they are willing to try a beta is that it's supposed to
> be more stable than an alpha.  If we pull back on our stability
> commitments (like "no further initdbs expected") we'll just discourage
> people from trying betas in future.

I agree with Tom - two weeks are nothing against to an lost of credit and trust.

Pavel
>
>                        regards, tom lane
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>

2010/3/21 Bruce Momjian <bruce(at)momjian(dot)us>:
> Hitoshi Harada wrote:
>> 2010/3/20 Bruce Momjian <bruce(at)momjian(dot)us>:
>> > I have completed the 9.0 release notes:
>> >
>> > ? ? ? ?http://developer.postgresql.org/pgdocs/postgres/release-9-0.html
>> >
>>
>> I wonder if we need note a minor compatibility from extending window
>> function's frame.
>>
>> - Change BETWEEN from TYPE_FUNC_NAME_KEYWORD from COL_NAME_KEYWORD
>
> I see.  The change appears to be from "can be function or type name" to
> "cannot be function or type name", according to
> misc.c::pg_get_keywords().
>
> What error will they see if they do use an invalid name?  Will it be
> clear that they just need to rename it?

No, it's only parser error as other syntactic changes.

# 9.0
regression=# create or replace function between(i int) returns int as
$$ select $1 + $1 $$ language sql;
ERROR: syntax error at or near "("
LINE 1: create or replace function between(i int) returns int as $$ ...

whereas 8.4 can create it successfully.

This is still ok, as well as 8.4.

regression=# select 1 as between;
between
---------
1
(1 row)

Regards,

--
Hitoshi Harada

Hitoshi Harada wrote:
> 2010/3/21 Bruce Momjian <bruce(at)momjian(dot)us>:
> > Hitoshi Harada wrote:
> >> 2010/3/20 Bruce Momjian <bruce(at)momjian(dot)us>:
> >> > I have completed the 9.0 release notes:
> >> >
> >> > ? ? ? ?http://developer.postgresql.org/pgdocs/postgres/release-9-0.html
> >> >
> >>
> >> I wonder if we need note a minor compatibility from extending window
> >> function's frame.
> >>
> >> - Change BETWEEN from TYPE_FUNC_NAME_KEYWORD from COL_NAME_KEYWORD
> >
> > I see. ?The change appears to be from "can be function or type name" to
> > "cannot be function or type name", according to
> > misc.c::pg_get_keywords().
> >
> > What error will they see if they do use an invalid name? ?Will it be
> > clear that they just need to rename it?
>
> No, it's only parser error as other syntactic changes.
>
> # 9.0
> regression=# create or replace function between(i int) returns int as
> $$ select $1 + $1 $$ language sql;
> ERROR: syntax error at or near "("
> LINE 1: create or replace function between(i int) returns int as $$ ...
>
> whereas 8.4 can create it successfully.
>
> This is still ok, as well as 8.4.
>
> regression=# select 1 as between;
> between
> ---------
> 1
> (1 row)

Oh, I see now. They keyword BETWEEN had to be changed for window
functions, not that window function behavior would trigger the error.

OK. We normally don't record changes in the category of keywords in the
release notes unless it is a keyword that we would expect to cause
trouble. This is particularly true for keywords that are common for
function names but are not well known as SQL keywords. I don't think
this case has to be recorded in the release notes. If someone reports
the problem during beta we can revisit the idea.

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

PG East: http://www.enterprisedb.com/community/nav-pg-east-2010.do

Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
>> http://wiki.postgresql.org/wiki/PostgreSQL_9.0_Open_Items

> I have just been looking at the xmlconcat bug on that list. I can't
> think of any better solution than parsing the resulting string to make
> sure it is well-formed before we return,

That might be a reasonable thing to do as a safety check, but I can't
escape the feeling that what this fundamentally is is a data typing
error, traceable to the lack of differentiation between xml documents
and xml fragments. Is there a way to attack it based on saying that the
inputs can't be documents, or stripping the document overhead if they are?

regards, tom lane

Tom Lane wrote:
> Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
>
>>> http://wiki.postgresql.org/wiki/PostgreSQL_9.0_Open_Items
>>>
>
>
>> I have just been looking at the xmlconcat bug on that list. I can't
>> think of any better solution than parsing the resulting string to make
>> sure it is well-formed before we return,
>>
>
> That might be a reasonable thing to do as a safety check, but I can't
> escape the feeling that what this fundamentally is is a data typing
> error, traceable to the lack of differentiation between xml documents
> and xml fragments. Is there a way to attack it based on saying that the
> inputs can't be documents, or stripping the document overhead if they are?
>

Yeah, maybe. According to
<http://www.w3.org/TR/REC-DOM-Level-1/level-one-core.html> the only
legal child of an XML Document node that is not also a legal child of a
DocumentFragment node is a DocumentType node. So we could probably just
look for one of those in each argument node and strip it out. That
should be fairly lightweight in the common case where it's not present -
we'd just be searching for a fixed string. Removing it if found would be
more complex. We'd have to parse the node to remove it, since a legal
DocumentType node string could appear legally inside a CDATA node.

That has the advantage that it would fix the error rather than failing,
but I'm slightly nervous about silently mangling user supplied XML. I
guess we do that in a few other cases to make other combinations
function sanely.

cheers

andrew

On sön, 2010-03-21 at 13:07 -0400, Andrew Dunstan wrote:
> Yeah, maybe. According to
> <http://www.w3.org/TR/REC-DOM-Level-1/level-one-core.html> the only
> legal child of an XML Document node that is not also a legal child of a
> DocumentFragment node is a DocumentType node. So we could probably just
> look for one of those in each argument node and strip it out. That
> should be fairly lightweight in the common case where it's not present -
> we'd just be searching for a fixed string. Removing it if found would be
> more complex. We'd have to parse the node to remove it, since a legal
> DocumentType node string could appear legally inside a CDATA node.

According to the SQL/XML standard, the document type declaration should
apparently be stripped when doing a concatenation. (This makes sense
because the result of a concatenation can never be valid according to a
DTD.)

But if we are not comfortable about being able to do that safely, I
would be OK with just raising an error if a concatenation is attempted
where one value contains a DTD. The impact in practice should be low.

On Sat, Mar 20, 2010 at 5:02 AM, Bruce Momjian <bruce(at)momjian(dot)us> wrote:
> Interestingly the 9.0 release notes contain 201 items, while the 8.4
> release notes contained 314 items.

Is the following pg_dump change covered by the release notes? I
couldn't find it. It was the last committed patch from the 2010-01
commitfest...

http://archives.postgresql.org/pgsql-committers/2010-02/msg00233.php

https://commitfest.postgresql.org/action/patch_view?id=247

Joachim

Joachim Wieland wrote:
> On Sat, Mar 20, 2010 at 5:02 AM, Bruce Momjian <bruce(at)momjian(dot)us> wrote:
> > Interestingly the 9.0 release notes contain 201 items, while the 8.4
> > release notes contained 314 items.
>
> Is the following pg_dump change covered by the release notes? I
> couldn't find it. It was the last committed patch from the 2010-01
> commitfest...
>
> http://archives.postgresql.org/pgsql-committers/2010-02/msg00233.php
>
> https://commitfest.postgresql.org/action/patch_view?id=247

pg_dump support is part of the larger release item:

Add the ability to control large object permissions with
GRANT/REVOKE(KaiGai Kohei)

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

PG East: http://www.enterprisedb.com/community/nav-pg-east-2010.do

Peter Eisentraut wrote:
> On sön, 2010-03-21 at 13:07 -0400, Andrew Dunstan wrote:
>
>> Yeah, maybe. According to
>> <http://www.w3.org/TR/REC-DOM-Level-1/level-one-core.html> the only
>> legal child of an XML Document node that is not also a legal child of a
>> DocumentFragment node is a DocumentType node. So we could probably just
>> look for one of those in each argument node and strip it out. That
>> should be fairly lightweight in the common case where it's not present -
>> we'd just be searching for a fixed string. Removing it if found would be
>> more complex. We'd have to parse the node to remove it, since a legal
>> DocumentType node string could appear legally inside a CDATA node.
>>
>
> According to the SQL/XML standard, the document type declaration should
> apparently be stripped when doing a concatenation. (This makes sense
> because the result of a concatenation can never be valid according to a
> DTD.)
>
> But if we are not comfortable about being able to do that safely, I
> would be OK with just raising an error if a concatenation is attempted
> where one value contains a DTD. The impact in practice should be low.
>

Right. Can you find a way to do that using the libxml API? I haven't
managed to, and I'm pretty sure I can construct XML that fails every
simple string search test I can think of, either with a false negative
or a false positive.

cheers

andrew

Bruce Momjian <bruce(at)momjian(dot)us> wrote:

> I have completed the 9.0 release notes:
> http://developer.postgresql.org/pgdocs/postgres/release-9-0.html

There is an additional incompatibilitiy in pg_largeobject catalog.
We need to rewrite queries to test existences of large objests
from SELECT DISTINCT(loid) FROM pg_largeobject
to SELECT oid FROM pg_largeobject_metadata
because an empty large object doesn't have rows in pg_largeobject.

Regards,
---
Takahiro Itagaki
NTT Open Source Software Center

Bruce,

I thought this year we were going to start using people's full names
instead of the first names, for clarity. No?

--
-- Josh Berkus
PostgreSQL Experts Inc.
http://www.pgexperts.com

On Tue, Mar 23, 2010 at 1:09 PM, Josh Berkus <josh(at)agliodbs(dot)com> wrote:
> I thought this year we were going to start using people's full names
> instead of the first names, for clarity.  No?

+1 for that approach.

...Robert

On mån, 2010-03-22 at 19:38 -0400, Andrew Dunstan wrote:
> > But if we are not comfortable about being able to do that safely, I
> > would be OK with just raising an error if a concatenation is
> attempted
> > where one value contains a DTD. The impact in practice should be
> low.
> >
>
> Right. Can you find a way to do that using the libxml API? I haven't
> managed to, and I'm pretty sure I can construct XML that fails every
> simple string search test I can think of, either with a false negative
> or a false positive.

The documentation on that is terse as usual. In any case, you will need
to XML parse the input values, and so you might as well resort to
parsing the output value to see if it is well-formed, which should catch
this mistake and possibly others.

Josh Berkus wrote:
> Bruce,
>
> I thought this year we were going to start using people's full names
> instead of the first names, for clarity. No?

OK, I will do this once Josh is done with his modifications.

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

PG East: http://www.enterprisedb.com/community/nav-pg-east-2010.do

Peter Eisentraut wrote:
> On mån, 2010-03-22 at 19:38 -0400, Andrew Dunstan wrote:
>
>>> But if we are not comfortable about being able to do that safely, I
>>> would be OK with just raising an error if a concatenation is
>>>
>> attempted
>>
>>> where one value contains a DTD. The impact in practice should be
>>>
>> low.
>>
>>>
>>>
>> Right. Can you find a way to do that using the libxml API? I haven't
>> managed to, and I'm pretty sure I can construct XML that fails every
>> simple string search test I can think of, either with a false negative
>> or a false positive.
>>
>
> The documentation on that is terse as usual. In any case, you will need
> to XML parse the input values, and so you might as well resort to
> parsing the output value to see if it is well-formed, which should catch
> this mistake and possibly others.
>
>

Actually, I have come to the conclusion that the biggest problem in this
area is that we accept XML documents with a leading DOCTYPE node at all.
Our docs state:

The xml type can store well-formed "documents", as defined by the
XML standard, as well as "content" fragments, which are defined by
the production XMLDecl? content in the XML standard.

A document with a leading DOCTYPE node matches neither of these rules,
and when we strip the XMLDecl from a piece of XML where it's followed by
a DOCTYPE node we turn something that is legal XML into something that
isn't, even by our own (or possibly the standard's) relaxed definition.
A doctypedecl can only follow an XMLDecl, see
<http://www.w3.org/TR/2006/REC-xml11-20060816/#sec-prolog-dtd>.

So I think we need to go back to the drawing board a bit, rather than
patch a particular reported error case. But these problems are not at
all new to 9.0, and coming up to beta as I hope we are is not the time
for it. I think it will have to wait to 9.1.

cheers

andrew

On ons, 2010-03-24 at 14:51 -0400, Andrew Dunstan wrote:
> Actually, I have come to the conclusion that the biggest problem in
> this
> area is that we accept XML documents with a leading DOCTYPE node at
> all.
> Our docs state:
>
> The xml type can store well-formed "documents", as defined by the
> XML standard, as well as "content" fragments, which are defined by
> the production XMLDecl? content in the XML standard.
>
> A document with a leading DOCTYPE node matches neither of these
> rules,
> and when we strip the XMLDecl from a piece of XML where it's followed
> by
> a DOCTYPE node we turn something that is legal XML into something
> that
> isn't, even by our own (or possibly the standard's) relaxed
> definition.
> A doctypedecl can only follow an XMLDecl, see
> <http://www.w3.org/TR/2006/REC-xml11-20060816/#sec-prolog-dtd>.

Our version of SQL/XML support references SQL:2003 which references XML
1.0, where omitting the XMLDecl is legal. You can't omit the XMLDecl in
XML 1.1, because you need it to communicate the fact that it's version
1.1.

But note that that is correctly supported:

=# select xmlconcat('<?xml version="1.0"?><foo/>', '<?xml
version="1.0"?><bar/>');
xmlconcat
--------------
<foo/><bar/>

and

=# select xmlconcat('<?xml version="1.1"?><foo/>', '<?xml
version="1.1"?><bar/>');
xmlconcat
-----------------------------------
<?xml version="1.1"?><foo/><bar/>

Peter Eisentraut wrote:
> Our version of SQL/XML support references SQL:2003 which references XML
> 1.0, where omitting the XMLDecl is legal. You can't omit the XMLDecl in
> XML 1.1, because you need it to communicate the fact that it's version
> 1.1.
>
>
>

Hmm. OK. Well here is a patch that tries to fix the xmlconcat error,
anyway. It seems to work, but maybe could stand a little tightening.

cheers

andrew

Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
> Hmm. OK. Well here is a patch that tries to fix the xmlconcat error,
> anyway. It seems to work, but maybe could stand a little tightening.

I liked your previous idea (rethink the whole mess in 9.1) better.

As far as the patch itself is concerned, the complete lack of error
checks seems scary, and I wonder whether the case sensitivity and
lack of whitespace tolerance in the string comparisons is OK.

regards, tom lane

Tom Lane wrote:
> Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
>
>> Hmm. OK. Well here is a patch that tries to fix the xmlconcat error,
>> anyway. It seems to work, but maybe could stand a little tightening.
>>
>
> I liked your previous idea (rethink the whole mess in 9.1) better.
>
> As far as the patch itself is concerned, the complete lack of error
> checks seems scary,

Yes, this wasn't intended as the final patch. If it's not wanted right
now, that's fine too. I just wanted to get it on the record as possibly
something useful when we do come to reconsider the whole mess. Getting
to grips with the libxml2 API is no fun, and it's better not to have to
repeat it if possible ;-)

> and I wonder whether the case sensitivity and
> lack of whitespace tolerance in the string comparisons is OK.
>
>
>

The tokens were chosen with some care to be such that no whitespace
tolerance would be needed (or correct). XML is case sensitive, so that's
not an issue either.

cheers

andrew

Andrew Dunstan <andrew(at)dunslane(dot)net> wrote:

> Hmm. OK. Well here is a patch that tries to fix the xmlconcat error,
> anyway. It seems to work, but maybe could stand a little tightening.

Can we take the patch for 9.0? The bug is registered as an open item:
http://wiki.postgresql.org/wiki/PostgreSQL_9.0_Open_Items

As far as the patch, I found there are still two issues even after it applied:

1. A linebreak is added at the line end DOCTYPE exists.
=# SELECT xmlconcat('foo', xmlparse(DOCUMENT '<!DOCTYPE html><html/>'));
xmlconcat
------------
foo<html/>+

(1 row)

2. DOCUMENT could have <?xml> before DOCTYPE.
=# SELECT xmlconcat('foo', xmlparse(DOCUMENT '<?xml version="1.0"?> <!DOCTYPE html> <html/>'));
xmlconcat
-----------
foo
(1 row)

Regards,
---
Takahiro Itagaki
NTT Open Source Software Center

Takahiro Itagaki <itagaki(dot)takahiro(at)oss(dot)ntt(dot)co(dot)jp> writes:
> Can we take the patch for 9.0? The bug is registered as an open item:
> http://wiki.postgresql.org/wiki/PostgreSQL_9.0_Open_Items

Given that there are still problems with it, applying the patch for 9.0
would mean changing the behavior of xmlconcat in 9.0 and then again in
9.1. I don't think that's a good idea. Better to leave it alone until
we have a full fix.

regards, tom lane

Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> Takahiro Itagaki <itagaki(dot)takahiro(at)oss(dot)ntt(dot)co(dot)jp> writes:
> > Can we take the patch for 9.0? The bug is registered as an open item:
> > http://wiki.postgresql.org/wiki/PostgreSQL_9.0_Open_Items
>
> Given that there are still problems with it, applying the patch for 9.0
> would mean changing the behavior of xmlconcat in 9.0 and then again in
> 9.1. I don't think that's a good idea. Better to leave it alone until
> we have a full fix.

Ok, I added it in ToDo list, and removed it from 9.0 open items.
better handling of PIs and DTDs in xmlconcat()

Regards,
---
Takahiro Itagaki
NTT Open Source Software Center