Bug #3651
closedInvalid "Type error" thrown during comparison
100%
Description
I'm attaching an XML document and a bunch of XSLT stylesheets.
Using some code like this:
public static void main(String[] args) throws TransformerException, IOException {
Transformer tr = new net.sf.saxon.TransformerFactoryImpl().newTransformer(new StreamSource("C:\\Users\\radu_coravu\\Desktop\\untypedComparison\\resources\\xhtml2ditaDriver.xsl"));
StringWriter wr = new StringWriter();
tr.transform(new StreamSource(new File("C:\\Users\\radu_coravu\\Desktop\\untypedComparison\\test.xml")), new StreamResult(wr));
wr.close();
System.err.println("AAAAAA " + wr.toString());
}
I receive this error:
Type error at char 196 in xsl:variable/@select on line 517 column 103 of xhtml2dita.xsl:
XPTY0004: Cannot compare xs:untypedAtomic to xs:untypedAtomic
at xsl:apply-templates (file:/C:/Users/radu_coravu/Desktop/untypedComparison/resources/xhtml2dita.xsl#484)
processing /html/body[1]/table[1]/tr[2]/td[1]
at xsl:apply-templates (file:/C:/Users/radu_coravu/Desktop/untypedComparison/resources/xhtml2dita.xsl#470)
processing /html/body[1]/table[1]/tr[2]
at xsl:call-template name="table" (file:/C:/Users/radu_coravu/Desktop/untypedComparison/resources/xhtml2dita.xsl#424)
in built-in template rule for /html/body[1]/table[1] in the unnamed mode
in built-in template rule for /html/body[1] in the unnamed mode
at xsl:apply-templates (file:///C:/Users/radu_coravu/Desktop/untypedComparison/resources/xhtml2ditaDriver.xsl#167)
processing /html
I cannot reproduce the error using Saxon PE 9.8, so it only occurs with Saxon HE 9.8.
If you look at line line 517 column 103 of xhtml2dita.xsl I've just about cast to xs:integer everything I got my hands on and I still get the error reported.
Files
Updated by Radu Coravu almost 7 years ago
Disabling byte code compilation and optimizations does not work, the error message changes slightly.
Updated by Radu Coravu almost 7 years ago
Rewriting one of the xpath conditions from the initial:
<xsl:variable name="previousCellsWithRowSpans" select="
ancestor::e:table//(e:th | e:td)[@rowspan castable as xs:integer][@rowspan][f:getRowIndex(.) < $currentRowIndex][f:getColIndex(.) <= $currentColIndex][@rowspan + f:getRowIndex(.) > $currentRowIndex]"/>
to:
<xsl:variable name="previousCellsWithRowSpans" select="
ancestor::e:table//(e:th | e:td)[@rowspan castable as xs:integer][@rowspan][f:getRowIndex(.) < $currentRowIndex][f:getColIndex(.) <= $currentColIndex][number(@rowspan) + number(f:getRowIndex(.)) - number($currentRowIndex) > 0]"/>
seemed to work though. But doing something like this:
[number(@rowspan) + number(f:getRowIndex(.)) > number($currentRowIndex)]
did not work.
Updated by Michael Kay almost 7 years ago
Reproduced the problem in my development environment, so it doesn't seem to be HE-specific.
The message "cannot compare xs:A with xs:B" happens when we are comparing A with B and get a ClassCastException. When the CCE is caused by something unrelated to the types of the operands, the error message is quite misleading.
Updated by Michael Kay almost 7 years ago
The actual CCE is
java.lang.ClassCastException: net.sf.saxon.value.UntypedAtomicValue cannot be cast to java.lang.Comparable
at net.sf.saxon.expr.sort.ComparableAtomicValueComparer.compareAtomicValues(ComparableAtomicValueComparer.java:73)
at net.sf.saxon.expr.ValueComparison.compare(ValueComparison.java:743)
We should't be using a ComparableAtomicValueComparer to compare untyped atomic values; UntypedAtomicValue is not a Comparable because the result of comparison depends on collations.
Updated by Michael Kay almost 7 years ago
This function has been inlined:
<xsl:function name="f:getRowIndex" as="xs:integer">
<xsl:param name="cell" as="node()"/>
<xsl:variable name="precedingRows" select="$cell/parent::e:tr/preceding-sibling::e:tr"/>
<xsl:variable name="currentRowIndex" select="count($precedingRows) + 1"/>
<xsl:value-of select="$currentRowIndex"/>
</xsl:function>
and I have a suspicion that the optimizer has tried to turn the xsl:value-of into an xsl:sequence to avoid the conversion from integer to text node and back again. But changing it to xsl:sequence doesn't make the problem go away.
Updated by Michael Kay almost 7 years ago
The actual boolean expression being evaluated is
(let $cell := .
return (convert((count($cell/preceding-sibling::element(Q{xxx}td))) + (count($cell/preceding-sibling::element(Q{xxx}th))))))
le $currentColIndex
The convert() call here is an AtomicSequenceConverter with a required type of xs:untypedAtomic.
So it looks as this results from inlining the function
<xsl:function name="f:getColIndex" as="xs:integer">
<xsl:param name="cell" as="node()"/>
<xsl:value-of select="count($cell/preceding-sibling::e:td) + count($cell/preceding-sibling::e:th)"/>
</xsl:function>
with a faulty attempt to eliminate the redundant conversion to text node and atomization.
If I change both functions getRowIndex and getColIndex to use xsl;sequence instead of xsl:value-of, the problem goes away.
I think that what is happening here is probably that the static type analysis before the function inlining computes a type of xs:integer (which means that all the attempts to cast to xs:integer are optimized away), but after inlining, the final implicit conversion of the function result from untypedAtomic to integer is being lost, resulting in two untypedAtomic values being compared using code that was expecting to compare two integers.
Updated by Michael Kay almost 7 years ago
On entry to type checking, the body of the getColIndex function is:
ValueOf(string-join(convert(data(mergeAdj((count($cell/(reverse(preceding-sibling::element(Q{http://www.oxygenxml.com/xsl/conversion-elements}td))))) + (count($cell/(reverse(preceding-sibling::element(Q{http://www.oxygenxml.com/xsl/conversion-elements}th)))))))), " "))
After type-checking this becomes
convertUntyped(Q{http://www.w3.org/2001/XMLSchema}untypedAtomic(string-join(convert((count($cell/(reverse(preceding-sibling::element(Q{http://www.oxygenxml.com/xsl/conversion-elements}td))))) + (count($cell/(reverse(preceding-sibling::element(Q{http://www.oxygenxml.com/xsl/conversion-elements}th)))))), " ")))
The convertUntyped() here represents an UntypedSequenceConverter with a required type of xs:integer. I don't know why the string-join() hasn't been eliminated at this stage, because the result of "+" has to be a singleton; but it has gone by the time the function inlining happens.
Function inlining is then applied to the function call that initializes the variable $currentColIndex at line 508. After inlining, the function body is again type-checked and optimized (so it can benefit from new information about the types of its arguments) and at this type-checking stage the conversion of the untyped text node to an integer appears to get lost.
Specifically, we have an UntypedSequenceConverter U whose operand is an AtomicSequenceConverter A, and U.typeCheck() has the rule that if A cannot return untypedAtomic values, then the expression U(A(X)) is reduced to A(X); and the code is taking this path. This is because A.getSpecialProperties() & StaticProperty.NOT_UNTYPED_ATOMIC) != 0 is true. But A should not have this property because it is clearly capable of returning untypedAtomic values.
AtomicSequenceConverter.computeSpecialProperties() delegates to UnaryExpression.computeSpecialProperties() which in turn returns the special properties of its operand. This is clearly incorrect: the operand X cannot return untypedAtomic values, but A(X) can and does.
Added the logic to AtomicSequenceConverter to set or unset the property NOT_UNTYPED_ATOMIC as appropriate, and the test case now works.
Updated by Michael Kay almost 7 years ago
- Category set to Internals
- Status changed from New to Resolved
- Assignee set to Michael Kay
- Priority changed from Low to Normal
- Applies to branch 9.8, trunk added
- Fix Committed on Branch 9.8, trunk added
Updated by Radu Coravu almost 7 years ago
Thanks for finding the time to look into this and providing the fix, sorry for being selfish and not spending more time on minimizing the XSLT samples. And this goes double for the new issue I just added #3655.
Updated by O'Neil Delpratt almost 7 years ago
- Status changed from Resolved to Closed
- % Done changed from 0 to 100
- Fixed in Maintenance Release 9.8.0.8 added
Bug fix applied in the Saxon 9.8.0.8 maintenance release.
Please register to edit this issue