From: Casey J. <cas...@jo...> - 2013-04-30 16:13:10
|
Since the query processor has gotten stricter with the new update I have ran into a few things which don't work anymore. One of which produces this somewhat cryptic message: ERROR cannot convert xs:boolean('true') to a node set Which I know can be caused by a number of things, but in my case it was this code: let $resolved-additional-atts := for $att-ref in $attUses[*not(@default or @required="true")*] return $att-ref/@ref Specifically: *not(@default or @required="true")* It was very hard to find because the processor did not report the error on the correct line, instead it reported it several functions up in the execution stack. I had to walk through the xquery with log pushes to find the problem. To fix it, I simply changed the statement to a different equivalent boolean logic: *not(@default) and not(@required="true")* I'm sure there is a good reason for this, but the obscurity was pretty irritating. Hopefully I can help someone that finds themselves in a similar problem. Cheers, Casey -- Casey Jordan easyDITA a product of Jorsek LLC "CaseyDJordan" on LinkedIn, Twitter & Facebook (585) 348 7399 easydita.com This message is intended only for the use of the Addressee(s) and may contain information that is privileged, confidential, and/or exempt from disclosure under applicable law. If you are not the intended recipient, please be advised that any disclosure copying, distribution, or use of the information contained herein is prohibited. If you have received this communication in error, please destroy all copies of the message, whether in electronic or hard copy format, as well as attachments, and immediately contact the sender by replying to this e-mail or by phone. Thank you. |
From: <wol...@ex...> - 2013-04-30 16:36:36
|
> ERROR cannot convert xs:boolean('true') to a node set This is actually a bug in the implementation of the "or" operator. I encountered it a few days ago and have a fix on my laptop. It causes a test to fail though and I need to investigate some more. Wolfgang |
From: Casey J. <cas...@jo...> - 2013-04-30 16:38:18
|
Hmm, that is interesting because I didn't see any failing tests on our CI instance. Please let me know asap what you find out because, given that we have about 100k lines of xquery, I am pretty confident this issue exists somewhere else as well. On Tue, Apr 30, 2013 at 12:36 PM, <wol...@ex...> wrote: > > ERROR cannot convert xs:boolean('true') to a node set > > This is actually a bug in the implementation of the "or" operator. I > encountered it a few days ago and have a fix on my laptop. It causes a test > to fail though and I need to investigate some more. > > Wolfgang -- -- Casey Jordan easyDITA a product of Jorsek LLC "CaseyDJordan" on LinkedIn, Twitter & Facebook (585) 348 7399 easydita.com This message is intended only for the use of the Addressee(s) and may contain information that is privileged, confidential, and/or exempt from disclosure under applicable law. If you are not the intended recipient, please be advised that any disclosure copying, distribution, or use of the information contained herein is prohibited. If you have received this communication in error, please destroy all copies of the message, whether in electronic or hard copy format, as well as attachments, and immediately contact the sender by replying to this e-mail or by phone. Thank you. |
From: Dmitriy S. <sha...@gm...> - 2013-04-30 17:19:21
|
Wolfgang, can you share failing test? I did fix similar for "and" some time ago, this one should be similar. On Tue, Apr 30, 2013 at 8:36 PM, <wol...@ex...> wrote: > > ERROR cannot convert xs:boolean('true') to a node set > > This is actually a bug in the implementation of the "or" operator. I > encountered it a few days ago and have a fix on my laptop. It causes a test > to fail though and I need to investigate some more. > -- Dmitriy Shabanov |
From: Jason S. <js...@in...> - 2013-04-30 23:52:21
|
I am SO glad to see that you guys are already working on this one. We're in the middle of upgrading to 2.0, and this is the next hurdle for us. :-) It's really easy to recreate (and it has something to do with the OrExp, I figured out that much): 1) Create a collection named /blah 2) Inside this collection, create a document named blah.xml. 3) Document text something like <blah>Some text.</blah> 4) Run the following query using the Java Admin Client: /blah[a="A" or b="B"] This causes the following exception. org.xmldb.api.base.XMLDBException: exerr:ERROR cannot convert xs:boolean('false') to a node set at org.exist.xmldb.RemoteXPathQueryService.throwException(RemoteXPathQueryService.java:183) at org.exist.xmldb.RemoteXPathQueryService.query(RemoteXPathQueryService.java:104) at org.exist.xmldb.RemoteXPathQueryService.query(RemoteXPathQueryService.java:71) at org.exist.xmldb.RemoteXPathQueryService.execute(RemoteXPathQueryService.java:445) at org.exist.client.QueryDialog$QueryThread.run(QueryDialog.java:602) Caused by: org.exist.xquery.XPathException: exerr:ERROR exerr:ERROR cannot convert xs:boolean('false') to a node set at org.exist.xmldb.RemoteXPathQueryService.throwException(RemoteXPathQueryService.java:182) ... 4 more Caused by: org.exist.xquery.XPathException: exerr:ERROR exerr:ERROR cannot convert xs:boolean('false') to a node set at org.exist.xmldb.RemoteXPathQueryService.throwException(RemoteXPathQueryService.java:182) at org.exist.xmldb.RemoteXPathQueryService.query(RemoteXPathQueryService.java:104) at org.exist.xmldb.RemoteXPathQueryService.query(RemoteXPathQueryService.java:71) at org.exist.xmldb.RemoteXPathQueryService.execute(RemoteXPathQueryService.java:445) at org.exist.client.QueryDialog$QueryThread.run(QueryDialog.java:602) Some qualifiers: * It doesn't have to be the Admin Client. This is just the easiest way for me to explain it. I am seeing this from direct calls to the backend API as well. * There needs to exist at least one document with a "blah" document-element. * The stuff in the predicate does not matter, other than the OrExp. If the expression in the predicate is true, the error still occurs. I am looking forward to getting the fix! Thanks so much! Jason Smith Software Engineer InfoTrust Group, Inc. 500 Discovery Parkway, Suite 200 Superior, CO 80027 Email js...@in... WEB www.infotrustgroup.com This e-mail and all information included herein do not constitute a legal agreement accorded by INFOTRUST GROUP and its affiliates and subsidiaries. All legal agreements must be formulated in writing by a legal representative of INFOTRUST GROUP. Any views or opinions presented in this email are solely those of the author and do not necessarily represent those of the company. ________________________________________ From: Casey Jordan [cas...@jo...] Sent: Tuesday, April 30, 2013 10:13 AM To: exi...@li... ml Subject: [Exist-open] [ERROR cannot convert xs:boolean('true') to a node set] Something to be aware of if you are updating to lastest RC. Since the query processor has gotten stricter with the new update I have ran into a few things which don't work anymore. One of which produces this somewhat cryptic message: ERROR cannot convert xs:boolean('true') to a node set Which I know can be caused by a number of things, but in my case it was this code: let $resolved-additional-atts := for $att-ref in $attUses[not(@default or @required="true")] return $att-ref/@ref Specifically: not(@default or @required="true") It was very hard to find because the processor did not report the error on the correct line, instead it reported it several functions up in the execution stack. I had to walk through the xquery with log pushes to find the problem. To fix it, I simply changed the statement to a different equivalent boolean logic: not(@default) and not(@required="true") I'm sure there is a good reason for this, but the obscurity was pretty irritating. Hopefully I can help someone that finds themselves in a similar problem. Cheers, Casey -- Casey Jordan easyDITA a product of Jorsek LLC "CaseyDJordan" on LinkedIn, Twitter & Facebook (585) 348 7399 easydita.com<http://easydita.com> This message is intended only for the use of the Addressee(s) and may contain information that is privileged, confidential, and/or exempt from disclosure under applicable law. If you are not the intended recipient, please be advised that any disclosure copying, distribution, or use of the information contained herein is prohibited. If you have received this communication in error, please destroy all copies of the message, whether in electronic or hard copy format, as well as attachments, and immediately contact the sender by replying to this e-mail or by phone. Thank you. |
From: Jason S. <js...@in...> - 2013-05-02 14:14:46
|
Is the fix for this imminent, or will it take a while? I'm just trying to decide whether I should take time to hack in a fix for this (until the official one is ready) or just wait. Thanks! -----Original Message----- From: wol...@ex... [mailto:wol...@ex...] Sent: Tuesday, April 30, 2013 10:36 AM To: Casey Jordan Cc: exi...@li... ml Subject: Re: [Exist-open] [ERROR cannot convert xs:boolean('true') to a node set] Something to be aware of if you are updating to lastest RC. > ERROR cannot convert xs:boolean('true') to a node set This is actually a bug in the implementation of the "or" operator. I encountered it a few days ago and have a fix on my laptop. It causes a test to fail though and I need to investigate some more. Wolfgang ------------------------------------------------------------------------------ Introducing AppDynamics Lite, a free troubleshooting tool for Java/.NET Get 100% visibility into your production application - at no cost. Code-level diagnostics for performance bottlenecks with <2% overhead Download for free and get started troubleshooting in minutes. http://p.sf.net/sfu/appdyn_d2d_ap1 _______________________________________________ Exist-open mailing list Exi...@li... https://lists.sourceforge.net/lists/listinfo/exist-open |
From: <wol...@ex...> - 2013-05-02 14:18:54
|
> Is the fix for this imminent, or will it take a while? I'm just trying to decide whether I should take time to hack in a fix for this (until the official one is ready) or just wait. I have the fix, but no time to test it before commit. If you'd like to help, it's just a small change in org.exist.xquery.OpOr, around line 75: //<test>{() and ()}</test> has to return <test>false</test> if (getParent() instanceof EnclosedExpr || //First, the intermediate PathExpr (getParent() != null && getParent().getParent() == null)) { result = result.isEmpty() ? BooleanValue.FALSE : BooleanValue.TRUE; } (which seems a bit strange anyway) with a simple if (returnsType() == Type.BOOLEAN) { result = result.isEmpty() ? BooleanValue.FALSE : BooleanValue.TRUE; Wolfgang |
From: Jason S. <js...@in...> - 2013-05-02 15:19:55
|
Awesome. I will test this today and let you know if it blows anything up. Thanks! -----Original Message----- From: wol...@ex... [mailto:wol...@ex...] Sent: Thursday, May 02, 2013 8:19 AM To: Jason Smith Cc: Casey Jordan; exi...@li... ml Subject: Re: [Exist-open] [ERROR cannot convert xs:boolean('true') to a node set] Something to be aware of if you are updating to lastest RC. > Is the fix for this imminent, or will it take a while? I'm just trying to decide whether I should take time to hack in a fix for this (until the official one is ready) or just wait. I have the fix, but no time to test it before commit. If you'd like to help, it's just a small change in org.exist.xquery.OpOr, around line 75: //<test>{() and ()}</test> has to return <test>false</test> if (getParent() instanceof EnclosedExpr || //First, the intermediate PathExpr (getParent() != null && getParent().getParent() == null)) { result = result.isEmpty() ? BooleanValue.FALSE : BooleanValue.TRUE; } (which seems a bit strange anyway) with a simple if (returnsType() == Type.BOOLEAN) { result = result.isEmpty() ? BooleanValue.FALSE : BooleanValue.TRUE; Wolfgang |
From: Jason S. <js...@in...> - 2013-05-03 14:58:11
|
This does not fix the error. Here is the stack trace that shows where this error is occurring: Caused by: org.exist.xquery.XPathException: exerr:ERROR cannot convert xs:boolean('false') to a node set at org.exist.xquery.value.AtomicValue.toNodeSet(AtomicValue.java:240) at org.exist.xquery.Predicate.selectByNodeSet(Predicate.java:431) at org.exist.xquery.Predicate.evalPredicate(Predicate.java:317) at org.exist.xquery.LocationStep.processPredicate(LocationStep.java:251) at org.exist.xquery.LocationStep.applyPredicate(LocationStep.java:238) at org.exist.xquery.LocationStep.eval(LocationStep.java:459) at org.exist.xquery.AbstractExpression.eval(AbstractExpression.java:71) at org.exist.xquery.PathExpr.eval(PathExpr.java:264) at org.exist.xquery.AbstractExpression.eval(AbstractExpression.java:71) at org.exist.xquery.XQuery.execute(XQuery.java:257) at org.exist.xquery.XQuery.execute(XQuery.java:213) For now, I think I can hack a solution. Again, this is really easy to reproduce, and I am seeing it in several places in our application test suite. I'll be working on this "soonish," if no one else can get to it. I'll post again if I come up with a good solution. Jason Smith Software Engineer InfoTrust Group, Inc. 500 Discovery Parkway, Suite 200 Superior, CO 80027 Email js...@in... WEB www.infotrustgroup.com This e-mail and all information included herein do not constitute a legal agreement accorded by INFOTRUST GROUP and its affiliates and subsidiaries. All legal agreements must be formulated in writing by a legal representative of INFOTRUST GROUP. Any views or opinions presented in this email are solely those of the author and do not necessarily represent those of the company. ________________________________________ From: wol...@ex... [wol...@ex...] Sent: Thursday, May 02, 2013 8:18 AM To: Jason Smith Cc: Casey Jordan; exi...@li... ml Subject: Re: [Exist-open] [ERROR cannot convert xs:boolean('true') to a node set] Something to be aware of if you are updating to lastest RC. > Is the fix for this imminent, or will it take a while? I'm just trying to decide whether I should take time to hack in a fix for this (until the official one is ready) or just wait. I have the fix, but no time to test it before commit. If you'd like to help, it's just a small change in org.exist.xquery.OpOr, around line 75: //<test>{() and ()}</test> has to return <test>false</test> if (getParent() instanceof EnclosedExpr || //First, the intermediate PathExpr (getParent() != null && getParent().getParent() == null)) { result = result.isEmpty() ? BooleanValue.FALSE : BooleanValue.TRUE; } (which seems a bit strange anyway) with a simple if (returnsType() == Type.BOOLEAN) { result = result.isEmpty() ? BooleanValue.FALSE : BooleanValue.TRUE; Wolfgang |
From: Jason S. <js...@in...> - 2013-05-03 23:29:46
|
I think this error is present in the original 2.0 release, but I may be wrong. If I am, in fact, wrong, can somebody please correct me? :-) It definitely is a problem in the latest branch build. Wolfgang, here is the simple version of how you recreate the initial XQuery error for this thread, followed by some uneducated analysis of what I found, and a code solution that gets all my tests to pass. Anyone can feel free to use the code solution, but it is a total *HACK* and needs to be replaced with a real, solid, correct solution. QUERY: /blah[a='A' or b='B'] /blah[a='A' and b='B'] The first query causes the exception. The second is for reference. SETUP: Create document: /blah/blah.xml <blah><!-- <a>A</a><b>B</b> --></blah> Uncomment the inner elements to change behaviors. The OpOr version of the query does not fail if these are uncommented. ANALYSIS: The first thing I noticed is that for an OpOr, I see one call to Predicate.analyze() for: [((child::{}a = "A") or (child::{}b = "B"))] When I run an OpAnd, I usually see three calls to Predicate.analyze(): [((child::{}a = "A") and (child::{}b = "B"))] followed by [child::{}a = "A"] and [child::{}b = "B"] In all cases, the Predicate.executionMode gets set to NODE, not BOOLEAN. For OpAnd, this is okay most of the time, because the OpAnd.evaluate() call appears to be optimized out (the two smaller predicates are evaluated, but OpAnd.evaluate() is generally not called). So, IN SUMMARY, what appears to be happening is that we get into Predicate.evalPredicate() with Predicate.executionMode set to NODE, but with an OpOr, and occasionally an OpAnd. THE *HACK* FIX So here is the "hack" fix. Add the following method to Predicate: protected boolean isBooleanType(final Expression expression) { if(expression == null) { return false; } else if(expression instanceof OpOr || expression instanceof OpAnd) { return true; } else if(expression.getClass() == PathExpr.class) { final PathExpr pathExpr = (PathExpr)expression; return pathExpr.steps.size() == 1 && isBooleanType(pathExpr.getExpression(0)); } else { return false; } } This checks for an expression that is OpAnd or OpOr, or an expression that does nothing but wrap OpAnd or OpOr with a naked PathExpr. I found this happening in a couple of my tests, making this look a lot more complex than it is. Next, in Predicate.evalPredicate(), before the 'switch' statement, put in this code: if(isBooleanType(inner)) { recomputedExecutionMode = BOOLEAN; } This is intended to add logic to the whole 'recomputedExecutionMode' thing. Okay, now this fixed *most* of the problem. However, in OpOr, I found that in some cases I was getting back sequences that were supposed to be BOOLEAN, but contained (TRUE, FALSE). So I had to replace some of the calls to Sequence.effectiveBooleanValue() with this version, which is a little more lenient. I will leave the replacements as an exercise for the reader. private boolean effectiveBooleanValue(final Sequence sequence) throws XPathException { try { return sequence.effectiveBooleanValue(); } catch(final XPathException e) { /* * Assume FALSE is an empty sequence or a sequence of exactly one that contains * the value FALSE. Anything else is TRUE. Good assumption? Who knows? Wolfgang! */ return !( sequence.isEmpty() || (sequence.hasOne() && sequence.itemAt(0).getType() == Type.BOOLEAN && sequence.itemAt(0).toJavaObject(Boolean.class) == false)); } } I modified both OpAnd and OpOr to use this version. I *DO NOT* think that this code should be committed, as it is clearly a hack. But it does get all my tests to pass. :-) Jason Smith Software Engineer InfoTrust Group, Inc. 500 Discovery Parkway, Suite 200 Superior, CO 80027 Email js...@in... WEB www.infotrustgroup.com This e-mail and all information included herein do not constitute a legal agreement accorded by INFOTRUST GROUP and its affiliates and subsidiaries. All legal agreements must be formulated in writing by a legal representative of INFOTRUST GROUP. Any views or opinions presented in this email are solely those of the author and do not necessarily represent those of the company. |
From: Dmitriy S. <sha...@gm...> - 2013-05-05 08:07:02
|
Here simpler patch: ### Eclipse Workspace Patch 1.0 #P eXist-2.1dev Index: src/org/exist/xquery/OpOr.java =================================================================== --- src/org/exist/xquery/OpOr.java (revision 18348) +++ src/org/exist/xquery/OpOr.java (working copy) @@ -65,7 +65,10 @@ if (doOptimize) { // yes: try to optimize by looking at right operand final Sequence rs = right.eval(contextSequence, null); - if (rs.isPersistentSet()) { + if (rs.isEmpty() && ls.isEmpty()) { + result = Sequence.EMPTY_SEQUENCE; + + } else if (rs.isPersistentSet()) { NodeSet rl = ls.toNodeSet(); rl = rl.getContextNodes(contextId); NodeSet rr = rs.toNodeSet(); But I don't know yet is it workaround or solution. Try it and let me know. On Sat, May 4, 2013 at 3:25 AM, Jason Smith <js...@in...>wrote: > I think this error is present in the original 2.0 release, but I may be > wrong. > If I am, in fact, wrong, can somebody please correct me? :-) > It definitely is a problem in the latest branch build. > > Wolfgang, here is the simple version of how you recreate the initial > XQuery error for this thread, followed by some uneducated analysis of > what I found, and a code solution that gets all my tests to pass. > > Anyone can feel free to use the code solution, but it is a total *HACK* > and needs to be replaced with a real, solid, correct solution. > > QUERY: > /blah[a='A' or b='B'] > /blah[a='A' and b='B'] > The first query causes the exception. The second is for reference. > > SETUP: > Create document: > /blah/blah.xml > > <blah><!-- <a>A</a><b>B</b> --></blah> > > Uncomment the inner elements to change behaviors. The OpOr version > of the query does not fail if these are uncommented. > > > ANALYSIS: > The first thing I noticed is that for an OpOr, I see one call to > Predicate.analyze() for: > [((child::{}a = "A") or (child::{}b = "B"))] > > When I run an OpAnd, I usually see three calls to Predicate.analyze(): > [((child::{}a = "A") and (child::{}b = "B"))] > followed by > [child::{}a = "A"] > and > [child::{}b = "B"] > > In all cases, the Predicate.executionMode gets set to NODE, not BOOLEAN. > For OpAnd, this is okay most of the time, because the OpAnd.evaluate() > call appears to be optimized out (the two smaller predicates are > evaluated, but OpAnd.evaluate() is generally not called). > > So, IN SUMMARY, what appears to be happening is that we get into > Predicate.evalPredicate() with Predicate.executionMode set to NODE, but > with an OpOr, and occasionally an OpAnd. > > > THE *HACK* FIX > So here is the "hack" fix. > > Add the following method to Predicate: > > protected boolean isBooleanType(final Expression expression) > { > if(expression == null) > { > return false; > } > else if(expression instanceof OpOr || expression > instanceof OpAnd) > { > return true; > } > else if(expression.getClass() == PathExpr.class) > { > final PathExpr pathExpr = (PathExpr)expression; > return pathExpr.steps.size() == 1 && > isBooleanType(pathExpr.getExpression(0)); > } > else > { > return false; > } > } > > This checks for an expression that is OpAnd or OpOr, or an expression that > does nothing but wrap OpAnd or OpOr with a naked PathExpr. I found this > happening > in a couple of my tests, making this look a lot more complex than it is. > > Next, in Predicate.evalPredicate(), before the 'switch' statement, put in > this > code: > > if(isBooleanType(inner)) > { > recomputedExecutionMode = BOOLEAN; > } > > This is intended to add logic to the whole 'recomputedExecutionMode' thing. > > Okay, now this fixed *most* of the problem. However, in OpOr, I found that > in some cases I was getting back sequences that were supposed to be > BOOLEAN, but > contained (TRUE, FALSE). So I had to replace some of the calls to > Sequence.effectiveBooleanValue() > with this version, which is a little more lenient. I will leave the > replacements as an exercise > for the reader. > > private boolean effectiveBooleanValue(final Sequence sequence) throws > XPathException > { > try > { > return sequence.effectiveBooleanValue(); > } > catch(final XPathException e) > { > /* > * Assume FALSE is an empty sequence or a sequence > of exactly one that contains > * the value FALSE. Anything else is TRUE. Good > assumption? Who knows? Wolfgang! > */ > return !( > sequence.isEmpty() > || (sequence.hasOne() > && > sequence.itemAt(0).getType() == Type.BOOLEAN > && > sequence.itemAt(0).toJavaObject(Boolean.class) == false)); > } > } > > I modified both OpAnd and OpOr to use this version. > > I *DO NOT* think that this code should be committed, as it is clearly a > hack. > But it does get all my tests to pass. :-) > -- Dmitriy Shabanov |
From: Dmitriy S. <sha...@gm...> - 2013-05-05 08:11:28
|
A bit better code: ### Eclipse Workspace Patch 1.0 #P eXist-2.1dev Index: src/org/exist/xquery/OpOr.java =================================================================== --- src/org/exist/xquery/OpOr.java (revision 18348) +++ src/org/exist/xquery/OpOr.java (working copy) @@ -65,7 +65,10 @@ if (doOptimize) { // yes: try to optimize by looking at right operand final Sequence rs = right.eval(contextSequence, null); - if (rs.isPersistentSet()) { + if (rs.isEmpty() && ls.isEmpty()) { + result = Sequence.EMPTY_SEQUENCE; + + } else if (rs.isPersistentSet() || rs.isEmpty()) { NodeSet rl = ls.toNodeSet(); rl = rl.getContextNodes(contextId); NodeSet rr = rs.toNodeSet(); On Sun, May 5, 2013 at 12:06 PM, Dmitriy Shabanov <sha...@gm...>wrote: > Here simpler patch: > > ### Eclipse Workspace Patch 1.0 > #P eXist-2.1dev > Index: src/org/exist/xquery/OpOr.java > =================================================================== > --- src/org/exist/xquery/OpOr.java (revision 18348) > +++ src/org/exist/xquery/OpOr.java (working copy) > @@ -65,7 +65,10 @@ > if (doOptimize) { > // yes: try to optimize by looking at right operand > final Sequence rs = right.eval(contextSequence, null); > - if (rs.isPersistentSet()) { > + if (rs.isEmpty() && ls.isEmpty()) { > + result = Sequence.EMPTY_SEQUENCE; > + > + } else if (rs.isPersistentSet()) { > NodeSet rl = ls.toNodeSet(); > rl = rl.getContextNodes(contextId); > NodeSet rr = rs.toNodeSet(); > > > But I don't know yet is it workaround or solution. > > Try it and let me know. > > > On Sat, May 4, 2013 at 3:25 AM, Jason Smith <js...@in...>wrote: > >> I think this error is present in the original 2.0 release, but I may be >> wrong. >> If I am, in fact, wrong, can somebody please correct me? :-) >> It definitely is a problem in the latest branch build. >> >> Wolfgang, here is the simple version of how you recreate the initial >> XQuery error for this thread, followed by some uneducated analysis of >> what I found, and a code solution that gets all my tests to pass. >> >> Anyone can feel free to use the code solution, but it is a total *HACK* >> and needs to be replaced with a real, solid, correct solution. >> >> QUERY: >> /blah[a='A' or b='B'] >> /blah[a='A' and b='B'] >> The first query causes the exception. The second is for reference. >> >> SETUP: >> Create document: >> /blah/blah.xml >> >> <blah><!-- <a>A</a><b>B</b> --></blah> >> >> Uncomment the inner elements to change behaviors. The OpOr version >> of the query does not fail if these are uncommented. >> >> >> ANALYSIS: >> The first thing I noticed is that for an OpOr, I see one call to >> Predicate.analyze() for: >> [((child::{}a = "A") or (child::{}b = "B"))] >> >> When I run an OpAnd, I usually see three calls to Predicate.analyze(): >> [((child::{}a = "A") and (child::{}b = "B"))] >> followed by >> [child::{}a = "A"] >> and >> [child::{}b = "B"] >> >> In all cases, the Predicate.executionMode gets set to NODE, not BOOLEAN. >> For OpAnd, this is okay most of the time, because the OpAnd.evaluate() >> call appears to be optimized out (the two smaller predicates are >> evaluated, but OpAnd.evaluate() is generally not called). >> >> So, IN SUMMARY, what appears to be happening is that we get into >> Predicate.evalPredicate() with Predicate.executionMode set to NODE, but >> with an OpOr, and occasionally an OpAnd. >> >> >> THE *HACK* FIX >> So here is the "hack" fix. >> >> Add the following method to Predicate: >> >> protected boolean isBooleanType(final Expression expression) >> { >> if(expression == null) >> { >> return false; >> } >> else if(expression instanceof OpOr || expression >> instanceof OpAnd) >> { >> return true; >> } >> else if(expression.getClass() == PathExpr.class) >> { >> final PathExpr pathExpr = (PathExpr)expression; >> return pathExpr.steps.size() == 1 && >> isBooleanType(pathExpr.getExpression(0)); >> } >> else >> { >> return false; >> } >> } >> >> This checks for an expression that is OpAnd or OpOr, or an expression that >> does nothing but wrap OpAnd or OpOr with a naked PathExpr. I found this >> happening >> in a couple of my tests, making this look a lot more complex than it is. >> >> Next, in Predicate.evalPredicate(), before the 'switch' statement, put in >> this >> code: >> >> if(isBooleanType(inner)) >> { >> recomputedExecutionMode = BOOLEAN; >> } >> >> This is intended to add logic to the whole 'recomputedExecutionMode' >> thing. >> >> Okay, now this fixed *most* of the problem. However, in OpOr, I found that >> in some cases I was getting back sequences that were supposed to be >> BOOLEAN, but >> contained (TRUE, FALSE). So I had to replace some of the calls to >> Sequence.effectiveBooleanValue() >> with this version, which is a little more lenient. I will leave the >> replacements as an exercise >> for the reader. >> >> private boolean effectiveBooleanValue(final Sequence sequence) throws >> XPathException >> { >> try >> { >> return sequence.effectiveBooleanValue(); >> } >> catch(final XPathException e) >> { >> /* >> * Assume FALSE is an empty sequence or a >> sequence of exactly one that contains >> * the value FALSE. Anything else is TRUE. Good >> assumption? Who knows? Wolfgang! >> */ >> return !( >> sequence.isEmpty() >> || (sequence.hasOne() >> && >> sequence.itemAt(0).getType() == Type.BOOLEAN >> && >> sequence.itemAt(0).toJavaObject(Boolean.class) == false)); >> } >> } >> >> I modified both OpAnd and OpOr to use this version. >> >> I *DO NOT* think that this code should be committed, as it is clearly a >> hack. >> But it does get all my tests to pass. :-) >> > -- Dmitriy Shabanov |
From: <wol...@ex...> - 2013-05-09 22:16:52
|
Hi Jason, thanks for your test case and your analysis of the bug. I did some further debugging today and found that the small patch suggested by Dmitriy is correct and a good fix, because it is very unlikely to introduce side effects. I have thus committed it (with a small variation) along with your test case: http://sourceforge.net/p/exist/code/18411/ Wolfgang Am 04.05.2013 um 01:25 schrieb Jason Smith <js...@in...>: > I think this error is present in the original 2.0 release, but I may be wrong. > If I am, in fact, wrong, can somebody please correct me? :-) > It definitely is a problem in the latest branch build. > > Wolfgang, here is the simple version of how you recreate the initial > XQuery error for this thread, followed by some uneducated analysis of > what I found, and a code solution that gets all my tests to pass. > > Anyone can feel free to use the code solution, but it is a total *HACK* > and needs to be replaced with a real, solid, correct solution. > > QUERY: > /blah[a='A' or b='B'] > /blah[a='A' and b='B'] > The first query causes the exception. The second is for reference. > > SETUP: > Create document: > /blah/blah.xml > > <blah><!-- <a>A</a><b>B</b> --></blah> > > Uncomment the inner elements to change behaviors. The OpOr version > of the query does not fail if these are uncommented. > > > ANALYSIS: > The first thing I noticed is that for an OpOr, I see one call to > Predicate.analyze() for: > [((child::{}a = "A") or (child::{}b = "B"))] > > When I run an OpAnd, I usually see three calls to Predicate.analyze(): > [((child::{}a = "A") and (child::{}b = "B"))] > followed by > [child::{}a = "A"] > and > [child::{}b = "B"] > > In all cases, the Predicate.executionMode gets set to NODE, not BOOLEAN. > For OpAnd, this is okay most of the time, because the OpAnd.evaluate() > call appears to be optimized out (the two smaller predicates are > evaluated, but OpAnd.evaluate() is generally not called). > > So, IN SUMMARY, what appears to be happening is that we get into > Predicate.evalPredicate() with Predicate.executionMode set to NODE, but > with an OpOr, and occasionally an OpAnd. > > > THE *HACK* FIX > So here is the "hack" fix. > > Add the following method to Predicate: > > protected boolean isBooleanType(final Expression expression) > { > if(expression == null) > { > return false; > } > else if(expression instanceof OpOr || expression instanceof OpAnd) > { > return true; > } > else if(expression.getClass() == PathExpr.class) > { > final PathExpr pathExpr = (PathExpr)expression; > return pathExpr.steps.size() == 1 && isBooleanType(pathExpr.getExpression(0)); > } > else > { > return false; > } > } > > This checks for an expression that is OpAnd or OpOr, or an expression that > does nothing but wrap OpAnd or OpOr with a naked PathExpr. I found this happening > in a couple of my tests, making this look a lot more complex than it is. > > Next, in Predicate.evalPredicate(), before the 'switch' statement, put in this > code: > > if(isBooleanType(inner)) > { > recomputedExecutionMode = BOOLEAN; > } > > This is intended to add logic to the whole 'recomputedExecutionMode' thing. > > Okay, now this fixed *most* of the problem. However, in OpOr, I found that > in some cases I was getting back sequences that were supposed to be BOOLEAN, but > contained (TRUE, FALSE). So I had to replace some of the calls to Sequence.effectiveBooleanValue() > with this version, which is a little more lenient. I will leave the replacements as an exercise > for the reader. > > private boolean effectiveBooleanValue(final Sequence sequence) throws XPathException > { > try > { > return sequence.effectiveBooleanValue(); > } > catch(final XPathException e) > { > /* > * Assume FALSE is an empty sequence or a sequence of exactly one that contains > * the value FALSE. Anything else is TRUE. Good assumption? Who knows? Wolfgang! > */ > return !( > sequence.isEmpty() > || (sequence.hasOne() > && sequence.itemAt(0).getType() == Type.BOOLEAN > && sequence.itemAt(0).toJavaObject(Boolean.class) == false)); > } > } > > I modified both OpAnd and OpOr to use this version. > > I *DO NOT* think that this code should be committed, as it is clearly a hack. > But it does get all my tests to pass. :-) > > Jason Smith > Software Engineer > InfoTrust Group, Inc. > 500 Discovery Parkway, Suite 200 > Superior, CO 80027 > Email js...@in... > WEB www.infotrustgroup.com > This e-mail and all information included herein do not constitute a legal agreement accorded by INFOTRUST GROUP and its affiliates and subsidiaries. All legal agreements must be formulated in writing by a legal representative of INFOTRUST GROUP. Any views or opinions presented in this email are solely those of the author and do not necessarily represent those of the company. > |
From: Jason S. <js...@in...> - 2013-05-09 23:21:39
|
I merged 18411 down to the 2.0.x branch (currently 18383), and it passes the "hard" suite of tests. So as far as I can tell, this is a 100% solution. Dmitriy, you rock. Wolfgang, thank you. Couple of things: 1) I wasn't expecting you to check in the test case with that name. :-) XQueryBrokenTest should not be a test case in the code base, since the XQuery is not broken! So how about TestXPathOpOrSpecialCase, or something like that. Pick a name if you like. And next time I send you something like that, I will be much more careful to name it appropriately. 2) Can you merge the changes into the 2.0.x branch? If you need anything else from me, let me know, and Wolfgang, Dmitriy, thanks again! Jason Smith Software Engineer InfoTrust Group, Inc. 500 Discovery Parkway, Suite 200 Superior, CO 80027 Email js...@in... WEB www.infotrustgroup.com This e-mail and all information included herein do not constitute a legal agreement accorded by INFOTRUST GROUP and its affiliates and subsidiaries. All legal agreements must be formulated in writing by a legal representative of INFOTRUST GROUP. Any views or opinions presented in this email are solely those of the author and do not necessarily represent those of the company. ________________________________________ From: wol...@ex... [wol...@ex...] Sent: Thursday, May 09, 2013 4:16 PM To: Jason Smith Cc: exi...@li... ml Subject: Re: [Exist-open] [ERROR cannot convert xs:boolean('true') to a node set] Something to be aware of if you are updating to lastest RC. Hi Jason, thanks for your test case and your analysis of the bug. I did some further debugging today and found that the small patch suggested by Dmitriy is correct and a good fix, because it is very unlikely to introduce side effects. I have thus committed it (with a small variation) along with your test case: http://sourceforge.net/p/exist/code/18411/ Wolfgang Am 04.05.2013 um 01:25 schrieb Jason Smith <js...@in...>: > I think this error is present in the original 2.0 release, but I may be wrong. > If I am, in fact, wrong, can somebody please correct me? :-) > It definitely is a problem in the latest branch build. > > Wolfgang, here is the simple version of how you recreate the initial > XQuery error for this thread, followed by some uneducated analysis of > what I found, and a code solution that gets all my tests to pass. > > Anyone can feel free to use the code solution, but it is a total *HACK* > and needs to be replaced with a real, solid, correct solution. > > QUERY: > /blah[a='A' or b='B'] > /blah[a='A' and b='B'] > The first query causes the exception. The second is for reference. > > SETUP: > Create document: > /blah/blah.xml > > <blah><!-- <a>A</a><b>B</b> --></blah> > > Uncomment the inner elements to change behaviors. The OpOr version > of the query does not fail if these are uncommented. > > > ANALYSIS: > The first thing I noticed is that for an OpOr, I see one call to > Predicate.analyze() for: > [((child::{}a = "A") or (child::{}b = "B"))] > > When I run an OpAnd, I usually see three calls to Predicate.analyze(): > [((child::{}a = "A") and (child::{}b = "B"))] > followed by > [child::{}a = "A"] > and > [child::{}b = "B"] > > In all cases, the Predicate.executionMode gets set to NODE, not BOOLEAN. > For OpAnd, this is okay most of the time, because the OpAnd.evaluate() > call appears to be optimized out (the two smaller predicates are > evaluated, but OpAnd.evaluate() is generally not called). > > So, IN SUMMARY, what appears to be happening is that we get into > Predicate.evalPredicate() with Predicate.executionMode set to NODE, but > with an OpOr, and occasionally an OpAnd. > > > THE *HACK* FIX > So here is the "hack" fix. > > Add the following method to Predicate: > > protected boolean isBooleanType(final Expression expression) > { > if(expression == null) > { > return false; > } > else if(expression instanceof OpOr || expression instanceof OpAnd) > { > return true; > } > else if(expression.getClass() == PathExpr.class) > { > final PathExpr pathExpr = (PathExpr)expression; > return pathExpr.steps.size() == 1 && isBooleanType(pathExpr.getExpression(0)); > } > else > { > return false; > } > } > > This checks for an expression that is OpAnd or OpOr, or an expression that > does nothing but wrap OpAnd or OpOr with a naked PathExpr. I found this happening > in a couple of my tests, making this look a lot more complex than it is. > > Next, in Predicate.evalPredicate(), before the 'switch' statement, put in this > code: > > if(isBooleanType(inner)) > { > recomputedExecutionMode = BOOLEAN; > } > > This is intended to add logic to the whole 'recomputedExecutionMode' thing. > > Okay, now this fixed *most* of the problem. However, in OpOr, I found that > in some cases I was getting back sequences that were supposed to be BOOLEAN, but > contained (TRUE, FALSE). So I had to replace some of the calls to Sequence.effectiveBooleanValue() > with this version, which is a little more lenient. I will leave the replacements as an exercise > for the reader. > > private boolean effectiveBooleanValue(final Sequence sequence) throws XPathException > { > try > { > return sequence.effectiveBooleanValue(); > } > catch(final XPathException e) > { > /* > * Assume FALSE is an empty sequence or a sequence of exactly one that contains > * the value FALSE. Anything else is TRUE. Good assumption? Who knows? Wolfgang! > */ > return !( > sequence.isEmpty() > || (sequence.hasOne() > && sequence.itemAt(0).getType() == Type.BOOLEAN > && sequence.itemAt(0).toJavaObject(Boolean.class) == false)); > } > } > > I modified both OpAnd and OpOr to use this version. > > I *DO NOT* think that this code should be committed, as it is clearly a hack. > But it does get all my tests to pass. :-) > > Jason Smith > Software Engineer > InfoTrust Group, Inc. > 500 Discovery Parkway, Suite 200 > Superior, CO 80027 > Email js...@in... > WEB www.infotrustgroup.com > This e-mail and all information included herein do not constitute a legal agreement accorded by INFOTRUST GROUP and its affiliates and subsidiaries. All legal agreements must be formulated in writing by a legal representative of INFOTRUST GROUP. Any views or opinions presented in this email are solely those of the author and do not necessarily represent those of the company. > |
From: <wol...@ex...> - 2013-05-10 06:56:41
|
> 1) I wasn't expecting you to check in the test case with that name. :-) XQueryBrokenTest should not be a test case in the code base, since the XQuery is not broken! So how about TestXPathOpOrSpecialCase, or something like that. Sure. It was late and I just wanted to get this out of my way before I forget it. > 2) Can you merge the changes into the 2.0.x branch? Ok, they are merged now. Wolfgang |
From: Jason S. <js...@in...> - 2013-05-10 14:16:59
|
Thanks again. Besides all the feature improvements (from 1.4.x to 2.0), the new web UI presents *really* well. Thanks to the entire EXist team for putting out a really outstanding product. You all put a lot of work into this, and it really shows. Jason Smith Software Engineer InfoTrust Group, Inc. 500 Discovery Parkway, Suite 200 Superior, CO 80027 Email js...@in... WEB www.infotrustgroup.com This e-mail and all information included herein do not constitute a legal agreement accorded by INFOTRUST GROUP and its affiliates and subsidiaries. All legal agreements must be formulated in writing by a legal representative of INFOTRUST GROUP. Any views or opinions presented in this email are solely those of the author and do not necessarily represent those of the company. ________________________________________ From: wol...@ex... [wol...@ex...] Sent: Friday, May 10, 2013 12:56 AM To: Jason Smith Cc: exi...@li... ml Subject: Re: [Exist-open] [ERROR cannot convert xs:boolean('true') to a node set] Something to be aware of if you are updating to lastest RC. > 1) I wasn't expecting you to check in the test case with that name. :-) XQueryBrokenTest should not be a test case in the code base, since the XQuery is not broken! So how about TestXPathOpOrSpecialCase, or something like that. Sure. It was late and I just wanted to get this out of my way before I forget it. > 2) Can you merge the changes into the 2.0.x branch? Ok, they are merged now. Wolfgang |