Bug #6393
closed
XSLT 1.0: Comparison of string-valued nodeset and number fails
Category:
XPath Conformance
Applies to JS Branch:
2, Trunk
Fix Committed on JS Branch:
2, Trunk
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>
Yes this should work.
Which XSLT compiler are you using to compile this - The SaxonJS compiler (XX) or the SaxonJ compiler (XJ)?
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.)
I used the SaxonJS compiler.
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.
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.
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.
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.
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?
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.
- 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.
- 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.
Yes, I'm afraid you'll need to wait for the next release.
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?
Apologies, this is both embarrassing and frustrating. But thank you for alerting us to this.
SaxonJS 2.7 is on its way...
- 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
.