Project

Profile

Help

Bug #5811

closed

LINQ integration is broken

Added by Michael Kay almost 2 years ago. Updated over 1 year ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
.NET API
Sprint/Milestone:
Start date:
2023-01-13
Due date:
% Done:

100%

Estimated time:
Legacy ID:
Applies to branch:
12, trunk
Fix Committed on Branch:
12, trunk
Fixed in Maintenance Release:
Platforms:
.NET

Description

The testing (and in part, the implementation) of LINQ support is inadequate. There appear to be no saved/committed unit tests. This doesn't mean the code is untested - a lot of it seems to be working - but there are definitely gaps.

I have now discovered (on the unmerged local branch "localize") a set of tests that were written when the coding was done but weren't saved in the project to be run routinely. So the code was largely working in Feb 2022, but it stopped working subsequently as a result of changes elsewhere in the product, and the fact that the tests weren't integrated meant this was overlooked.

Actions #1

Updated by Michael Kay almost 2 years ago

I've created TestLinq.cs as a modified copy of TestDOM.cs, converting those tests that are reasonably easy to convert. This reveals that most things are working; however there are crashes because XNodeWrapper and XAttributeWrapper fail to implement the method getNamespaceUri().

[This tells us something about the history: it suggests that the classes were tested at some stage, before the getNamespaceUri() method was added to the NodeInfo interface.]

Adding the method, all the tests that have been converted so far pass.

Actions #2

Updated by Michael Kay almost 2 years ago

LinqDestination seems only partially implemented. There's are supporting classes LinqDestinationImpl and XDocBuilder, but there is no Saxon.Api.LinqDestination as one might expect.

I've added LinqDestination (which is trivial) on top of these, and I've converted the unit tests for DomDestination. Most are working without any trouble.

testBug4509 fails - we're failing to eliminate duplicated namespace declarations. See bug #4509.

(In fact, XDocBuilder at line 65 has a TODO: avoid creating redundant namespaces)

Actions #3

Updated by Michael Kay almost 2 years ago

  • Project changed from 20 to Saxon
  • Category set to .NET API
  • Priority changed from Low to Normal
  • Applies to branch 12, trunk added
  • Platforms .NET added
Actions #4

Updated by Michael Kay almost 2 years ago

  • Subject changed from LINQ inadequately tested to LINQ integration is broken
  • Description updated (diff)
Actions #5

Updated by Michael Kay almost 2 years ago

As well as the unit tests, I have now adapted the QT3 test driver so on request it can build the source document as a DOM or LINQ tree. Running the AxisStep tests against LINQ, I'm getting:

334 successes, 1 failures, 0 incorrect ErrorCode, 13 not run
Failing tests:
  K2-Axes-102

Turns out that this is caused by the fact that method XNodeWrapper.getAttributeValue(NamespaceUri, string) is not implemented. This is called when building an index to implement the predicate used by this test case.

Actions #6

Updated by Michael Kay almost 2 years ago

Now running the full QT3 test suite with -tree:linq, we get:

Failing tests:
  fn-data - 1
  fn-doc - 7
  fn-element-with-id - 5
  fn-for-each - 2
  fn-for-each-pair - 1
  fn-generate-id - 4
  fn-has-children - 4
  fn-id - 20
  fn-idref - 26
  fn-innermost - 19
  fn-last - 10
  fn-local-name - 3
  fn-name - 3
  fn-nilled - 17
  fn-node-name - 2
  fn-normalize-space - 1
  fn-outermost - 18
  fn-path - 20
  fn-position - 4
  fn-remove - 1
  fn-root - 2
  fn-string - 1
  fn-string-length - 1
  fn-unordered - 4
  xs-numeric - 2
  op-bang - 2
  op-except - 7
  op-intersect - 6
  op-NOTATION-equal - 10
  op-numeric-add - 4
  op-union - 2
  prod-ArrowPostfix - 1
  prod-AxisStep.abbr - 2
  prod-AxisStep.ancestor - 4
  prod-AxisStep.following - 36
  prod-AxisStep.following-sibling - 36
  prod-AxisStep.preceding - 38
  prod-AxisStep.preceding-sibling - 36
  prod-AxisStep.unabbr - 12
  prod-CastExpr.schema - 13
  prod-Comment - 1
  prod-ContextItemExpr - 2
  prod-DirElemContent - 7
  prod-ExtensionExpr - 4
  prod-FLWORExpr - 18
  prod-FunctionDecl - 3
  prod-GroupByClause - 2
  prod-LetClause - 1
  prod-Lookup - 2
  prod-OrderByClause - 2
  prod-OrderingModeDecl - 6
  prod-Predicate - 6
  prod-SchemaImport - 17
  prod-UnaryLookup - 1
  prod-WhereClause - 1
  prod-WindowClause - 15
  misc-CombinedErrorCodes - 1
  misc-HigherOrderFunctions - 1
  app-Demos - 4
  app-FunctxFn - 5
  app-UseCaseSEQ - 1
  app-UseCaseSTRING - 1
  app-Walmsley - 2
  app-XMark - 4

