Thread: xpath processing brain dead
Andrew Gierth was just pointing out to me how badly broken our XPath processing is. For fear of passing an ill formed fragment of xml to the processor, we strip the xml declaration if any and surround what's left with '<x>" and '</x>' and prepend '/x' to the supposed xpath. This is just horrible. It will break for every xpath expression that doesn't begin with a '/' and probably for many that do. This whole thing is a mess, and I suspect the only fix for now is to undo all the mangling of both the xml and the xpath expression. If the programmer passes an ill formed piece of xml to the processor that is their lookout, but I think we should ensure that we give back correct results on well formed input. The only good piece of news is that the xpath procedures in contrib/xml2 don't apparently suffer these faults. cheers andrew
Andrew Dunstan <andrew@dunslane.net> writes:
> For fear of passing an ill formed fragment of xml to the processor, we
> strip the xml declaration if any and surround what's left with '<x>" and
> '</x>' and prepend '/x' to the supposed xpath. This is just horrible.
I seem to recall having complained about that at the time, but I didn't
(and don't) know enough about xpath to do any better.
> This whole thing is a mess, and I suspect the only fix for now is to
> undo all the mangling of both the xml and the xpath expression.
I don't think we should change the behavior if it's just to arrive at
another less-than-desirable behavior. Whacking semantics around afresh
with each release does not endear us to users. If we know how to fix it
right, great; but if we can't then we should keep compatibility with 8.3
until we can.
regards, tom lane
Tom Lane wrote: > Andrew Dunstan <andrew@dunslane.net> writes: > >> For fear of passing an ill formed fragment of xml to the processor, we >> strip the xml declaration if any and surround what's left with '<x>" and >> '</x>' and prepend '/x' to the supposed xpath. This is just horrible. >> > > I seem to recall having complained about that at the time, but I didn't > (and don't) know enough about xpath to do any better. > Well, a few of us do. I guess I took my eye off the ball a bit back when we were putting this into 8.3. > >> This whole thing is a mess, and I suspect the only fix for now is to >> undo all the mangling of both the xml and the xpath expression. >> > > I don't think we should change the behavior if it's just to arrive at > another less-than-desirable behavior. Whacking semantics around afresh > with each release does not endear us to users. If we know how to fix it > right, great; but if we can't then we should keep compatibility with 8.3 > until we can. > > > Honestly, this is a bug, pure and simple. There really can't be an argument about that. For the stable branch, we could make the following changes that should result in a Pareto improvement (nothing gets worse while some things get better): * only do the xml transformation if the xml is known not to be be well formed * if we need to mangle the xpath expressiondue to doing the xml transformation, then unless the xpath expression begins with a '/', prepend it with'/x//'. (two slashes corresponds to the descendent axis in xpath - in effect it stands for any number of descendentelements). But that's just a holding operation. For 8.4 we should stop this nonsense and simply say that it is up to the programmer to ensure that the xml passed to the processor is well formed. The thing that is so very bad about this is that if the programmer *has* made sure that the inputs are correct, s/he can still end up with broken results. If we're going to try to fix bad inputs, we must make damn sure that we don't break on correct inputs as a result. However, I can't off hand think of a lightning way to fix bad inputs that doesn't carry some danger to good inputs. Until someone comes up with something tolerably bulletproof, I suggest that we simply say that it is the programmer's responsibility. cheers andrew
Andrew Dunstan wrote: > For fear of passing an ill formed fragment of xml to the processor, we > strip the xml declaration if any and surround what's left with '<x>" and > '</x>' and prepend '/x' to the supposed xpath. This is just horrible. It > will break for every xpath expression that doesn't begin with a '/' and > probably for many that do. > > This whole thing is a mess, and I suspect the only fix for now is to > undo all the mangling of both the xml and the xpath expression. If the > programmer passes an ill formed piece of xml to the processor that is > their lookout, but I think we should ensure that we give back correct > results on well formed input. It's not about ill-formed pieces, it is about (well-formed) content fragments that are not full documents (exactly one root element). libxml2 doesn't support xpath on content fragments. The tradeoff here is either supporting no xpath at all on content fragments or supporting some xpath on both kinds of xml data. Whoever made the initial implementation of this (Nikolay, Cc) decided for the latter, but I can't say I'm happy with it either. I'd be OK with changing it, but I haven't had time to get to it, unfortunately. See also <http://wiki.postgresql.org/wiki/XPath_Todo#XPath>.
Peter Eisentraut wrote: > Andrew Dunstan wrote: >> For fear of passing an ill formed fragment of xml to the processor, >> we strip the xml declaration if any and surround what's left with >> '<x>" and '</x>' and prepend '/x' to the supposed xpath. This is just >> horrible. It will break for every xpath expression that doesn't begin >> with a '/' and probably for many that do. >> >> This whole thing is a mess, and I suspect the only fix for now is to >> undo all the mangling of both the xml and the xpath expression. If >> the programmer passes an ill formed piece of xml to the processor >> that is their lookout, but I think we should ensure that we give back >> correct results on well formed input. > > It's not about ill-formed pieces, it is about (well-formed) content > fragments that are not full documents (exactly one root element). > libxml2 doesn't support xpath on content fragments. > > The tradeoff here is either supporting no xpath at all on content > fragments or supporting some xpath on both kinds of xml data. Whoever > made the initial implementation of this (Nikolay, Cc) decided for the > latter, but I can't say I'm happy with it either. I'd be OK with > changing it, but I haven't had time to get to it, unfortunately. > > See also <http://wiki.postgresql.org/wiki/XPath_Todo#XPath>. I don't think it is our responsibility to remedy the deficiencies of libxml2, especially if it involves breaking the processing of valid xpath expressions on well formed XML. If someone can come up with a correct scheme for handling fragments, I'm all ears, but otherwise I think a) we should rip this out of 8.4 and b) we should try to make 8.3 slightly less broken at least, along the lines of my earlier suggestion. cheers andrew
Andrew Dunstan <andrew@dunslane.net> writes:
> I don't think it is our responsibility to remedy the deficiencies of
> libxml2, especially if it involves breaking the processing of valid
> xpath expressions on well formed XML.
> If someone can come up with a correct scheme for handling fragments, I'm
> all ears, but otherwise I think a) we should rip this out of 8.4 and b)
> we should try to make 8.3 slightly less broken at least, along the lines
> of my earlier suggestion.
I'm okay with removing this for 8.4, but I think it's too late to
change the behavior of 8.3.
regards, tom lane
Tom Lane wrote: > Andrew Dunstan <andrew@dunslane.net> writes: > >> I don't think it is our responsibility to remedy the deficiencies of >> libxml2, especially if it involves breaking the processing of valid >> xpath expressions on well formed XML. >> > > >> If someone can come up with a correct scheme for handling fragments, I'm >> all ears, but otherwise I think a) we should rip this out of 8.4 and b) >> we should try to make 8.3 slightly less broken at least, along the lines >> of my earlier suggestion. >> > > I'm okay with removing this for 8.4, but I think it's too late to > change the behavior of 8.3. > > > It's never too late to fix a bug. The current behaviour is just plain nonsense ... if your xpath expression is 'foo/bar' it ends up searching for '/xfoo/bar' which can't possibly be right. Surely we can at least fix that. cheers andrew
Andrew Dunstan <andrew@dunslane.net> writes:
> Tom Lane wrote:
>> I'm okay with removing this for 8.4, but I think it's too late to
>> change the behavior of 8.3.
> It's never too late to fix a bug.
When the proposed fix involves breaking cases that used to behave
usefully, you need a much stronger argument than that.
regards, tom lane
Tom Lane wrote: > Andrew Dunstan <andrew@dunslane.net> writes: > >> Tom Lane wrote: >> >>> I'm okay with removing this for 8.4, but I think it's too late to >>> change the behavior of 8.3. >>> > > >> It's never too late to fix a bug. >> > > When the proposed fix involves breaking cases that used to behave > usefully, you need a much stronger argument than that. > What I have proposed for 8.3 should not break a single case that currently behaves usefully. If anyone has a counter-example please show it. What I have proposed for 8.4 possibly would break current "useful" behaviour (FSVO "useful"), but should be done anyway on correctness grounds. cheers andrew
> What I have proposed for 8.3 should not break a single case that currently > behaves usefully. If anyone has a counter-example please show it. > > What I have proposed for 8.4 possibly would break current "useful" behaviour > (FSVO "useful"), but should be done anyway on correctness grounds. I dunno, aren't XML document fragments sort of a pretty common case? ...Robert
Robert Haas wrote: >> What I have proposed for 8.3 should not break a single case that currently >> behaves usefully. If anyone has a counter-example please show it. >> >> What I have proposed for 8.4 possibly would break current "useful" behaviour >> (FSVO "useful"), but should be done anyway on correctness grounds. >> > > I dunno, aren't XML document fragments sort of a pretty common case? > > > They are. So what? How does that contradict either of the statements made above? Programmers using libxml2 are used to handling this problem. Why must postgres alone use a totally brain dead and utterly incorrect wrapping to solve a problem that everyone else leaves up to the programmer to handle? People in this thread are not concentrating on the fact that what we do now can break correct input. That makes it an unquestionable bug, IMNSHO. There seems to be agreement about what to do for 8.4, so we seem to be arguing about what to do for 8.3. Are you *really* arguing that prepending the xpath expression with '/x' in all cases should be allowed to continue? If you are I can only assume your use of xml/xpath is probably fairly light. cheers andrew
On Thu, Feb 26, 2009 at 3:18 PM, Andrew Dunstan <andrew@dunslane.net> wrote: >> I dunno, aren't XML document fragments sort of a pretty common case? > > They are. So what? How does that contradict either of the statements made > above? > > Programmers using libxml2 are used to handling this problem. Why must > postgres alone use a totally brain dead and utterly incorrect wrapping to > solve a problem that everyone else leaves up to the programmer to handle? I can't think of any reason, especially not when you put it that way. :-) > People in this thread are not concentrating on the fact that what we do now > can break correct input. That makes it an unquestionable bug, IMNSHO. There > seems to be agreement about what to do for 8.4, so we seem to be arguing > about what to do for 8.3. Are you *really* arguing that prepending the xpath > expression with '/x' in all cases should be allowed to continue? If you are > I can only assume your use of xml/xpath is probably fairly light. Mine is very light indeed. But the change you're proposing seems like it could CONCEIVABLY break a working application that counts on PostgreSQL's particular flavor of misbehavior, and therefore it seems questionable to put that into a stable branch. The fact that the application perhaps should not have been written that way to begin with is neither here nor there. We don't want people to be afraid of upgrading to the latest point release when we fix, say, a backend crash or a data corruption bug. ...Robert
On Thu, 2009-02-26 at 15:37 -0500, Robert Haas wrote: > Mine is very light indeed. But the change you're proposing seems like > it could CONCEIVABLY break a working application that counts on > PostgreSQL's particular flavor of misbehavior, That is what release notes are for. Joshua D. Drake -- PostgreSQL - XMPP: jdrake@jabber.postgresql.org Consulting, Development, Support, Training 503-667-4564 - http://www.commandprompt.com/ The PostgreSQL Company, serving since 1997
Robert Haas wrote: > But the change you're proposing seems like > it could CONCEIVABLY break a working application that counts on > PostgreSQL's particular flavor of misbehavior, and therefore it seems > questionable to put that into a stable branch. The fact that the > application perhaps should not have been written that way to begin > with is neither here nor there. We don't want people to be afraid of > upgrading to the latest point release when we fix, say, a backend > crash or a data corruption bug. > > > Let me reiterate the changes that I propose, and again challenge you to provide a working counter-example if you believe it will break anything not currently broken, even cases involving fragments. First, I propose that we abandon this mangling, if, and only if, the xml is in fact a well formed XML document. Since the whole point of the mangling is to handle situations where the XML is not a well formed document, that seems fairly straight-forward. If this change were to upset any user, it must be because they are relying on undisputably incorrect results. Second, I propose that, in the remaining cases, where we do mangle the XML, if the xpath expression does not begin with a '/', instead of prepending it with '/x/, which can not possibly be correct under any circumstance, we prepend it with '/x//' which has some possibility of giving correct results. IOW, these proposals will expand the number of correct results from the code, without contributing any new incorrect cases. These changes are *very* conservative, as is usual when we fix things on stable branches. cheers andrew
I wrote: > > Second, I propose that, in the remaining cases, where we do mangle the > XML, if the xpath expression does not begin with a '/', instead of > prepending it with '/x/, which can not possibly be correct under any > circumstance, we prepend it with '/x//' which has some possibility of > giving correct results. > ^^^ '/x' cheers andrew
Andrew Dunstan <andrew@dunslane.net> writes:
> First, I propose that we abandon this mangling, if, and only if, the xml
> is in fact a well formed XML document. Since the whole point of the
> mangling is to handle situations where the XML is not a well formed
> document, that seems fairly straight-forward. If this change were to
> upset any user, it must be because they are relying on undisputably
> incorrect results.
> Second, I propose that, in the remaining cases, where we do mangle the
> XML, if the xpath expression does not begin with a '/', instead of
> prepending it with '/x/, which can not possibly be correct under any
> circumstance, we prepend it with '/x//' which has some possibility of
> giving correct results.
Hmm, does this proposal require adding a test of well-formed-ness to
a code path that doesn't currently have one? If so, is that likely
to contribute any noticeable slowdown?
I can't offhand see an objection to this other than possible performance
impact.
regards, tom lane
On Thu, Feb 26, 2009 at 3:54 PM, Andrew Dunstan <andrew@dunslane.net> wrote: > Robert Haas wrote: >> But the change you're proposing seems like >> it could CONCEIVABLY break a working application that counts on >> PostgreSQL's particular flavor of misbehavior, and therefore it seems >> questionable to put that into a stable branch. The fact that the >> application perhaps should not have been written that way to begin >> with is neither here nor there. We don't want people to be afraid of >> upgrading to the latest point release when we fix, say, a backend >> crash or a data corruption bug. > > Let me reiterate the changes that I propose, and again challenge you to > provide a working counter-example if you believe it will break anything not > currently broken, even cases involving fragments. > > First, I propose that we abandon this mangling, if, and only if, the xml is > in fact a well formed XML document. Since the whole point of the mangling is > to handle situations where the XML is not a well formed document, that seems > fairly straight-forward. If this change were to upset any user, it must be > because they are relying on undisputably incorrect results. > > Second, I propose that, in the remaining cases, where we do mangle the XML, > if the xpath expression does not begin with a '/', instead of prepending it > with '/x/, which can not possibly be correct under any circumstance, we > prepend it with '/x//' which has some possibility of giving correct results. > > IOW, these proposals will expand the number of correct results from the > code, without contributing any new incorrect cases. These changes are *very* > conservative, as is usual when we fix things on stable branches. > > cheers > > andrew You are right. Nuff said, ...Robert
On Feb 26, 2009, at 9:37 AM, Peter Eisentraut wrote: > It's not about ill-formed pieces, it is about (well-formed) content > fragments that are not full documents (exactly one root element). > libxml2 doesn't support xpath on content fragments. exslt:node-set() to the rescue? Or is that/equivalent functionality not easily accessed at the C-level with libxml2? http://www.exslt.org/exsl/functions/node-set/index.html
James Pye wrote: > On Feb 26, 2009, at 9:37 AM, Peter Eisentraut wrote: >> It's not about ill-formed pieces, it is about (well-formed) content >> fragments that are not full documents (exactly one root element). >> libxml2 doesn't support xpath on content fragments. > > exslt:node-set() to the rescue? Or is that/equivalent functionality > not easily accessed at the C-level with libxml2? > > http://www.exslt.org/exsl/functions/node-set/index.html A node-set isn't a document. In any case, this functionality doesn't appear to be in libxml2, it's in libxslt according to the reference you provided. cheers andrew
On Feb 26, 2009, at 5:41 PM, Andrew Dunstan wrote: >> http://www.exslt.org/exsl/functions/node-set/index.html > > A node-set isn't a document. yes.. :) I guess what I'm saying is that if: tinman=# SELECT XML'<foo/><bar/>'; xml -------------- <foo/><bar/> (1 row) is considered to be valid per *SQL-XML*, then it should probably be treated as a node-set in the context of xpath, not mangled with <x>...</x>.. Certainly, I would expect an implicit "node-set() call" long before wrapping the fragment in <x>...</x> and prefixing my xpath query. However, I find the validity of the above, XML'<foo/><bar/>', a bit disturbing to begin with. :P > In any case, this functionality doesn't appear to be in libxml2, > it's in libxslt according to the reference you provided. I think that's *just* referencing the list of xslt implementations that the extension function is known to be available in.. I doubt that means to imply that the function or equivalent functionality is not available in libxml2 itself. I'd wager that equivalent functionality could be implemented if it weren't directly/already available.. =)
James Pye wrote: > On Feb 26, 2009, at 5:41 PM, Andrew Dunstan wrote: >>> http://www.exslt.org/exsl/functions/node-set/index.html >> >> A node-set isn't a document. > > yes.. :) > > I guess what I'm saying is that if: > > tinman=# SELECT XML'<foo/><bar/>'; > xml > -------------- > <foo/><bar/> > (1 row) > > is considered to be valid per *SQL-XML*, then it should probably be > treated as a node-set in the context of xpath, not mangled with > <x>...</x>.. > > Certainly, I would expect an implicit "node-set() call" long before > wrapping the fragment in <x>...</x> and prefixing my xpath query. > > However, I find the validity of the above, XML'<foo/><bar/>', a bit > disturbing to begin with. :P > >> In any case, this functionality doesn't appear to be in libxml2, >> it's in libxslt according to the reference you provided. > > I think that's *just* referencing the list of xslt implementations > that the extension function is known to be available in.. I doubt that > means to imply that the function or equivalent functionality is not > available in libxml2 itself. I'd wager that equivalent functionality > could be implemented if it weren't directly/already available.. =) > James, can you point me at any call in libxml2 which will evaluate an xpath expression in the context of a nodeset instead of a document? Quite apart from anything else, xpath requires there to be a (single) context node (see http://www.w3.org/TR/xpath20/#context ). For a doc, we set that node to the document node, but what would it be for a node-set or a fragment? If we can't get over that hurdle we're screwed in pursuing your line of thought. cheers andrew
Tom Lane wrote: > Hmm, does this proposal require adding a test of well-formed-ness to > a code path that doesn't currently have one? If so, is that likely > to contribute any noticeable slowdown? > > I can't offhand see an objection to this other than possible performance > impact. > > > Yeah, testing the well-formedness might cost a bit. We could short-circuit the test by applying some comparatively fast heuristic tests. Or we could decide that we'll just fix the xpath prefix part for 8.3 and keep the wrapping. I don't want to spend a huge effort on fixing something I regard as fundamentally broken. I'll do some tests to see what the cost of extra xml parsing might be. cheers andrew We
On Thu, 2009-02-26 at 13:51 -0500, Robert Haas wrote: > > What I have proposed for 8.3 should not break a single case that currently > > behaves usefully. If anyone has a counter-example please show it. > > > > What I have proposed for 8.4 possibly would break current "useful" behaviour > > (FSVO "useful"), but should be done anyway on correctness grounds. > > I dunno, aren't XML document fragments sort of a pretty common case? I'd rather argue that xml datatype should not even accept anything but complete xml documents. Same as int field does not accept int[]. Or maybe we rather need separate xmldocument and xmlforest/xmlfragments types in next releases and leave the "base" xml as it is but deprecated due to inability to fix it without breaking backwards compatibility. -- Hannu Krosing http://www.2ndQuadrant.com PostgreSQL Scalability and Availability Services, Consulting and Training
Hannu Krosing wrote: > On Thu, 2009-02-26 at 13:51 -0500, Robert Haas wrote: > >>> What I have proposed for 8.3 should not break a single case that currently >>> behaves usefully. If anyone has a counter-example please show it. >>> >>> What I have proposed for 8.4 possibly would break current "useful" behaviour >>> (FSVO "useful"), but should be done anyway on correctness grounds. >>> >> I dunno, aren't XML document fragments sort of a pretty common case? >> > > I'd rather argue that xml datatype should not even accept anything but > complete xml documents. Same as int field does not accept int[]. > > Or maybe we rather need separate xmldocument and xmlforest/xmlfragments > types in next releases and leave the "base" xml as it is but deprecated > due to inability to fix it without breaking backwards compatibility. > > Some of the functions, including some specified in the standard, produce fragments. That's why we have the 'IS DOCUMENT' test. You can also force validation as a document by saying SET XML OPTION DOCUMENT; cheers andrew
Andrew Dunstan wrote:
>
>
> Tom Lane wrote:
>> Hmm, does this proposal require adding a test of well-formed-ness to
>> a code path that doesn't currently have one? If so, is that likely
>> to contribute any noticeable slowdown?
>>
>> I can't offhand see an objection to this other than possible performance
>> impact.
>>
>>
>>
>
> Yeah, testing the well-formedness might cost a bit. We could
> short-circuit the test by applying some comparatively fast heuristic
> tests.
>
> Or we could decide that we'll just fix the xpath prefix part for 8.3
> and keep the wrapping. I don't want to spend a huge effort on fixing
> something I regard as fundamentally broken.
>
> I'll do some tests to see what the cost of extra xml parsing might be.
The extra cost appears to be fairly negligible.
regression=# create table xpathtest3 as select xmlconcat(xmlelement(name
unique1, unique1), '\n\t',xmlelement(name unique2, unique2),
'\n\t',xmlelement(name two, two), '\n\t',xmlelement(name four,
four),'\n\t',xmlelement(name ten,ten),'\n\t',xmlelement(name
twenty,twenty),'\n\t',xmlelement(name
hundred,hundred),'\n\t',xmlelement(name
thousand,thousand),'\n\t',xmlelement(name
twothusand,twothousand),'\n\t',xmlelement(name
fivethous,fivethous),'\n\t',xmlelement(name
tenthous,tenthous),'\n\t',xmlelement(name
odd,odd),'\n\t',xmlelement(name even,even),'\n\t',xmlelement(name
stringu1,stringu1),'\n\t',xmlelement(name
stringu2,stringu2),'\n\t',xmlelement(name string4,string4),'\n') from tenk1;
regression=# select count(*) from (select
xpath('//two[text()="0"]/text()',xmlconcat) as elems from xpathtest3,
generate_series(1,10) ) x ;
count
--------
100000
(1 row)
Time: 27.722 ms
Proposed patch for 8.3 attached. (Note: it only reparses in the
non-document case)
cheers
andrew
Index: src/backend/utils/adt/xml.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/utils/adt/xml.c,v
retrieving revision 1.68.2.6
diff -c -r1.68.2.6 xml.c
*** src/backend/utils/adt/xml.c 10 Nov 2008 18:02:27 -0000 1.68.2.6
--- src/backend/utils/adt/xml.c 27 Feb 2009 20:59:28 -0000
***************
*** 3320,3360 ****
xml_init();
! /*
! * To handle both documents and fragments, regardless of the fact whether
! * the XML datum has a single root (XML well-formedness), we wrap the XML
! * datum in a dummy element (<x>...</x>) and extend the XPath expression
! * accordingly. To do it, throw away the XML prolog, if any.
! */
! if (len >= 5 &&
! xmlStrncmp((xmlChar *) datastr, (xmlChar *) "<?xml", 5) == 0)
! {
! i = 5;
! while (i < len &&
! !(datastr[i - 1] == '?' && datastr[i] == '>'))
! i++;
!
! if (i == len)
! xml_ereport(ERROR, ERRCODE_INTERNAL_ERROR,
! "could not parse XML data");
! ++i;
! datastr += i;
! len -= i;
! }
!
! string = (xmlChar *) palloc((len + 8) * sizeof(xmlChar));
! memcpy(string, "<x>", 3);
! memcpy(string + 3, datastr, len);
! memcpy(string + 3 + len, "</x>", 5);
! len += 7;
!
! xpath_expr = (xmlChar *) palloc((xpath_len + 3) * sizeof(xmlChar));
! memcpy(xpath_expr, "/x", 2);
! memcpy(xpath_expr + 2, VARDATA(xpath_expr_text), xpath_len);
! xpath_expr[xpath_len + 2] = '\0';
! xpath_len += 2;
xmlInitParser();
--- 3320,3332 ----
xml_init();
! string = (xmlChar *) palloc((len + 8) * sizeof(xmlChar));
! xpath_expr = (xmlChar *) palloc((xpath_len + 5) * sizeof(xmlChar));
! memcpy (string, datastr, len);
! string[len] = '\0';
!
xmlInitParser();
***************
*** 3367,3375 ****
xml_ereport(ERROR, ERRCODE_OUT_OF_MEMORY,
"could not allocate parser context");
doc = xmlCtxtReadMemory(ctxt, (char *) string, len, NULL, NULL, 0);
! if (doc == NULL)
! xml_ereport(ERROR, ERRCODE_INVALID_XML_DOCUMENT,
! "could not parse XML data");
xpathctx = xmlXPathNewContext(doc);
if (xpathctx == NULL)
xml_ereport(ERROR, ERRCODE_OUT_OF_MEMORY,
--- 3339,3410 ----
xml_ereport(ERROR, ERRCODE_OUT_OF_MEMORY,
"could not allocate parser context");
doc = xmlCtxtReadMemory(ctxt, (char *) string, len, NULL, NULL, 0);
!
! if (doc == NULL || xmlDocGetRootElement(doc) == NULL)
! {
!
! /*
! * In case we have a fragment rather than a well-formed XML document,
! * which has a single root (XML well-formedness), we try again after
! * transforming the xml by stripping away the XML prolog, if any, and
! * wrapping the remainder in a dummy element (<x>...</x>),
! * and later extending the XPath expression accordingly.
! */
! if (len >= 5 &&
! xmlStrncmp((xmlChar *) datastr, (xmlChar *) "<?xml", 5) == 0)
! {
! i = 5;
! while (i < len &&
! !(datastr[i - 1] == '?' && datastr[i] == '>'))
! i++;
!
! if (i == len)
! xml_ereport(ERROR, ERRCODE_INTERNAL_ERROR,
! "could not parse XML data");
!
! ++i;
!
! datastr += i;
! len -= i;
! }
!
! memcpy(string, "<x>", 3);
! memcpy(string + 3, datastr, len);
! memcpy(string + 3 + len, "</x>", 5);
! len += 7;
!
! doc = xmlCtxtReadMemory(ctxt, (char *) string, len, NULL, NULL, 0);
!
! if (doc == NULL)
! xml_ereport(ERROR, ERRCODE_INVALID_XML_DOCUMENT,
! "could not parse XML data");
!
! if (*VARDATA(xpath_expr_text) == '/')
! {
! memcpy(xpath_expr, "/x", 2);
! memcpy(xpath_expr + 2, VARDATA(xpath_expr_text), xpath_len);
! xpath_expr[xpath_len + 2] = '\0';
! xpath_len += 2;
! }
! else
! {
! memcpy(xpath_expr, "/x//", 4);
! memcpy(xpath_expr + 4, VARDATA(xpath_expr_text), xpath_len);
! xpath_expr[xpath_len + 4] = '\0';
! xpath_len += 4;
! }
!
! }
! else
! {
! /*
! * if we didn't need to mangle the XML, we don't need to mangle the
! * xpath either.
! */
! memcpy(xpath_expr, VARDATA(xpath_expr_text), xpath_len);
! xpath_expr[xpath_len] = '\0';
! }
!
xpathctx = xmlXPathNewContext(doc);
if (xpathctx == NULL)
xml_ereport(ERROR, ERRCODE_OUT_OF_MEMORY,
Andrew Dunstan <andrew@dunslane.net> writes:
>> I'll do some tests to see what the cost of extra xml parsing might be.
> The extra cost appears to be fairly negligible.
Uh, you didn't actually show a comparison of before and after?
What it looks like to me is that this approach is free or nearly so for
well-formed documents, but doubles the parsing cost for forests.
Which is likely to annoy anyone who's really depending on the
capability.
Also,
> ! if (*VARDATA(xpath_expr_text) == '/')
This is risking a core dump if the xpath expr is of zero length. You
need something like
if (xpath_len > 0 && *VARDATA(xpath_expr_text) == '/')
It would also be a good idea if the allocation of string and xpath_expr
had a comment about why it's allocating extra space (something like "see
hacks below for use of this extra space" would be sufficient).
regards, tom lane
Tom Lane wrote: > Andrew Dunstan <andrew@dunslane.net> writes: > >>> I'll do some tests to see what the cost of extra xml parsing might be. >>> > > >> The extra cost appears to be fairly negligible. >> > > Uh, you didn't actually show a comparison of before and after? > What it looks like to me is that this approach is free or nearly so for > well-formed documents, but doubles the parsing cost for forests. > Which is likely to annoy anyone who's really depending on the > capability. > The difference is lost in the noise. Without fix: Time: 24.619 ms Time: 24.245 ms Time: 25.179 ms With fix: Time: 24.084 ms Time: 21.980 ms Time: 23.765 ms The test is done on 10,000 short fragments each parsed 10 times (or 20 times with the fix). I'll test again on some longer fragments since you don't seem convinced. > Also, > > >> ! if (*VARDATA(xpath_expr_text) == '/') >> > > This is risking a core dump if the xpath expr is of zero length. You > need something like > > if (xpath_len > 0 && *VARDATA(xpath_expr_text) == '/') > OK. > It would also be a good idea if the allocation of string and xpath_expr > had a comment about why it's allocating extra space (something like "see > hacks below for use of this extra space" would be sufficient). > OK. cheers andrew
On Fri, 2009-02-27 at 16:37 -0500, Andrew Dunstan wrote: > > Hannu Krosing wrote: > > On Thu, 2009-02-26 at 13:51 -0500, Robert Haas wrote: > > > >>> What I have proposed for 8.3 should not break a single case that currently > >>> behaves usefully. If anyone has a counter-example please show it. > >>> > >>> What I have proposed for 8.4 possibly would break current "useful" behaviour > >>> (FSVO "useful"), but should be done anyway on correctness grounds. > >>> > >> I dunno, aren't XML document fragments sort of a pretty common case? > >> > > > > I'd rather argue that xml datatype should not even accept anything but > > complete xml documents. Same as int field does not accept int[]. > > > > Or maybe we rather need separate xmldocument and xmlforest/xmlfragments > > types in next releases and leave the "base" xml as it is but deprecated > > due to inability to fix it without breaking backwards compatibility. > > > > > > Some of the functions, including some specified in the standard, produce > fragments. That's why we have the 'IS DOCUMENT' test. But then you could use xmlfragments as the functions return type, no ? Does tha standard require that the same field type must store both documents and fragments ? > You can also force validation as a document by saying SET XML OPTION > DOCUMENT; > > cheers > > andrew -- Hannu Krosing http://www.2ndQuadrant.com PostgreSQL Scalability and Availability Services, Consulting and Training
On Feb 26, 2009, at 7:03 PM, Andrew Dunstan wrote:
> can you point me at any call in libxml2 which will evaluate an xpath
> expression in the context of a nodeset instead of a document?
No, I can't. node-sets are XPath objects not xmlNode objects, so I
don't think it would be as simple as modifying:
xml.c:xpath() { ... xpathctx->node = xmlDocGetRootElement(doc);
with the result of xmlXPathNewNodeSet..
[snip other questions]
My *guess* would be that if we were to use a node-set instead, we'd
still have to prefix the XPath query. In this case, with a function
call to an xpath extension function that creates the NodeSet from the
content fragment(s?) of the document created by xml_parse(ie, more or
less, a re-implementation of exsl:node-set() tailored for our use-
case). Well, that or force the user to call it explicitly. Possible or
not--wrt using a content fragment/document as the context node, I find
this less desirable than the current mangling, so I'm becoming quite
indifferent. :)
Hannu Krosing wrote: >>> >>> >> Some of the functions, including some specified in the standard, produce >> fragments. That's why we have the 'IS DOCUMENT' test. >> > > But then you could use xmlfragments as the functions return type, no ? > > > Not in the case of xpath, no. There is a very complete standard for the Xpath data model, and we need to adhere to it. cheers andrew
sigh.. I got curious. :P
On Feb 27, 2009, at 7:19 PM, James Pye wrote:
> Well, that or force the user to call it explicitly.
Attached is the patch that I used to get the results below..
This is just a proof of concept, so it's quite lacking. Notably, it
doesn't even try to identify well-formed documents.
Purpose/idea being, give the user access to the poorly-formed document
as a node-set via the "fragment" function instead of mangling the
xpath and xml:
postgres=# SELECT xpath('fragment()//*', 'bleh<foo/><bar/>'::xml);
xpath
-------
{}
(1 row)
postgres=# SELECT xpath('fragment()//*', 'bleh<meh><sub/></meh><foo/
><bar/>'::xml);
xpath
----------
{<sub/>}
(1 row)
postgres=# SELECT xpath('fragment()/*', 'bleh<meh><sub/></meh><foo/
><bar/>'::xml);
xpath
----------
{<sub/>}
(1 row)
postgres=# SELECT xpath('fragment()', 'bleh<meh><sub/></meh><foo/><bar/
>'::xml);
xpath
------------------------
{bleh,"<meh>
<sub/>
</meh>",<foo/>,<bar/>}
(1 row)
postgres=# SELECT xpath('/*', 'bleh<meh><sub/></meh><foo/><bar/>'::xml);
xpath
-------
{}
(1 row)
postgres=# SELECT xpath('fragment()[local-name()="foo"]/@att',
'bleh<meh><sub/></meh><foo att="sometin"/><bar/>'::xml);
xpath
-----------
{sometin}
(1 row)
postgres=# SELECT xpath('fragment()[local-name()="meh"]/*',
'bleh<meh><sub/></meh><foo att="sometin"/><bar/>'::xml);
xpath
----------
{<sub/>}
(1 row)
postgres=# SELECT xpath('fragment()[local-name()="meh" or local-
name()="bar"]', 'bleh<meh><sub/></meh><foo att="sometin"/><bar/>'::xml);
xpath
-----------------
{"<meh>
<sub/>
</meh>",<bar/>}
(1 row)
postgres=# SELECT xpath('fragment()[local-name()="bar"]',
'bleh<meh><sub/></meh><foo att="sometin"/><bar/>'::xml);
xpath
----------
{<bar/>}
(1 row)
postgres=# SELECT xpath('fragment()[@*]',
'bleh<meh><sub/></meh>othertext<foo att="sometin"/><bar/>'::xml);
xpath
----------------------------
{"<foo att=\"sometin\"/>"}
(1 row)
Can't say that I've ever been thrilled with using node-sets, but
*shrug*.
I'm sleepy now..
Attachment
James Pye wrote: > sigh.. I got curious. :P > > On Feb 27, 2009, at 7:19 PM, James Pye wrote: >> Well, that or force the user to call it explicitly. > > > Attached is the patch that I used to get the results below.. > This is just a proof of concept, so it's quite lacking. Notably, it > doesn't even try to identify well-formed documents. This is entirely out of the question for 8.3, as it's a significant change of behaviour. I'd also want to see this usage blessed by some xpath guru ... I'm not sure it meets the standard's requirements, but I could be wrong. And it seems to me much better to provide the facility as a separate function e.g. xpath_fragment() (if at all) rather than by adding on a non-standard xpath function, but that's just a first impression. Nice piece of work, though. cheers andrew
On Feb 28, 2009, at 7:53 AM, Andrew Dunstan wrote: > This is entirely out of the question for 8.3, as it's a significant > change of behaviour. Yep. Even with implicit prefixing, the semantics are very different. What got me thinking about it was this: http://archives.postgresql.org/pgsql-bugs/2008-07/msg00058.php If it's desirable to avoid prefixing, what options remain? (At least I find it desirable to avoid prefixing =) > I'd also want to see this usage blessed by some xpath guru ... I'm > not sure it meets the standard's requirements, but I could be wrong. Oh, the context node question you raised? I think it would be easy to expect that the standard is expecting a well-formed document to query against in the first place, so I *do* think it's a very valid concern. http://www.w3.org/TR/xpath http://www.w3.org/TR/xpath#data-model http://www.w3.org/TR/xpath#infoset Curious, if we constructed an actual document fragment node from the node list and set it as the document's root, would that be enough to satisfy any requirements? It does appear to talk about nodes quite generally. In the current case, we're shaving the corners of the square peg so it will fit in the round hole. In fragment()'s case, it seems we would be trying to circumvent the round hole altogether.. I don't really like either way. :P
I wrote:
>
>
>
> I'll test again on some longer fragments since you don't seem convinced.
>
>
I set up a test with a much larger XML fragment - over 1Mb - basically
it's the English source of the SVN Turtle book.
The result is that the extra parsing cost is still pretty much unmeasurable:
regression=# select avg(length(foo)) from (select
repeat(xpath('//title',src)::text,i) as foo from xpathtest4,
generate_series(1,100) as i ) x; avg
----------------------1309869.000000000000
(1 row)
Without fix:
Time: 5695.930 ms
Time: 4855.837 ms
Time: 5453.044 ms
With fix:
Time: 5232.810 ms
Time: 5272.375 ms
Time: 5528.434 ms
So I'm going to go ahead and commit this change for 8.3, with Tom's
suggested ammendments.
cheers
andrew
On Fri, 2009-02-27 at 22:55 -0500, Andrew Dunstan wrote: > > Hannu Krosing wrote: > >>> > >>> > >> Some of the functions, including some specified in the standard, produce > >> fragments. That's why we have the 'IS DOCUMENT' test. > >> > > > > But then you could use xmlfragments as the functions return type, no ? > > > > > > > > Not in the case of xpath, no. single xml document is a sub-case of xmlforest, so xmlforest should be allowed as return type, no ? > There is a very complete standard for the Xpath data model, > and we need to adhere to it. Is declaring a single all-covering "xml" data type the best or even the only way to adhere ? > cheers > > andrew > -- Hannu Krosing http://www.2ndQuadrant.com PostgreSQL Scalability and Availability Services, Consulting and Training
Hannu Krosing wrote: >> Some of the functions, including some specified in the standard, produce >> fragments. That's why we have the 'IS DOCUMENT' test. >> > > But then you could use xmlfragments as the functions return type, no ? > > Does tha standard require that the same field type must store both > documents and fragments ? > > Yes, the standard very explicitly provides for one XML type which need not be an XML document. We have no choice about that. cheers andrew
On Sun, 2009-03-01 at 10:13 -0500, Andrew Dunstan wrote: > > Hannu Krosing wrote: > >> Some of the functions, including some specified in the standard, produce > >> fragments. That's why we have the 'IS DOCUMENT' test. > >> > > > > But then you could use xmlfragments as the functions return type, no ? > > > > Does tha standard require that the same field type must store both > > documents and fragments ? > > > > > > Yes, the standard very explicitly provides for one XML type which need > not be an XML document. We have no choice about that. What is it then ? A sequence of XML elements ? Which standard does postgreSQL XML type need to confirm to - general XML DB, Xpath or some other XML ? XML Path Language (XPath) Version 1.0 -------------------------------------- starts with this Abstract: "XPath is a language for addressing parts of an XML document, designed to be used by both XSLT and XPointer." So I think that using Xpath on anything else than "XML document" is invalid and results are undefined. XML 1.0 and XML 1.1 ------------------- Also, both XML 1.0 and XML 1.1 standards are about a thing called an "XML document", so I don't see anything there, which would make us accept anything else as being "XML". And SQL 2008, Part 14: XML-Related Specifications (SQL/XML) ----------------------------------------------------------- Says: "SQL defines a predefined data type named by the following <key word>: XML. ... The data types XML(DOCUMENT(UNTYPED)), XML(DOCUMENT(ANY)), XML(DOCUMENT(XMLSCHEMA)), XML(CONTENT(UNTYPED)), XML(CONTENT(ANY)), XML(CONTENT(XMLSCHEMA)), and XML(SEQUENCE) are referred to as the XML types. Values of XML types are called XML values." So while the type itself could be called XML, there are several subtypes, like Document, Content and Sequence Could the XML(SEQUENCE) better be represented as an array of xml documents aka. xml[] , and maybe CONTENT could be done as xmlelement[] where xmlelement can be any single XML element, including CDATA and plain text ? > cheers > > andrew -- Hannu Krosing http://www.2ndQuadrant.com PostgreSQL Scalability and Availability Services, Consulting and Training
Hannu Krosing wrote: > On Sun, 2009-03-01 at 10:13 -0500, Andrew Dunstan wrote: > >> Hannu Krosing wrote: >> >>>> Some of the functions, including some specified in the standard, produce >>>> fragments. That's why we have the 'IS DOCUMENT' test. >>>> >>>> >>> But then you could use xmlfragments as the functions return type, no ? >>> >>> Does tha standard require that the same field type must store both >>> documents and fragments ? >>> >>> >>> >> Yes, the standard very explicitly provides for one XML type which need >> not be an XML document. We have no choice about that. >> > > What is it then ? A sequence of XML elements ? > In its typically oblique way, the 2003 draft says this: NOTE 1 — An XML root information item is similar to an XML document information item, with the following modifications:an XML root information item does not have a [document element] property, a [base URI] property, a [characterencoding scheme] property, or an [all declarations processed] property; and the [children] property permitsmore than one XML element information item. An SQL/XML information item is either an XML root information item or one of the following (defined in Subclause 3.1.3,“Definitions provided in Part 14”): an XML attribute information item, an XML character information item, an XMLcomment information item, an XML document type declaration information item, an XML element information item, an XMLnamespace information item, an XML notation information item, an XML processing instruction information item, an XMLunexpanded entity reference information item, or an XML unparsed entity information item. An XML value is either the null value, or a collection of SQL/XML information items, consisting of exactly one XMLroot information item, plus any other SQL/XML information items that can be reached recursively by traversing theproperties of the SQL/XML information items. > Which standard does postgreSQL XML type need to confirm to - general XML > DB, Xpath or some other XML ? > I think the XML type needs to conform to the SQL/XML spec. However, we are trying to apply XPath, which has a different data model, to that type - hence the impedance mismatch. I think that the best we can do (for 8.4, having fixed 8.3 as best we can without adversely changing behaviour) is to throw the responsibility for ensuring that the XML passed to the function is an XML document back on the programmer. Anything else, especially any mangling of the XPath expression, presents a very real danger of breaking on correct input. cheers andrew
Andrew Dunstan wrote: > can you point me at any call in libxml2 which will evaluate an xpath > expression in the context of a nodeset instead of a document? Quite > apart from anything else, xpath requires there to be a (single) context > node (see http://www.w3.org/TR/xpath20/#context ). For a doc, we set > that node to the document node, but what would it be for a node-set or a > fragment? If we can't get over that hurdle we're screwed in pursuing > your line of thought. Which may hint at the fact that running xpath on content fragments is ill-defined to begin with?!?
On Sun, 2009-03-01 at 18:22 -0500, Andrew Dunstan wrote: > I think the XML type needs to conform to the SQL/XML spec. However, we > are trying to apply XPath, which has a different data model, to that > type - hence the impedance mismatch. > > I think that the best we can do (for 8.4, having fixed 8.3 as best we > can without adversely changing behaviour) is to throw the > responsibility > for ensuring that the XML passed to the function is an XML document > back on the programmer. Anything else, especially any mangling of the > XPath > expression, presents a very real danger of breaking on correct input. Can we provide a single function to bridge the gap between fragment and document? It will be clearer to do this than to see various forms of appending/munging, even if that function is a simple wrapper around an append. -- Simon Riggs www.2ndQuadrant.comPostgreSQL Training, Services and Support
Peter Eisentraut wrote: > Andrew Dunstan wrote: >> can you point me at any call in libxml2 which will evaluate an xpath >> expression in the context of a nodeset instead of a document? Quite >> apart from anything else, xpath requires there to be a (single) >> context node (see http://www.w3.org/TR/xpath20/#context ). For a doc, >> we set that node to the document node, but what would it be for a >> node-set or a fragment? If we can't get over that hurdle we're >> screwed in pursuing your line of thought. > > Which may hint at the fact that running xpath on content fragments is > ill-defined to begin with?!? > Right. But that's no excuse for what we have been doing, which was demonstrably providing false results on good input. cheers andrew
Simon Riggs wrote: > On Sun, 2009-03-01 at 18:22 -0500, Andrew Dunstan wrote: > > >> I think the XML type needs to conform to the SQL/XML spec. However, we >> are trying to apply XPath, which has a different data model, to that >> type - hence the impedance mismatch. >> >> I think that the best we can do (for 8.4, having fixed 8.3 as best we >> can without adversely changing behaviour) is to throw the >> responsibility >> for ensuring that the XML passed to the function is an XML document >> back on the programmer. Anything else, especially any mangling of the >> XPath >> expression, presents a very real danger of breaking on correct input. >> > > Can we provide a single function to bridge the gap between fragment and > document? It will be clearer to do this than to see various forms of > appending/munging, even if that function is a simple wrapper around an > append. > > I have no objection to providing an *extra* function that explicitly wraps non-documents and prefixes the xpath expression in that case, and is documented to have limitations. But I don't think we can provide a single function that always "does the right thing", especially when that is so ill-defined in the case of fragments. cheers andrew
On Mon, 2009-03-02 at 07:54 -0500, Andrew Dunstan wrote: > > Simon Riggs wrote: > > On Sun, 2009-03-01 at 18:22 -0500, Andrew Dunstan wrote: > > > > > >> I think the XML type needs to conform to the SQL/XML spec. However, we > >> are trying to apply XPath, which has a different data model, to that > >> type - hence the impedance mismatch. > >> > >> I think that the best we can do (for 8.4, having fixed 8.3 as best we > >> can without adversely changing behaviour) is to throw the > >> responsibility > >> for ensuring that the XML passed to the function is an XML document > >> back on the programmer. Anything else, especially any mangling of the > >> XPath > >> expression, presents a very real danger of breaking on correct input. > >> > > > > Can we provide a single function to bridge the gap between fragment and > > document? It will be clearer to do this than to see various forms of > > appending/munging, even if that function is a simple wrapper around an > > append. > > > > > > I have no objection to providing an *extra* function that explicitly > wraps non-documents and prefixes the xpath expression in that case, and > is documented to have limitations. But I don't think we can provide a > single function that always "does the right thing", especially when that > is so ill-defined in the case of fragments. Is it just that in you _can't_ use Xpath on fragments, and you _need_ to pass full documents to Xpath ? At least this is my reading of Xpath standard. > cheers > > andrew -- Hannu Krosing http://www.2ndQuadrant.com PostgreSQL Scalability and Availability Services, Consulting and Training
Hannu Krosing wrote: > Is it just that in you _can't_ use Xpath on fragments, and you _need_ to > pass full documents to Xpath ? > > At least this is my reading of Xpath standard. It is easy to read the XPath standard that way, because the concept of fragments is not defined outside of SQL/XML, and is therefore unknown to the XPath standard. The question at hand is rather whether we can usefully adapt it.
Hannu Krosing wrote: > Is it just that in you _can't_ use Xpath on fragments, and you _need_ to > pass full documents to Xpath ? > > At least this is my reading of Xpath standard. > > I think that's possibly overstating it., unless I have missed something (W3 standards are sometimes not much more clear than the SQL standards ;-( ) For instance, there's this, that implies at least that the tree might not be a document: A "/" at the beginning of a path expression is an abbreviation for the initial step fn:root(self::node()) treat as document-node()/ (however, if the "/" is the entire path expression, the trailing "/" is omitted from the expansion.)The effect of this initial step is to begin the path at the root node of the tree that contains the contextnode. If the context item is not a node, a type error is raised [err:XPTY0020]. At evaluation time, if the rootnode above the context node is not a document node, a dynamic error is raised [err:XPDY0050]. The problem is that we certainly do have to provide a context node (the standard is clear about that), and unless we want to convert a non-document to a node-set as James suggested and then apply the xpath expression to each node in the node-set, we have no way of sanely specifying the context node. cheers andrew
On Mon, 2009-03-02 at 15:25 +0200, Peter Eisentraut wrote: > Hannu Krosing wrote: > > Is it just that in you _can't_ use Xpath on fragments, and you _need_ to > > pass full documents to Xpath ? > > > > At least this is my reading of Xpath standard. > > It is easy to read the XPath standard that way, because the concept of > fragments is not defined outside of SQL/XML, and is therefore unknown to > the XPath standard. How is the opposite - Does SQL/XML specify Xpath usage for XML(SEQUENCE) and XML(CONTENT) ? > The question at hand is rather whether we can > usefully adapt it. This sounds like trying to adapt integer arithmetic to lists-of-integers. Even for simple things like addition, there are several ways of doing it [1,2,3] + [1,1,1] = [1,2,3,1,1,1] [1,2,3] + [1,1,1] = [2,3,4] [1,2,3] + [1,1,1] = [[1,2,3],[1,1,1]] all seem possible and "logical" -- Hannu Krosing http://www.2ndQuadrant.com PostgreSQL Scalability and Availability Services, Consulting and Training
Andrew Dunstan wrote: > > > Hannu Krosing wrote: >> Is it just that in you _can't_ use Xpath on fragments, and you _need_ to >> pass full documents to Xpath ? >> At least this is my reading of Xpath standard. >> >> > > I think that's possibly overstating it., unless I have missed > something (W3 standards are sometimes not much more clear than the SQL > standards ;-( ) > > For instance, there's this, that implies at least that the tree might > not be a document: > > A "/" at the beginning of a path expression is an abbreviation for > the initial step fn:root(self::node()) treat as document-node()/ > (however, if the "/" is the entire path expression, the trailing "/" > is omitted from the expansion.) The effect of this initial step is > to begin the path at the root node of the tree that contains the > context node. If the context item is not a node, a type error is > raised [err:XPTY0020]. At evaluation time, if the root node above > the context node is not a document node, a dynamic error is raised > [err:XPDY0050]. > > The problem is that we certainly do have to provide a context node > (the standard is clear about that), and unless we want to convert a > non-document to a node-set as James suggested and then apply the xpath > expression to each node in the node-set, we have no way of sanely > specifying the context node. > No-one has come up with an answer to this, so I propose to remove the hackery. That leaves the question of what to do when the xml is not a well formed document ... raise an error? cheers andrew
On Mar 20, 2009, at 8:56 AM, Andrew Dunstan wrote: > Andrew Dunstan wrote: >> A "/" at the beginning of a path expression is an abbreviation for >> the initial step fn:root(self::node()) treat as document-node()/ >> (however, if the "/" is the entire path expression, the trailing >> "/" >> is omitted from the expansion.) The effect of this initial step is >> to begin the path at the root node of the tree that contains the >> context node. If the context item is not a node, a type error is >> raised [err:XPTY0020]. At evaluation time, if the root node above >> the context node is not a document node, a dynamic error is raised >> [err:XPDY0050]. >> >> The problem is that we certainly do have to provide a context node >> (the standard is clear about that), and unless we want to convert a >> non-document to a node-set as James suggested and then apply the >> xpath expression to each node in the node-set, we have no way of >> sanely specifying the context node. libxml2 only supports xpath1. Why are you referencing xpath20? And, if SQL-XML requires an xpath20 conforming xpath() function, we have bigger problems than "'/x' + xpath_str". ;) Although, this is not terribly important to me as: > No-one has come up with an answer to this, I don't feel fragment()/node-set() is a good idea from a usability standpoint alone. I only mentioned it because that's how I've always worked with fragments in the xslt1 world.. Mere curiosity drove most of the remaining interest I had in it. > so I propose to remove the hackery. I think this would probably be best for the core xpath() function. However, it may be wise to relocate the munging functionality into another function so users don't have invent their own when they feel so inclined to work with fragments. > raise an error? +1, "xpath requires a well-formed document"