Project

Profile

Help

Bug #6393

closed

XSLT 1.0: Comparison of string-valued nodeset and number fails

Added by Heiko Theißen 8 months ago. Updated about 2 months ago.

Status:
Closed
Priority:
Normal
Category:
XPath Conformance
Sprint/Milestone:
-
Start date:
2024-04-15
Due date:
% Done:

100%

Estimated time:
Applies to JS Branch:
2, Trunk
Fix Committed on JS Branch:
2, Trunk
Fixed in JS Release:
SEF Generated with:
Platforms:
Company:
-
Contact person:
-
Additional contact persons:
-

Description

Consider the following source file and stylesheet:

test.xml

<?xml-stylesheet type="text/xsl" href="test.xsl"?>
<a>A</a>

test.xsl

<?xml version="1.0"?>
<xsl:stylesheet xmlns:xsl="http://www.w3.org/1999/XSL/Transform" version="1.0">
  <xsl:template match="/">
    <span>
      <xsl:value-of select="a=1"/>
    </span>
  </xsl:template>
</xsl:stylesheet>

When I try to transform this using SaxonJS (2.6.0), I get an error:

>xslt3 -xsl:test.xsl -s:test.xml
Transformation failure: Error FORG0001 at test.xsl#4
  Supplied value "A" is not a valid xs:double
Error FORG0001 at test.xsl#4
  Supplied value "A" is not a valid xs:double

When I open the XML file in my Chrome browser and have it transformed by the browser, it works:

<span>false</span>
Actions #1

Updated by Michael Kay 8 months ago

Yes this should work.

Which XSLT compiler are you using to compile this - The SaxonJS compiler (XX) or the SaxonJ compiler (XJ)?

Actions #2

Updated by Heiko Theißen 8 months ago

Michael Kay wrote in #note-1:

Which XSLT compiler are you using to compile this - The SaxonJS compiler (XX) or the SaxonJ compiler (XJ)?

I use SaxonJS 2.6.0. (I understand that SaxonJ is a Java program, but I only use Node.js.)

Actions #3

Updated by Heiko Theißen 8 months ago

I used the SaxonJS compiler.

Actions #4

Updated by Michael Kay 8 months ago

It looks to me as if the XX compiler is failing to generate the atomization (data) expression for a general comparison in 1.0 mode (a gc10 instruction).

A data node is present in the SEF file both (a) when we compile with XJ in 1.0 mode, and (b) when we compile with XX in 3.0 mode.

It seems likely that execution in 1.0 compatibility mode is under-tested.

Actions #5

Updated by Michael Kay 8 months ago

It's possible that the lack of a data node is by design.

The relevant code in Compare.gc10comparer is