Looks like preceding-sibling and following-sibling are the next place to look.

Test t:followingsibling-2 crashes like this:

-s:prod-AxisStep.following-sibling -t:followingsibling-2
in TestSet prod-AxisStep.following-sibling
System.NullReferenceException: Object reference not set to an instance of an object.
   at Saxon.Hej.tree.iter.IncrementalIterator.next() in /Users/mike/GitHub/saxon2020/build/cs/Saxon/Hej/tree/iter/IncrementalIterator.cs:line 13
   at Saxon.Hej.tree.util.Navigator.AxisFilter.next() in /Users/mike/GitHub/saxon2020/build/cs/Saxon/Hej/tree/util/Navigator.cs:line 723

This turns out to be code with a trivial error in IncrementalIterator - which is actually a Java class, though it is now longer used in SaxonJ.

This takes the results to:

30762 successes, 220 failures, 0 incorrect ErrorCode, 769 not run
Failing tests:
  fn-data - 1
  fn-doc - 5
  fn-element-with-id - 5
  fn-for-each - 2
  fn-for-each-pair - 1
  fn-generate-id - 4
  fn-has-children - 4
  fn-id - 20
  fn-idref - 26
  fn-innermost - 3
  fn-local-name - 1
  fn-name - 1
  fn-nilled - 15
  fn-normalize-space - 1
  fn-outermost - 2
  fn-remove - 1
  fn-root - 2
  fn-string - 1
  fn-string-length - 1
  xs-numeric - 2
  op-except - 3
  op-intersect - 2
  op-NOTATION-equal - 10
  op-numeric-add - 4
  prod-ArrowPostfix - 1
  prod-AxisStep.ancestor - 2
  prod-AxisStep.following-sibling - 1
  prod-AxisStep.preceding - 1
  prod-AxisStep.preceding-sibling - 1
  prod-AxisStep.unabbr - 11
  prod-CastExpr.schema - 13
  prod-Comment - 1
  prod-ContextItemExpr - 2
  prod-DirElemContent - 7
  prod-FLWORExpr - 18
  prod-FunctionDecl - 3
  prod-GroupByClause - 2
  prod-LetClause - 1
  prod-Lookup - 2
  prod-OrderByClause - 2
  prod-OrderingModeDecl - 2
  prod-Predicate - 2
  prod-SchemaImport - 17
  prod-UnaryLookup - 1
  prod-WhereClause - 1
  prod-WindowClause - 15
  misc-CombinedErrorCodes - 1
  misc-HigherOrderFunctions - 1
  app-Demos - 1
  app-FunctxFn - 4
  app-UseCaseSEQ - 1
  app-Walmsley - 2

Note that some of these, like fn:id and fn:idref, are expected to fail because we don't recognise the ID and IDREF properties on nodes in a LINQ tree.

Actions #7

Updated by Michael Kay almost 2 years ago

In fn-has-children-032, the has-children() function is returning false for an element that has a single whitespace text node child.

We are applying the test element.FirstNode==null, which returns true. However, element.IsEmpty returns false.

This appears to be a test driver issue: we should load the document with LoadOptions.PreserveWhitespace set.

Unfortunately this introduces extra test failures: prod-AxisStep is now showing 17 failures, where there were none before.

I think this is because we are now exposing whitespace text node children of the document node, which should never appear in the XDM view.

Fixing this gets us down to 181 test failures.

Actions #8

Updated by Michael Kay almost 2 years ago

The failures in windowing tests arise when sorting attribute nodes into document order; the method XAttributeWrapper.getTreeInfo() is returning null;

Down to 172 failures.

Actions #9

Updated by Michael Kay almost 2 years ago

