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

Function call as map value (official syntax) #1725

Closed duncdrum closed 6 years ago

duncdrum commented 6 years ago

What is the problem

When trying to update some older code that uses maps, I found that exist struggles with the xpath3.1 map syntax : instead of :=. The sample query below returns the following error in exist. xquery: err:XPST0081 No namespace defined for prefix pos:fn

This looks like a lexer problem the old (:=) syntax returns the desired maps in exist.

map {
    1: 5
}

map {
    2: 3
}

~~However, in the problematic case checking with Saxon9.7.0.19, I see: Cannot add a map to an XML tree which suggests I m trying to do something that I shouldn't that has nothing to do with namespaces.~~

~~The old syntax won't run: The ':=' notation is no longer accepted in map expressions: use ':' instead~~

So I am not sure what the correct response from eXist should be, ie if the query should return a result of if even the old syntax should be changed to raise an error.

What did you expect

Replacing := with :according to Xpath 3.1 specification to not result in broken queries.

this might be of interest from the specs:

For instance, consider the expression map{a:b}. Although it matches the EBNF for MapConstructor (with a matching MapKeyExpr and b matching MapValueExpr), the "longest possible match" rule requires that a:b be parsed as a QName, which results in a syntax error. Changing the expression to map{a :b} or map{a: b} will prevent this, resulting in the intended parse.

Describe how to reproduce or add a test

for $n at $pos in ("hello", "you")
return
    map {  $pos: fn:string-length($n) }

see #1510

Context information

joewiz commented 6 years ago

Confirmed, BaseX and Saxon return map { 1: 5 }, map { 2: 3 } for the test query.

joewiz commented 6 years ago

Relevant spec: https://www.w3.org/TR/xquery-31/#id-map-constructors

adamretter commented 6 years ago

Okay so the following example of a simple XQuery using maps works just fine in eXist-db 3.7.0-SNAPSHOT:

map {
    1: 5
}
,
map {
    2: 3
}

However, a more complex formulation of a query (which should produce the same result) fails in eXist-db 3.7.0-SNAPSHOT:

for $n at $pos in ("hello", "you")
return
    map {  $pos: fn:string-length($n) }

The culrpit is the map key being created from a variable expression, i.e. the $pos string followed by the : literal character. If you replace $pos: with 1: the query executes ok.

I will look into our XQuery lexer and parser to determine what is missing...

adamretter commented 6 years ago

Ultimately this boils down to our implementation of exprSingle not being correct in XQuery.g with regards the XQuery 3.1 spec:

mapConstructor throws XPathException
    :
    a:"map"! LCURLY! ( mapAssignment ( COMMA! mapAssignment )* )? RCURLY!
    {
        #mapConstructor = #(#[MAP, "map"], #mapConstructor);
        #mapConstructor.copyLexInfo(#a);
    }
    ;

mapAssignment throws XPathException
    :
    exprSingle COLON^ ( EQ! )? exprSingle
    ;
exprSingle throws XPathException
:
    ( ( "for" | "let" ) DOLLAR ) => flworExpr
    | ( "try" LCURLY ) => tryCatchExpr
    | ( ( "some" | "every" ) DOLLAR ) => quantifiedExpr
    | ( "if" LPAREN ) => ifExpr
    | ( "switch" LPAREN ) => switchExpr
    | ( "typeswitch" LPAREN ) => typeswitchExpr
    | ( "update" ( "replace" | "value" | "insert" | "delete" | "rename" )) => updateExpr
    | orExpr
    ;

In particular the flworExpr needs reworking...

adamretter commented 6 years ago

On further investigation and tracing I think out definition of unionExpr is wrong, we have:

UnionExpr ::= IntersectExceptExpr ( ("union" | "|") UnionExpr )*

but we should have according to https://www.w3.org/TR/xquery-31/#doc-xquery31-UnionExpr :

UnionExpr ::= IntersectExceptExpr ( ("union" | "|") IntersectExceptExpr )*

There may be further issues too, but that should be fixed first IMHO.

I have tried to fix it but Antlr2 is driving me mad! Personally I will wait until we have a new XQuery engine, or someone with more Antlr2 experience wishes to fix it.

wolfgangmm commented 6 years ago

I believe the issue is caused by our parsing of QNames. The spec explicitely discusses this in the section on map constructors:

For instance, consider the expression map{a:b}. Although it matches the EBNF for MapConstructor (with a matching MapKeyExpr and b matching MapValueExpr), the "longest possible match" rule requires that a:b be parsed as a QName, which results in a syntax error. Changing the expression to map{a :b} or map{a: b} will prevent this, resulting in the intended parse.

So we need to implement the "longest possible match" rule, which unfortunately turns out to not be straightforward.

duncdrum commented 6 years ago

Since there is a specs defiant work-around I don't consider this high priority, but we should not loose sight of the fact that the necessary changes impact #1510 and our ability to deprecate the old syntax in 5.0.0