BaseXdb / basex

BaseX Main Repository.
http://basex.org
BSD 3-Clause "New" or "Revised" License
678 stars 268 forks source link

Prevent typeswitch optimization from selecting supertype branches #2159

Closed GuntherRademacher closed 1 year ago

GuntherRademacher commented 1 year ago

An XQuery typeswitch returned a wrong value, e.g.

for $value in (0, 0.0)
return
  typeswitch ($value)
  case xs:integer return 'integer'
  case xs:decimal return 'decimal'
  default return 'neither'

returned ('decimal', 'decimal'), where it should have been ('integer', 'decimal').

This is caused by Typeswitch.optimize selecting a branch based on the static type of the operand expression (in the example above, xs:decimal):

    // check if always the same branch will be evaluated
    Expr expr = this;
    // choose branch that can be statically determined
    TypeswitchGroup tg = null;
    for(final TypeswitchGroup group : groups) {
      if(tg == null && group.instance(ct)) tg = group;
    }

However this is not valid when the selected branch is preceded by some other branch, where a sequence type for that branch is a subtype of the sequence type of the selected branch.

This fix replaces TypeswitchGroup.instance by a method returning the matching types, and it matches those against sequence types of preceding branches. The optimization is then only applied in absence of any matches with preceding branches.

ChristianGruen commented 1 year ago

…another excellent pull request!

I decided to rewrite the code to use classical loops. As I’m somewhat paranoid about performance, I have so far resisted switching to the Streams API, although it’s often much better readable and more compact than classical loops. In trivial cases, the runtime will more or less be the same, so it’s probably just a matter of time to get me convinced.

Instead of Stream.anyMatch, I’m calling Checks.any:

https://github.com/BaseXdb/basex/blob/1b6255d3ea7b441a80aa93a18f4b5e23185215cb/basex-core/src/main/java/org/basex/util/Checks.java#L12-L20

It’s a bit faster, as it directly operates on Iterables/arrays, but the syntax is more cryptic.

GuntherRademacher commented 1 year ago

Thanks, Christian. I had noticed that there is just one stream in the code base, but I thought I'd just start with the conviction work :wink: