Horton

Re: Fix XML handling with DOCTYPE

Ryan Lambert <ryan@rustprooflabs.com>

View thread View on postgr.es
From: Ryan Lambert <ryan@rustprooflabs.com>
To: pgsql-hackers@lists.postgresql.org
Cc: Pavel Stehule <pavel.stehule@gmail.com>, Chapman Flack <chap@anastigmatix.net>
Date: 2019-03-27T01:39:37Z
Lists: pgsql-hackers 
The following review has been posted through the commitfest application:
make installcheck-world:  tested, passed
Implements feature:       tested, passed
Spec compliant:           not tested
Documentation:            tested, passed

Overall I think this patch [1] improves the docs and help explain edge case functionality that many of us, myself included, don't fully understand.  I can't verify technical accuracy for many of the details (nuances between XPath 1.0, et. al), but overall my experience with the XML functionality lines up with what has been documented here.  Adding the clear declaration of "XPath 1.0" instead of the generic "XPath" helps make it clear of the functional differences and helps frame the differences for new users.

I have two recommendations for features.sgml.  You state: 

>  relies on the libxml library

Should this be clarified as the libxml2 library?  That's what I installed to build postgres from source (Ubuntu 16/18).  If it is the libxml library and the "2" is irrelevant, or it it works with either, it might be nice to have a clarifying note to indicate that.

There are a few places where the parenthesis around a block of text seem unnecessary.  I don't think parens serve a purpose when a full sentence is contained within.

> (The libxml library does seem to always return nodesets to PostgreSQL with their members in the same relative order they had in the input document; it does not commit to this behavior, and an XPath 1.0 expression cannot control it.)


It seems you are standardizing from "node set" to "nodeset", is that the preferred nomenclature or preference?

Hopefully this helps!  Thanks,

Ryan Lambert

[1] https://www.postgresql.org/message-id/attachment/100016/xml-functions-type-docfix-4.patch

The new status of this patch is: Waiting on Author

Commits

  1. Improve documentation about our XML functionality

  2. Improve documentation about our XML functionality.

  3. Doc: clarify that REASSIGN OWNED doesn't handle default privileges.

  4. Suppress Append and MergeAppend plan nodes that have a single child.

  5. Accept XML documents when xmloption = content, as required by SQL:2006+.

  6. Ensure xmloption = content while restoring pg_dump output.