Bug #4803
closedMultiple modules with the same module URI
100%
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.
Updated by Michael Kay over 3 years 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.
Updated by Michael Kay over 3 years 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.
Updated by Michael Kay over 3 years ago
QT3 test metadata changes proposed at https://github.com/w3c/qt3tests/issues/31
Updated by Michael Kay over 3 years 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.
Updated by Michael Kay over 3 years 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.
Updated by Michael Kay over 3 years 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.
Updated by Michael Kay over 3 years 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.
Updated by Michael Kay over 3 years 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.
Updated by Michael Kay over 3 years 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.
Updated by Michael Kay over 3 years 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.
Updated by O'Neil Delpratt over 3 years ago
Bug fix applied in the Saxon 10.3 maintenance release
Updated by O'Neil Delpratt over 3 years 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