if (isNumeric(a) || isNumeric(b)) {
    return toNumeric(a) === toNumeric(b);

which would work if toNumeric(a) did its own atomization. But it doesn't.

The difficulty with 1.0 general comparison is that you don't want to atomize one of the operands if the other operand is a boolean, and you don't in general know that until run-time. The XJ compiler is careful only to generate a data node if it knows that neither operand can be boolean, and frankly that's a bit pointless because it means the runtime has to be prepared to do atomization dynamically in any case. I think a better solution for gc10 would be not to attempt to generate a data node statically, and always do atomization at run-time if neither operand is a boolean.

Actions #6

Updated by Michael Kay 8 months ago

OK, but that's exactly what Expr.gc10 is trying to do. If singleBool(lhs) and singleBool(rhs) are both false, it calls atomizeArray on both operands.

It should then be calling toNumeric(a) === toNumeric(b) where the first toNumeric() returns NaN, so the result should be false. It's not obvious where the error is coming from.

Actions #7

Updated by Michael Kay 8 months ago

Working back from the error message, it comes from the call on Atomic.invalidValue() in Atomic.stringToNumber(), which must therefore have been called with failIfInvalid=true. That's probably T_double.fromString. Which is what numberFn is calling.

I think the call on t.fromString at Atomic.js line 472 should instead call t.fromStringUnfailing.

Actions #8

Updated by Heiko Theißen 8 months ago

Michael Kay wrote in #note-4:

A data node is present in the SEF file both (a) when we compile with XJ in 1.0 mode, and (b) when we compile with XX in 3.0 mode.

The same error occurs when I execute the same transformation (compiled with XX) with version="3.0". And your analysis about t.fromString vs. t.fromStringUnfailing also means that the problem is with the runtime, not the compiler, right?

Actions #9

Updated by Michael Kay 8 months ago

Yes. The difference between the two compilers took me down a false trail. The problem is with the runtime, specifically, the conversion to numeric throwing an error rather than returning NaN on this path.

However, in 2.0 or 3.0 mode the logic for general comparisons is rather different. This time the element <a>A</a> is cast to xs:double, rather than being converted using the number() function, and this cast should fail.

Actions #10

Updated by Debbie Lockett 8 months ago

  • Status changed from New to In Progress
  • Assignee set to Debbie Lockett

Tests expression-4302 and expression-4303 added to the xslt30 test suite - to run this test in 1.0 compatibility mode, and 2.0 mode, respectively.

I tried out Mike's suggested fix in Atomic.js line 472, but unfortunately that's not it. (The t.fromString call there is already inside a try/catch, which does the same thing as t.fromStringUnfailing would. So that's not where the error comes from.)

Having dug in further, the problem is that the SaxonJS run time code actually unconditionally applies the same rules for determining the "magnitude relationship" between atomic values, when in 2.0 mode or in 1.0 compatibility mode. However the rules differ - as explained at https://www.w3.org/TR/xpath20/#id-general-comparisons.

See the convToOther function inside Compare.gc. The gc10comparer.equals method includes the 1.0 mode handling for numerics, but this is only called by doCompare, which is called after convToOther. So the error message comes from the call of XS.double.cast(a) inside convToOther; but in 1.0 mode it should be converted to a numeric using the fn:number() function instead.

I think the fix is to amend convToOther inside Compare.gc to handle these differing rules for 1.0 mode.

Actions #11

Updated by Debbie Lockett 8 months ago

  • Status changed from In Progress to Resolved
  • Applies to JS Branch 2, Trunk added
  • Fix Committed on JS Branch 2, Trunk added

Code fix committed on saxonjs2 and main (SaxonJS 3 development) branches.

Actions #12

Updated by Heiko Theißen 8 months ago

Thanks for fixing that! I have downloaded saxon-js from https://www.npmjs.com/package/saxon-js, how can I obtain the correction? Only with the next release?

Actions #13

Updated by Debbie Lockett 8 months ago

Yes, I'm afraid you'll need to wait for the next release.

Actions #14

Updated by Heiko Theißen 2 months ago

Debbie Lockett wrote in #note-13:

Yes, I'm afraid you'll need to wait for the next release.

The successor release is not yet available (on npm at least). But for the last few days the npm package xslt3 has started with the message

SaxonJS 2.6 from Saxonica (*** NO LONGER SUPPORTED ***)

and debugging revealed that this is because the release date is more than a year in the past:

31536E6 < Date.now() - Date.parse(
SaxonJS$$module$Volumes$Saxonica$src$saxonica$saxon_js_enterprise$src$main$js$nodeJS$command
.getProcessorInfo().releaseDate )

Is the successor release available elsewhere?

Actions #15

Updated by Debbie Lockett about 2 months ago

Apologies, this is both embarrassing and frustrating. But thank you for alerting us to this.

SaxonJS 2.7 is on its way...

Actions #16

Updated by Debbie Lockett about 2 months ago

  • Status changed from Resolved to Closed
  • % Done changed from 0 to 100
  • Fixed in JS Release set to SaxonJS 2.7

Bug fix applied in the SaxonJS 2.7 maintenance release.

Please register to edit this issue

Also available in: Atom PDF Tracking page