Bug #2301
closed
Confusion between IDREF and NILLED properties in TinyTree
Fix Committed on Branch:
9.6
Fixed in Maintenance Release:
Description
This is a suspected bug from reading the code: it needs confirmation by creating a test case.
In the tinyTree, if an element node is nilled, we set the NILLED bit in the typeCode array: specifically NodeInfo.IS_NILLED, namely 1<<29.
When we test whether an element node has the IDREF property (used by the fn:idref() function), we return true if the IDREF bit is set: specifically, TinyTree.TYPECODE_IDREF, which is also 1 << 29.
So I think that the IDREF property will be true for any node that is nilled. However, there may be no adverse consequences, because the IDREF property is tested only for the fn:idref() function, and the fn:idref function id interested in a node only if its string value is non-empty, and a nilled node always has an empty string value.
One tricky consequence of this is that the same problem occurs in the PTree format, which is supposed to be stable across Saxon releases. In fact, I think the IDREF bit is never set for IDREF attributes (it is tested, but not set). So in a PTree, we can assume that the bit setting 1 << 29 always means "nilled". No bit setting is needed for IDREF elements because the property can be inferred from the type annotation. We only need extra information for IDREF attributes, where the property can be derived from a DTD rather than a schema.
Note also, as far as I can see the PTreeReader never sets the nilled property in the constructed tree, though the information is present in the input.
I've written a JUnit test to investigate this: TestTypes/testNillable. First thing to come out of this is that we get an IDREF validation error if an element of type IDREF is nilled (we treat the zero-length string as an IDREF to be matched against an ID). This can be fixed by a check in IdValidator.SimpleIdrefChecker.
Next thing we discover is that (for the TinyTree at least), the test for the idref() property is actually checking whether the type annotation is a type that includes an IDREF. This doesn't match the data model definition: for example if the type is xs:IDREFS and the list of IDREF values is empty, the data model says that the idrefs property should not be set. The same is true if the type is a union type that includes IDREF as an option, where there is actually no IDREF present. This is unlikely to matter much because we don't appear to make any meaningful use of NodeInfo.isIdRef(). Instead, tree models that support the fn:idref() function rely on indexing IDREF values as the tree is built. (But at present I'm having trouble seeing how this works...)
Added a test to QT3 (fn-idref-32) to show that an IDREF attribute can be nillable. Patch committed on the 9.6 and 9.7 branches to pass this test.
Note: NodeInfo.isIdRef() is called primarily by the extension function saxon:is-idref. This extension function is not documented, but it is used internally. The idref() function makes use of an internal key definition (see KeyManager.registerIdRefKey()), and the match pattern for this key is
match="(*|@*)[saxon:is-idref(.)]"
Reviewing the 9.7 code, the isIdRef() implementation for TinyTree attributes appears to be correct; but for elements it is returning true whenever the type annotation is an idRef type regardless of the actual instance.
The original problem reported in the title seems to no longer exist, since we now store a reference to the actual SchemaType object in the typeCode array, rather than an integer type code with extra bit settings. The isNilled property is now represented by holding a set of all nilled elements at the TinyTree level.
- Status changed from New to Resolved
- Assignee set to Michael Kay
- Priority changed from Low to Normal
- Found in version set to 9.6
- Fixed in version set to 9.7
I've decided to close this as RESOLVED for 9.7. Quite a few complex test cases were added to QT3 while exploring this issue, and Saxon 9.7 is passing these tests. So although there are still things in the code that look a bit fishy, in the absence of an actual test case that fails I don't think we should leave this open.
- Status changed from Resolved to Closed
- % Done changed from 0 to 100
- Fixed in version changed from 9.7 to 9.6.0.8
Bug fix applied in the Saxon 9.6.0.8 maintenance release
- Applies to branch 9.6 added
- Fix Committed on Branch 9.6 added
- Fixed in Maintenance Release 9.6.0.8 added
- Sprint/Milestone set to 9.6.0.8
Please register to edit this issue
Also available in: Atom
PDF