eXist-db / exist

eXist Native XML Database and Application Platform
https://exist-db.org
GNU Lesser General Public License v2.1
428 stars 179 forks source link

Weird "cannot convert xs:boolean('true') to a node set" message #2159

Open xatapult opened 6 years ago

xatapult commented 6 years ago

What is the problem`

The following XQuery gives an error message that as far as I understand things doesn't make sense:

let $col as document-node()* := collection('/db/apps/dashboard')
let $doc as element()* := $col/xyz (: Resolves to an empty sequence :)
return 
  $doc/*[not(self::abc)]
  (: Results in: exerr:ERROR cannot convert xs:boolean('true') to a node set :)

What did you expect

An empty sequence as output

Describe how to reproduce or add a test

Run the XQuery above

Context information

Please always add the following information

xatapult commented 6 years ago

The problem disappears if you write empty(self::abc) instead of not(self::abc) so that's an easy workaround.

However, still something seems wrong...

xatapult commented 6 years ago

Another hint: I thought it was the use of the self:: axis, but this gives the same error: $doc/*[not(*)]

joewiz commented 6 years ago

Here's a simplified test case:

<foo/>/xyz/*[not(self::abc)]

eXist returns:

exerr:ERROR cannot convert xs:boolean('true') to a node set

Saxon and BaseX both return the empty sequence.

duncdrum commented 5 years ago

might be related to #2158

adamretter commented 4 years ago

Testing on eXist-db 5.3.0-SNAPSHOT. So regards the examples of @xatapult and @joewiz I found the following to be simplest for me:

Works correctly

<foo/>/*[not(self::abc)]

Does NOT work correctly

<foo/>/xyz/*[not(self::abc)]

Analysis

Having looked through the code this is down to a bad optimisation in eXist-db around fn:not, which you can see here - https://github.com/eXist-db/exist/blob/eXist-5.2.0/exist-core/src/main/java/org/exist/xquery/functions/fn/FunNot.java#L75

public int returnsType() {
    //TODO: test for possible performance lost
    //return Type.BOOLEAN;
    return Type.subTypeOf(getArgument(0).returnsType(), Type.NODE)
        ? Type.NODE
        : Type.BOOLEAN;
}

Basically, this optimisation interacts with optimisation code in the predicate. Unfortunately that is quite complex, and obviously wrong in this instance... i.e. where the internal result of the predicate will be an empty sequence.

There are a few options for how this could be fixed:

  1. FunNot#getDependencies() which is also part of the optimisation, does not correctly calculate a dependency on the context item for the inner self:: axis call.
  2. If the optimised lookup by node in Predicate#evalPredicate throws the exception, we could fallback to the non-optimised boolean lookup.
  3. We could remove the optimisation entirely - I am not sure what the impact of that is.

This needs input from @wolfgangmm as I think that he likely authored these optimisations...

adamretter commented 4 years ago

@wolfgangmm also you should take a look at https://github.com/eXist-db/exist/issues/3289 which is similar in so far as it involved bad optimisations in the predicate evaluation, but is subtly different due to the cause that switches on those optimisations.

adamretter commented 4 years ago

Note we should check that - https://github.com/eXist-db/exist/pull/3075 did not introduce a regression in this area.