eXist-db / exist

eXist Native XML Database and Application Platform
https://exist-db.org
GNU Lesser General Public License v2.1
429 stars 179 forks source link

[BUG] internal module declaration throws java.lang.UnsupportedOperationException: TODO(AR) implement #3958

Open brobertson opened 3 years ago

brobertson commented 3 years ago

In eXist-db 5.2.0 this module declaration operates properly: import module namespace app="http://heml.mta.ca/Lace2/templates" at "modules/app.xql"

In eXist-db 5.3.0 it throws the error java.lang.UnsupportedOperationException: TODO(AR) implement

It seems that all internal modules passed to findDeclaringModule will throw this error according to the function code. Is it possible that line 106 of java/org/exist/xquery/VariableDeclaration.java ought to be uncommented and the unsupported declaration removed?

brobertson commented 3 years ago

I was able to fix this either by deleting the above declaration or variable declarations like this: declare variable $response:BAD-REQUEST := 400; So I clearly don't understand the problem and I'll close it.

adamretter commented 3 years ago

@brobertson I am afraid that the TODO(AR) means me! Can you help me to reproduce the issue you are seeing between 5.2.0 and 5.3.0? I would like to understand if it is a genuine regression...

brobertson commented 3 years ago

Hi, @adamretter Here's a minimal xquery script that doesn't throw this error in 5.2 but does in 5.3:

xquery version "3.1"; import module namespace response = "http://exist-db.org/xquery/response"; declare variable $response:NOT-FOUND := 404;

$response:NOT-FOUND

joewiz commented 3 years ago

I can confirm the issue. exist.log shows:

2021-06-29 12:20:14,557 [qtp1901070521-52] ERROR (XQueryServlet.java [process]:550) - TODO(AR) implement 
java.lang.UnsupportedOperationException: TODO(AR) implement
    at org.exist.xquery.VariableDeclaration.findDeclaringModule(VariableDeclaration.java:105) ~[exist-core-5.3.0.jar:5.3.0]
    at org.exist.xquery.VariableDeclaration.analyze(VariableDeclaration.java:81) ~[exist-core-5.3.0.jar:5.3.0]
    at org.exist.xquery.PathExpr.analyze(PathExpr.java:186) ~[exist-core-5.3.0.jar:5.3.0]
    at org.exist.xquery.XQueryContext.analyzeAndOptimizeIfModulesChanged(XQueryContext.java:1581) ~[exist-core-5.3.0.jar:5.3.0]
    at org.exist.xquery.XQuery.compile(XQuery.java:137) ~[exist-core-5.3.0.jar:5.3.0]
line-o commented 2 years ago

I suppose declaring a variable on a imported module should throw an error.

The below code throws a different exception

xquery version "3.1";

import module namespace semver = "http://exist-db.org/xquery/semver";

declare variable $semver:additional-variable := "";

$semver:additional-variable

Q{http://www.w3.org/2005/xqt-errors}XPDY0002 : It is a dynamic error if evaluation of an expression relies on some part of the dynamic context that has not been assigned a value. undefined value for variable '$semver:additional-variable'

adamretter commented 2 years ago

@line-o Nope that is fine and doesn't raise an error.

An error only comes about if the importing module declares a variable with a qname that conflicts with a qname from a variable declaration in the imported module. Then you would see something like:

An exception occurred during query execution: err:XQST0049 It is a static error if more than one variable declared or imported by a module has the same expanded QName. Variable: semver:additional-variable [at line 5, column 1]
adamretter commented 2 years ago

Consider the following for an ExternalModule (i.e. a library module implemented in XQuery):

/db/mod1.xqm

module namespace mod1 = "http://module1";
declare variable $mod1:var1 := "var1-library";

/db/main.xq

import module namespace mod1 = "http://module1" at "xmldb:exist:///db/mod1.xqm";
declare variable $mod1:var2 := "var2";

$mod1:var2

This query incorrectly causes the error:

An exception occurred during query execution: err:XPDY0002 undefined value for variable '$mod1:var2' [at line 4, column 2, source: String/427805132412344901]

This is a different error to importing an InternalModule (i.e. a library module implemented in Java) and doing the equivalent - but it seems that both have bugs with variable declarations in the main module that are for namespaces that imported into the main module.

Unfortunately InternalModule and ExternalModule have quite different implementations of their declareVariable function... neither of which at a glance look correct to me. So it will likely take a whole bunch of test cases and then some refactoring to fix this...

line-o commented 2 years ago

Fixing the internal module extension is trivial. A one-line fix.

line-o commented 2 years ago

I just don't get why XQuery allows this.

adamretter commented 2 years ago

Fixing the internal module extension is trivial. A one-line fix.

I tried an obvious one line fix and it causes problems with overwriting existing variables in InternalModules - so maybe you have a different one line fix?

adamretter commented 2 years ago

I just don't get why XQuery allows this.

There are a lot of things in the spec that are non-intuitive, but if we want to follow the spec then we need to follow it.

line-o commented 1 year ago

My one-line fix: Just returning the module instead of throwing the exception.

it causes problems with overwriting existing variables in InternalModules

Ah, so the duplicate variable declaration check must also be dynamic in addition with the static check. Also, now each variable in all namespaces must also be dynamically checked, since these might have been added later. And we have to ensure that those dynamic additions do not bleed over into other contexts. This is such a fun feature!

adamretter commented 1 year ago

That may be one way to solve it. My feeling was it was better to move the declarations to the correct modules rather than declaring them into the wrong modules.

line-o commented 1 year ago

That may be one way to solve it. My feeling was it was better to move the declarations to the correct modules rather than declaring them into the wrong modules.

I do not understand what you mean by wrong/ correct module in this context. Can you elaborate on that?