Version declaration ignored if leading comment

Bug #1205134 reported by Chris Hillery
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Zorba
Confirmed
Medium
Nicolae Brinza

Bug Description

Create a file called "foo.query" with the following contents:

(:comment:)
jsoniq version "1.0";

let $obj := { "field1" : 42 }
return $obj.field1

(I used the extention .query to prevent zorbacmd from guessing the parser to use based on the filename extension.)

Run with "zorba /tmp/foo.query", and get:

</tmp/foo.query>:2,8: static error [err:XPST0003]: invalid expression: syntax error, unexpected "version", expecting "end of file" or "," or "}"; raised at /home/ceej/zo/src/src/compiler/api/compiler_api.cpp:253

The spec (http://www.w3.org/TR/xquery-30/#id-version-declaration) says

"If [a Comment is present before the end of the version declaration], the result is implementation-dependent ; an implementation may raise an implementation-dependent static error, or ignore the comment."

It does not say that we can ignore the version declaration itself, however.

We should throw an error about the comment before the version declaration.

Revision history for this message
Chris Hillery (ceejatec) wrote :

Note that the version declaration IS being parsed as such in most cases. The following query runs without error:

------
(:comment:)
xquery version "1.0";

1
------

regardless of the filename extension, so it seems like the XQuery parser's grammar is capable of at least recognizing an *XQuery* version declaration after a comment. Conversely, if you create "foo.jq" (so that zorbacmd picks the JSONiq parser by default), this runs without error:

-----
(:comment:)
jsoniq version "1.0";

let $obj := { "field1" : 42 }
return $obj.field1
-----

So the JSONiq parser can parse JSONiq version declarations.

Somewhat weirdly, the following also works if the filename extension is .jq:

-----
(:comment:)
xquery version "1.0";

let $obj := { "field1" : 42 }
return $obj.field1
-----

So the version declaration is parsed *and ignored* in that case.

Changed in zorba:
assignee: nobody → Nicolae Brinza (nbrinza)
importance: Undecided → Medium
status: New → Confirmed
Revision history for this message
Chris Hillery (ceejatec) wrote :

My suggestion:

1. Change the XQuery parser to successfully parse both xquery and jsoniq version declarations.

2. Change both parsers to throw an explicit error if they come across a version declaration after a leading comment.

Revision history for this message
Chris Hillery (ceejatec) wrote :

IMHO the following should also raise an error in any case:

xquery version (:comment:) "1.0";

Right now it appears that this is successfully handled, even in a file named ".jq", so long as it is the first thing in the file. But it's still bad practice, and it prevents us from ever handling encoding declarations, so we should fail-fast with an error here too.

Revision history for this message
Ghislain Fourny (gislenius) wrote :

Hi Chris,

I am in favour of 2 (static error if comment before declaration) and feel nervous about 1.

Here is how I see it.

A. I think it is not a conformance problem, but a Zorba-specific "smart parser selection issue", which is outside of the scope of the XQuery or JSONiq specifications.

B. If a query with a JSONiq version declaration is parsed with an XQuery parser, a parsing error will be raised and vice versa, which I think is simple and clean. Changing the parsers to accept different version declarations would break conformance.

C. Zorba has the "smart" capability of detecting the version declaration before selecting a parser. This is easy to implement (i.e., without any parser rollback) if the version declaration is the first thing that appears. I think this is a good reason to have the XQuery parser (selected if the document does not begin with a jsoniq version declaration) raise an error if a version declaration appears after a comment.

D. I think that the file extension alone should not lead to errors if inconsistent with the content, which might be confusing to people used to Linux where the extension is just a part of the file name (if I am correct).

Does it make sense?

Revision history for this message
Chris Hillery (ceejatec) wrote :

A: I agree, this is a Zorba-specific issue.

B: That's actually not true right now. The JSONiq parser successfully parses (and then ignores) an xquery version declaration, as demonstrated in my last example above; but the XQuery parser throws an exception when it comes across a jsoniq version declaration.

C: Both parsers should IMHO raise an exception if they find any kind of version declaration after a comment. However, that error should be "version declaration after a comment", not "syntax error". The only way they can *identify* an attempted version declaration is if they can *parse* both kinds of version declarations. Hence my suggestion (1) above. Note that I only say they should *parse* it, not *honor* it!

D: I agree fully. The filename extension should only be used as a fallback in cases where the query file does not contain a version declaration.

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.