Thread: The two "XML Fixes" patches still in need of review

Alvaro has committed two of the patches in this CF entry[1], but the
remaining two have yet to attract review.

This message contains only those two, just as before[2] except rebased
over Alvaro's commits of the others.

<confession>
 There are two new changes. The <ulink>s added in the -docfix patch now
 have link text, as allowed according to b060e6c, and the -content patch
 now updates the definition of the 'content' type in datatypes.sgml,
 which I had overlooked before.
</confession>

xml-functions-type-docfix-3.patch adjusts the documentation of the XML type
and related functions to present some behavior and limitations more clearly.

xml-content-2006-2.patch changes the behavior of xmlparse and the
text-to-xml cast to allow any XML 'document' (including one with a DTD)
to be parsed as 'content', where the former behavior was to fail in that
case. This is the same as changing the definition of XML 'content' from
that of SQL:2003 to that of SQL:2006 and later. The later definition is
preferable, because it eliminates a case that can fail in, e.g., pg_restore
(which problem has been reported in the field).

The patches apply in that order (because the -docfix one adds language
describing the current 'content' behavior, then the -content one changes
the behavior, and the language to match it).

Regards,
-Chap

[1] https://commitfest.postgresql.org/22/1872/
[2]
https://www.postgresql.org/message-id/3e8eab9e-7289-6c23-5e2c-153cccea2257@anastigmatix.net

Chapman Flack <chap@anastigmatix.net> writes:
> Alvaro has committed two of the patches in this CF entry[1], but the
> remaining two have yet to attract review.
> This message contains only those two, just as before[2] except rebased
> over Alvaro's commits of the others.

Just to update this thread --- per the other thread at
https://postgr.es/m/CAN-V+g-6JqUQEQZ55Q3toXEN6d5Ez5uvzL4VR+8KtvJKj31taw@mail.gmail.com
I've now pushed a somewhat-adjusted version of the XML-content fix
patch.  The documentation patch needs some small rebasing to apply
after that one instead of before it.

I'm not going to touch the documentation patch myself; I don't know
enough about XML to review it competently.  I do have a small suggestion
though, which is that the large "Limits and Compatibility" section
you added doesn't really seem to me to belong where you put it.
Perhaps it'd make sense under the XML section in datatype.sgml,
but I think I might lean to making it a new section in Appendix D
(SQL Conformance).

            regards, tom lane

On 03/23/19 17:05, Tom Lane wrote:

> Just to update this thread --- per the other thread at
> https://postgr.es/m/CAN-V+g-6JqUQEQZ55Q3toXEN6d5Ez5uvzL4VR+8KtvJKj31taw@mail.gmail.com
> I've now pushed a somewhat-adjusted version of the XML-content fix
> patch.  The documentation patch needs some small rebasing to apply
> after that one instead of before it.

Will do.

> Perhaps it'd make sense under the XML section in datatype.sgml,
> but I think I might lean to making it a new section in Appendix D
> (SQL Conformance).

Sounds like the option (4) I proposed back in [1]. I suppose it won't
be much trouble to move.

> I'm not going to touch the documentation patch myself; I don't know
> enough about XML to review it competently.

I would be happy even to see it reviewed for style, and if anybody's
willing to look at the technical content even if not feeling ideally
prepared, I'd be happy to respond or give references for any points
in question.

It does fix a number of actual bugs in the existing documentation, so
while it would be ideal to apply a higher standard of technical review
to this patch, it'd be less ideal to have existing buggy doc lingering on
because nobody feels up to reviewing at that ideal higher standard.

Regards,
-Chap

[1] https://www.postgresql.org/message-id/5C298BCB.4060704%40anastigmatix.net

On 03/23/19 18:20, Chapman Flack wrote:
>> Perhaps it'd make sense under the XML section in datatype.sgml,
>> but I think I might lean to making it a new section in Appendix D
>> (SQL Conformance).
> 
> Sounds like the option (4) I proposed back in [1]. I suppose it won't
> be much trouble to move.

The current structure of that appendix is: a few introductory paragraphs
(not wrapped in any <sect... ...>), followed by two <sect1>s that are
both autogenerated (the supported and unsupported features tables).

The <sect1>s become independent pages, in the HTML version.

- Move the XML limits/conformance section to a new <sect1> there?

- Before the autogenerated tables, or after them?

The un-<sect>ed intro paragraphs are preceded (in HTML) by a table
of contents, so the new section should show up there.

- Also add a mention (say, in the next-to-last intro paragraph) that
  there are notes on the SQL/XML conformance that have their own
  section (and link to it)? Or just let the generated ToC link be enough?


Regards,
-Chap

On 03/23/19 18:20, Chapman Flack wrote:
> On 03/23/19 17:05, Tom Lane wrote:
>> I've now pushed a somewhat-adjusted version of the XML-content fix
>> patch.  The documentation patch needs some small rebasing to apply
>> after that one instead of before it.
> 
> Will do.
> 
>> Perhaps it'd make sense under the XML section in datatype.sgml,
>> but I think I might lean to making it a new section in Appendix D
>> (SQL Conformance).
> 
> Sounds like the option (4) I proposed back in [1]. I suppose it won't
> be much trouble to move.

PFA xml-functions-type-docfix-4.patch rebased, and with the limits/
compatibility section moved to Appendix D.

Regards,
-Chap