Bug #3899: Incorrect handling of mixed backward-compatibility xsl:key - SaxonJS - Saxonica Developer Community

Actions

Send by e-mail Copy link

Bug #3899

closed

Incorrect handling of mixed backward-compatibility xsl:key

Added by John Lumley about 6 years ago. Updated over 4 years ago.

Status:

Closed

Priority:

Low

Assignee:

John Lumley

Category:

XX compiler

Sprint/Milestone:

Start date:

2018-09-08

Due date:

% Done:

Estimated time:

Applies to JS Branch:

1.0, Trunk

Fix Committed on JS Branch:

Trunk

Fixed in JS Release:

Saxon-JS 2.0

SEF Generated with:

Platforms:

Company:

Contact person:

Additional contact persons:

Description

When operating in cases where two or more keys with the same name differ in backwards-compatibility, the implementation currently merges into the appropriate index (in document order) all matched nodes, regardless of source.

In 2.0+ keys are atomic, in 1.0 keys are strings. This means not only that the correct type for the given key backwards state should be used in forming the index entry keys, but also as the get key to extract nodes that were collected by the appropriate key declaration .

To correct this, separate indexes for 1.0 and 2.0 keys are collected and held as a pair against the key name. In cases of uniform compatibility the entry in the corresponding index are returned. With mixed compatibility entries from both indicies are combined with a node union (i.e. in document order and deduplicated)

Actions

Copy link

Updated by Michael Kay about 6 years ago

Not sure I understand. The spec says "A key (that is, a set of xsl:key declarations sharing the same key name) is processed in backwards compatible mode if (a) at least one of the xsl:key elements in the definition of the key enables backwards compatible behavior, and (b) the effective value of the composite attribute is no."

Looking at the Java code, we have the property backwardsCompatible both on the KeyDefinition (KD) and on the KeyDefinitionSet (KDS). Since the spec says it's a property of the KDS, we shouldn't really have the property on KD as well, but I think it's there as a temporary measure because we assemble the KDS later. The method KDS.addKeyDefinition() ensures that if any KD is BC, then the KDS is BC.

I think the problem is that when we export the KD, we are looking at KD.isBC rather than KDS.isBC, and can therefore end up with the "mixed" KDS that you describe.

Actions

Copy link

Updated by John Lumley about 6 years ago

Fair enough - I hadn't digested the implication of the 'one backwards, all backwards' implications of that paragraph and one of the backwards tests does have such a situation. Nothing committed to Saxon-JS. Will alter my compiler, check and then close the issue.

Actions

Copy link

Updated by John Lumley about 6 years ago

There is a minor addition needed - when we form up the index for a key, we need to record the BC state of that set such that when subsequently key() is evaluated, the correct value casting is used - this will be a function of the keyset, not any outmost XSLT version context. Tested on the XSLT compiler - need to test with EE-compiled SEFs.

Actions

Copy link