Project

Profile

Help

Bug #4803

Multiple modules with the same module URI

Added by Norm Tovey-Walsh 8 months ago. Updated 8 months ago.

Status:
Closed
Priority:
Normal
Category:
XQuery conformance
Sprint/Milestone:
-
Start date:
2020-10-24
Due date:
% Done:

100%

Estimated time:
Legacy ID:
Applies to branch:
10, trunk
Fix Committed on Branch:
10, trunk
Fixed in Maintenance Release:

Description

In an email (and Slack) discussion, several additional test cases for module import have been created. These should be packaged up and added to the QT3 test suite.

History

#1 Updated by Norm Tovey-Walsh 8 months ago

  • Assignee set to Norm Tovey-Walsh

#2 Updated by Michael Kay 8 months ago

A difficulty here is the test metadata, which defines modules in the form

<module uri="http://www.w3.org/TestModules/test" file="ModuleImport/test1-lib.xq"/>

In principle this would allow two modules with the same module URI:

<module uri="http://www.w3.org/TestModules/test" file="ModuleImport/test1-lib.xq"/>
<module uri="http://www.w3.org/TestModules/test" file="ModuleImport/test2-lib.xq"/>

But then what is the module resolver actually supposed to do? The Saxon test driver currently ignores the "location hints" in the import module declaration, and simply looks up the module URI in the test metadata to decide which file to load. If there are import module declarations with multiple location hints, or multiple import module declarations with different location hints, we don't currently have a mechanism for handling this, either in the test metadata or in the test driver.

I'd suggest expanding the metadata so it can say

<module uri="http://www.w3.org/TestModules/test" location="http://www.w3.org/ModuleImport/test1-lib.xq"  file="ModuleImport/test1-lib.xq"/>

with the semantics that if the import module includes a location hint, then it is resolved using the matching entry in the test metadata. To handle more than one location hint, there should be multiple <module> entries in the metadata.

#3 Updated by Michael Kay 8 months ago

Note, it seems the only QT3 test case that has an "import module" with a location hint is K-ModuleImport-2, and that's an error case.

So it's fairly clear that the scenario where multiple modules have the same target namespace is currently untested.

#4 Updated by Michael Kay 8 months ago

QT3 test metadata changes proposed at https://github.com/w3c/qt3tests/issues/31

#5 Updated by Michael Kay 8 months ago

I've added a test prod-ModuleImport/modules-30 to try out the new test metadata syntax, and I've implemented changes to the module URI resolver in QT3TestDriverHE to make this test pass.

#6 Updated by Michael Kay 8 months ago

  • Status changed from New to In Progress

I've added Adam's 3 test cases to prod-ModuleImport as modules-31, -32, -33.

-31 is failing; I think Saxon is getting this wrong.

-32 and -33 are passing, based on what I think the correct results should be.

#7 Updated by Michael Kay 8 months ago

So how do we fix modules-31? The relevant code is in UnboundFunctionLibrary.bindUnboundFunctionCalls(). (But we need to remember to make the same change for global variables as well).

This currently searches all the modules immediately imported into the module containing the relevant function reference. In principle, we should change this first to check that the namespace of the required function is imported into this module (this is straightforward using QueryModule.importsModuleNamespace()) and then to search all declared functions in the entire query. Unfortunately the second step isn't straightforward because Executable.globalFunctionLibrary() isn't built until the end of query processing.

In fact there's a more profound difficulty here, which is that this newly-discovered rule means that the target function isn't necessarily present anywhere in the transitive import tree of the containing module, which means that resolving the call can't always be achieved when the relevant module has been fully parsed; it needs to be deferred until the query as a whole has been processed. And of course, this completely scuppers separate compilation of query modules.

#8 Updated by Michael Kay 8 months ago

Investigating further, it seems that the fixup of unresolved function calls does in fact occur after all modules have been processed. This is done because (since 3.0) cycles are allowed. XQueryParser.makeXQueryExpression() invokes exec.fixupQueryModules(mainModule); as almost the last thing it does; unfortunately this is just before it populates the global function library maintained in the Executable. So let's see if anything breaks if we reverse the order, and build the global function library first.

#9 Updated by Michael Kay 8 months ago

Test modules-31 is now working with the following changes:

  • The QT3 test driver is changed to recognise the new location attribute

  • The QT3 test driver is changed to set the configuration property XQUERY_MULTIPLE_MODULE_IMPORTS

  • In `UnboundFunctionLibrary, we search the global function library held by the Executable

  • In XQueryExpression, we built the global function library before the final fixup of unbound function references

Given that XQuery 3.1 says (in ยง4.12.3 "Products should (by default or at user option) take account of all the location URIs in an "import module" declaration, treating each location URI as a reference to a module with the specified target namespace URI. " I think it would make sense to change defaults so that XQUERY_MULTIPLE_MODULE_IMPORTS is on by default. But these feels like a change for a major release.

#10 Updated by Michael Kay 8 months ago

  • Subject changed from Add more tests for multiple modules with the same module URI to Multiple modules with the same module URI

After all that, I've discovered that the product code changes were unnecessary; setting the property XQUERY_MULTIPLE_MODULE_IMPORTS in the test driver is all that's needed to make the test pass. So I'm reverting the changes, except those to the test driver.

#11 Updated by Michael Kay 8 months ago

  • Category set to XQuery conformance
  • Status changed from In Progress to Resolved
  • Priority changed from Low to Normal
  • Fix Committed on Branch 10, trunk added

Resolved; test driver changes only.

#12 Updated by O'Neil Delpratt 8 months ago

Bug fix applied in the Saxon 10.3 maintenance release

#13 Updated by O'Neil Delpratt 8 months ago

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

Please register to edit this issue

Also available in: Atom PDF