Tests like fn:nilled and prod-SchemaImport should not be run because the tree model cannot represent type annotations. This is a test driver issue. The Java test driver handles this by forcing the tree model to TinyTree if validation is requested.

Down to 35 failures.

Actions #10

Updated by Michael Kay almost 2 years ago

Test fn-except-node-args-006 is failing because the name() function applied to a processing instruction is returning an empty string. See XNodeWrapper.getDisplayName().

Down to 28 failures:

30954 successes, 28 failures, 0 incorrect ErrorCode, 769 not run
Failing tests:
  fn-doc - 3
  fn-for-each - 1
  fn-id - 6
  fn-idref - 6
  fn-innermost - 2
  fn-outermost - 2
  fn-remove - 1
  prod-ArrowPostfix - 1
  prod-Comment - 1
  prod-FunctionDecl - 3
  prod-LetClause - 1
  prod-Lookup - 2
  prod-UnaryLookup - 1
  misc-HigherOrderFunctions - 1
Actions #11

Updated by Michael Kay almost 2 years ago

This one is strange:

-s:prod-FunctionDecl -t:K-FunctionProlog-8
Assertion error ()  failed
Expected exception XPST0003; got success
Result:
true<=======

Turns out I've been running the tests with 4.0 syntax enabled. Now down to 18 failures:

31228 successes, 18 failures, 0 incorrect ErrorCode, 505 not run
Failing tests:
  fn-doc - 3
  fn-for-each - 1
  fn-id - 6
  fn-idref - 6
  fn-innermost - 2
  fn-outermost - 2
  prod-Comment - 1

Looking at the failing test XQueryComment012, the results of the test appear to be correct but the assert-xml comparison is failing.

assert-xml starts by comparing the two serialisations: in this case they are different because the reference results contain extraneous whitespace (within a start tag).

It then goes on to call saxon:deep-equal() - essentially a tailored version of deep-equal(). This is failing because it's comparing two elements that are reported as having different numbers of attributes. I'm suspicious about the logic of the Java code in SequenceTool.hasLength(). Simplifying this logic (it's used only by the test driver though it's in the main product).

Down to 17 failures.

Actions #12

Updated by Michael Kay almost 2 years ago

The fn-doc failures are due to not ignoring LINQ doctype nodes when scanning the children of a document node.

This also fixes the IDREF failures:

31235 successes, 11 failures, 0 incorrect ErrorCode, 505 not run
Failing tests:
  fn-for-each - 1
  fn-id - 6
  fn-innermost - 2
  fn-outermost - 2

The fn-id failures are because we aren't recognizing xml:id - this is a known restriction.

Id() for xml:id attributes now implemented.

The remaining 5 failures look fairly complicated. I suspect it's something to do with sorting into document order.

Actions #13

Updated by Michael Kay almost 2 years ago

I think this might be another case of returning whitespace text node children of the document node - we've filtered those out from the child axis of the document node, but not from other axes, for example the preceding and following axes.

Fixed that.

Now down to one failure: for-each-006

Actions #14

Updated by Michael Kay almost 2 years ago

Simplifying the call on local:union() to local:union(local:children#1, local:self#1) makes the test work successfully.

In fact I can simplify the test to

let $data := (/a) 
            return $data/*! (@* | .)

which (incorrectly) returns two attributes and no elements,

Indeed, the method XAttributeWrapper.compareOrder() is incorrect: it returns 0 when comparing the position of an attribute node with that of its parent element, which means the parent element does not get added to the result of the union.

The reason this method is necessary is that the LINQ comparer XNodeDocumentOrderComparer will only compare XNode instances, and an XAttribute is not an XNode.

Actions #15

Updated by Michael Kay almost 2 years ago

Fixed XAttributeWrapper.compareOrder().

Success! QT3 results with -tree:linq now

31246 successes, 0 failures, 0 incorrect ErrorCode, 505 not run

Actions #16

Updated by Michael Kay almost 2 years ago

  • Description updated (diff)
  • Status changed from New to Resolved
  • Sprint/Milestone set to 12.0
  • Fix Committed on Branch 12, trunk added
Actions #17

Updated by O'Neil Delpratt over 1 year ago

  • Status changed from Resolved to Closed
  • % Done changed from 0 to 100
  • Fixed in Maintenance Release 12.1 added

Bug fix applied in the Saxon 12.1 maintenance release.

Please register to edit this issue

Also available in: Atom PDF