Thanks for putting up with a new reviewer! --with-libxml is the PostgreSQL configure option to make it use libxml2. >
> The very web page http://xmlsoft.org/index.html says "The XML C parser > and toolkit of Gnome: libxml" and is all about libxml2. > > So I think I was unsure what convention to follow, and threw up my hands > and went with libxml. I could just as easily throw them up and go with > libxml2. Which do you think would be better? I think leaving it as libxml makes sense with all that. Good point that --with-libxml is used to build so I think staying with that works and is consistent. I agree that having this point included does clarify the how and why of the limitations of this implementation. I also over-parenthesize so I'm used to looking for that in my own writing. The full sentences were the ones that seemed excessive to me, I think the others are ok and I won't nit-pick either way on those (unless you want me to!). If you agree, I should go through and fix my nodesets to be node-sets. Yes, I like node-sets better, especially knowing it conforms to the spec's language. Thanks, *Ryan Lambert* On Tue, Mar 26, 2019 at 11:05 PM Chapman Flack <c...@anastigmatix.net> wrote: > On 03/26/19 21:39, Ryan Lambert wrote: > > 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 > > Thanks for the review! > > > 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 > > That's a good catch. I'm not actually sure whether there is any "libxml" > library that isn't libxml2. Maybe there was once and nobody admits to > hanging out with it. Most Google hits on "libxml" seem to be modules > that have libxml in their names and libxml2 as their actual dependency. > > Perl XML:LibXML: "This module is an interface to libxml2" > Haskell libxml: "Binding to libxml2" > libxml-ruby: "The Libxml-Ruby project provides Ruby language bindings > for the GNOME Libxml2 ..." > > --with-libxml is the PostgreSQL configure option to make it use libxml2. > > The very web page http://xmlsoft.org/index.html says "The XML C parser > and toolkit of Gnome: libxml" and is all about libxml2. > > So I think I was unsure what convention to follow, and threw up my hands > and went with libxml. I could just as easily throw them up and go with > libxml2. Which do you think would be better? > > On 03/26/19 23:52, Tom Lane wrote: > > Do we need to mention that at all? If you're not building from source, > > it doesn't seem very interesting ... but maybe I'm missing some reason > > why end users would care. > > The three places I've mentioned it were the ones where I thought users > might care: > > - why are we stuck at XPath 1.0? It's what we get from the library we use. > > - in what order do we get things out from a (hmm) node-set? Per XPath 1.0, > it's indeterminate (it's a set!), unlike XPath 2.0/XQuery where there's > a sequence type and you have order control. Observable behavior from > libxml2 (and you could certainly want to know this) is you get things > out > in document order, whether that's what you wanted or not, even though > this is undocumented, and even counter-documented[1], libxml2 behavior. > So it's an example of something you would fundamentally like to know, > where the only available answer depends precariously on the library > we happen to be using. > > - which limits in our implementation are inherent to the library, and > which are just current limits in our embedding of it? (Maybe this is > right at the border of what a user would care to know, but I know it's > a question that crosses my mind when I bonk into a limit I wasn't > expecting.) > > > There are a few places where the parenthesis around a block of text > > seem unnecessary. > > )blush( that's a long-standing wart in my writing ... seems I often think > in parentheses, then look and say "those aren't needed" and take them out, > only sometimes I don't. > > I skimmed just now and found a few instances of parenthesized whole > sentence: the one you quoted, and some (if argument is null, the result > is null), and (No rows will be produced if ....). Shall I deparenthesize > them all? Did you have other instances in mind? > > > It seems you are standardizing from "node set" to "nodeset", is that > > the preferred nomenclature or preference? > > Another good catch. I remember consciously making a last pass to get them > all consistent, and I wanted them consistent with the spec, and I see now > I messed up. > > XPath 1.0 [2] has zero instances of "nodeset", two of "node set" and about > six dozen of "node-set". The only appearances of "node set" without the > hyphen are in a heading and its ToC entry. The stuff under that heading > consistently uses node-set. It seems that's the XPath 1.0 term for sure. > > When I made my consistency pass, I must have been looking too recently > in libxml2 C source, rather than the spec. > > On 03/26/19 23:52, Tom Lane wrote: > > That seemed a bit jargon-y to me too. If that's standard terminology > > in the XML world, maybe it's fine; but I'd have stuck with "node set". > > It really was my intention (though I flubbed it) to use XPath 1.0's term > for XPath 1.0's concept; in my doc philosophy, that gives readers > the most breadcrumbs to follow for the rest of the details if they want > them. "Node set" might be some sort of squishy expository concept I'm > using, but node-set is a thing, in a spec. > > If you agree, I should go through and fix my nodesets to be node-sets. > > I do think the terminology matters here, especially because of the > differences between what you can do with a node-set (XPath 1.0 thing) > and with a sequence (XPath 2.0+,XQuery,SQL/XML thing). > > Let me know what you'd like best on these points and I'll revise the patch. > > Regards, > -Chap > > > [1] http://xmlsoft.org/html/libxml-xpath.html#xmlNodeSet : "array of nodes > in no particular order" > > [2] https://www.w3.org/TR/1999/REC-xpath-19991116/ >
