Bug 19597 - [QT3TS] prohibit-all-optional-features-2
[QT3TS] prohibit-all-optional-features-2
Status: CLOSED FIXED
Product: XPath / XQuery / XSLT
Classification: Unclassified
Component: XQuery 3 & XPath 3 Test Suite
Working drafts
PC Windows NT
: P2 normal
: ---
Assigned To: Ghislain Fourny
Mailing list for public feedback on specs from XSL and XML Query WGs
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-10-18 12:45 UTC by Tim Mills
Modified: 2013-06-11 17:31 UTC (History)
6 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Tim Mills 2012-10-18 12:45:29 UTC
This test:

     declare option prohibit-feature "all-optional-features";
     declare option require-feature "schema-aware";
     import schema namespace s = "http://www.w3.org/XQueryTest/RequireProhibitF
ature";
     ()

places the option declarations before the schema import, which is grammatically incorrect.

[6]    	Prolog 	   ::=
  ((DefaultNamespaceDecl | Setter | NamespaceDecl | Import) Separator)* 
  ((ContextItemDecl | AnnotatedDecl | OptionDecl) Separator)*

The option declarations come after declarations which affect namespace bindings, ensuring that all namespaces are bound in time for expanding the option qualified names.

It is therefore rather unfortunate that the option preventing schema import will come after the import.
Comment 1 Tim Mills 2012-10-18 13:09:24 UTC
prohibit-all-optional-features-3 has a similar problem.
Comment 2 Tim Mills 2012-10-18 13:24:06 UTC
Also:

prohibit-module-1
prohibit-schema-aware-1
prohibit-schema-aware-1
require-all-optional-features-1
require-module-1
prohibit-module-1
Comment 3 Ghislain Fourny 2012-10-18 13:40:17 UTC
Thanks Tim for noticing, it should be fixed now (swapping import and option declaration). I agree that the option's being after the schema import is unfortunate. I am keeping this open in case it needs a WG discussion.
Comment 4 Tim Mills 2012-10-18 15:12:05 UTC
Thanks.  I think require/prohibit-module-1 still needs fixing, and the error code should be XQST0016, not XPST0016.
Comment 5 O'Neil Delpratt 2012-10-22 15:37:15 UTC
Came across the same problem. Hope creator don't mind I have made the changes required to prohibit-module-1 and require-module-1 and committed them to cvs.
Comment 6 Tim Mills 2012-10-22 18:55:10 UTC
(In reply to comment #5)
> Came across the same problem. Hope creator don't mind I have made the
> changes required to prohibit-module-1 and require-module-1 and committed
> them to cvs.

These tests also have missing query bodies.
Comment 7 O'Neil Delpratt 2012-10-23 11:01:37 UTC
The bodies of these tests now added.
Comment 8 O'Neil Delpratt 2012-10-23 11:45:21 UTC
In the test case prohibit-schema-aware-2 I have added the follow line which was missing:
declare option prohibit-feature "schema-aware";
Comment 9 Tim Mills 2012-11-05 13:39:06 UTC
Confirmed fixed.
Comment 10 Christian Gruen 2013-05-06 15:55:38 UTC
For the test case "prohibit-module-1-s", I would propose to add XQST0059 as potential result, as the option declaration may be evaluated after the import of the module, thus yielding a "module not found" parsing error (and, as far as I have seen, the spec. does not require that the options are evaluated in a separate compiler step). As an alternative, the query could be modified to point to an existing module.
Comment 11 Tim Mills 2013-05-07 08:31:14 UTC
(In reply to comment #10)
> For the test case "prohibit-module-1-s", I would propose to add XQST0059 as
> potential result, as the option declaration may be evaluated after the
> import of the module, thus yielding a "module not found" parsing error (and,
> as far as I have seen, the spec. does not require that the options are
> evaluated in a separate compiler step). As an alternative, the query could
> be modified to point to an existing module.

Personal opinion follows.

The line 

declare option prohibit-feature "module";

means that the implementation must act "... as though the feature were not implemented... "  I can't see how an implementation which did not implement the module feature would attempt to resolve a module, and thus it would never raise XQST0059.
Comment 12 Michael Kay 2013-05-07 11:06:49 UTC
I agree with Christian (comment #10) and disagree with Tim (comment #11).

This test is designed to be run by implementations that support the module import feature (as indicated in the dependencies), and which may or may not have the capability to disable the feature.

When such an implementation encounters an "import module" it is perfectly entitled to attempt the module import, and because the module import cannot be resolved it is perfectly entitled to report the failure; and once an error has been reported, an implementation is perfectly entitled to report that error and then stop without looking at for further errors. If the module import were not in error, the processor would have to report an error saying that module import is prohibited, but there are two errors here (module import cannot be resolved, and module import not allowed) and I don't think we can demand that the second one takes priority.

Furthermore the test metadata contains a comment ("The test must be reported to fail if a different error is returned") which in my view has no force. The general rules apply: reporting a different error code does not cause a test to fail.

It's generally a good idea for tests to be designed so they only contain one error. This test doesn't meet that criterion; it would be best to fix it by making the "import module" resolve.

(Incidentally, the quoted phrase "must act as though" is a classic pitfall in spec language; I'm always very wary of specifications that use the "act as if" formula. They almost invariably raise questions about which of two conflicting statements in the specification takes precedence. If such constructs are unavoidable, then the provisions they override should acknowledge the conflict, for example "Unless clause 18(b) applies, a module import declaration causes a module to be imported...".)
Comment 13 Jonathan Robie 2013-06-04 15:51:01 UTC
(In reply to comment #11)

> Personal opinion follows.
> 
> The line 
> 
> declare option prohibit-feature "module";
> 
> means that the implementation must act "... as though the feature were not
> implemented... "  I can't see how an implementation which did not implement
> the module feature would attempt to resolve a module, and thus it would
> never raise XQST0059.

I agree. Here is the wording in the spec:

<quote>
A require-feature option declaration provides a whitespace-delimited list of named features that must be enabled for the module in which the option declaration occurs; if any declaration requires a feature that the implementation does not support, a static error [err:XQST0120] is raised. A prohibit-feature option declaration provides a list of named features that must not be enabled in the module in which the option declaration occurs; if any of these QNames corresponds to a feature that the implementation supports, it must disable that feature, acting as though the feature were not implemented, or raise a static error [err:XQST0131].
</quote>

Some people are asking for freedom to raise an error due to incorrect use of a prohibited feature. I don't like that - a user who gets error XQST0059 may go off and write a schema, put that in the correct location, and bump into the next error: schemas aren't allowed in this query in the first place. That's not helpful for the user.

Also, as Ghislain points out, if a feature is not implemented, raising errors defined by that feature is also not implemented, so XQST0039 does not make sense here.
Comment 14 Jonathan Robie 2013-06-04 15:55:52 UTC
The Working Group closed this bug adopting the resolution in Comment #13. 

Since Mike and Tim were not present, they should reopen if the feel this needs further discussion.
Comment 15 Ghislain Fourny 2013-06-04 16:08:11 UTC
If this helps, a possible alternate wording I suggested in February to address Mike's "Must act as though" concern is the following:

A prohibit-feature option declaration provides a list of named features that must not be supported; if any of these QNames corresponds to a feature that the implementation supports, it must raise a static error [err:XQST0128].

And to replace:

The features required or prohibited in one module of a query are independent of any features required or prohibited in other modules of the same query.

With a text that is very close to its counterpart on a 1.0 version declaration:

However, it is possible for an implementation to execute different modules with a different set of supported features. If a processor that normally supports feature A encounters a query that has a prohibit-feature option declaration on this feature A, it must do one of the following:
- Process the module using the semantics of this specification with feature A's not being supported.
- Raise a static error [err:XQST0128].

(And the same for require-feature).
Comment 16 Michael Kay 2013-06-04 18:17:28 UTC
Sorry, I don't like pushing back on a decision made in my absence, but this doesn't feel right to me. We have a general principle that when there is a static error in a query, processors are free to raise any or all static errors in the query; we don't generally have rules that say one of these takes precedence over the others, especially in cases where the one that takes precendence involves reading parts of the query that wouldn't be reached if you stop parsing after the first. The first error in this query is that the module import doesn't resolve; the second error is that module import is prohibited. There is no good reason to override the general rules that processors can report either or both of these errors; and if we're going to override a general rule with a specific one, then the wording needs to be very explicit that this is what we are doing.
Comment 17 Jonathan Robie 2013-06-04 18:24:44 UTC
The text says that the implementation must act as though the feature is not implemented, which means that it does not implement the semantics needed to raise an error like XQST0039. So I do think that is already implicit in the text.

I'd be happy to add a NOTE saying this, using XQST0039 as an example.
Comment 18 Michael Kay 2013-06-04 19:55:45 UTC
We state that the result must be "as if" the processor did not support module import, which means it "MUST raise a static error [err:XQST0016] if it encounters a module declaration or a module import." 

But the specification also says "It is a static error [err:XQST0059] if the implementation is not able to process a module import by finding a valid module definition with the specified target namespace." 

And the specification also states "If more than one error is present, or if an error condition comes within the scope of more than one error defined in this specification, then any non-empty subset of these errors may be reported." 

So a processor that does not support module import is allowed to report either or both of these errors, and therefore a processor for which "import module" has been disabled is also allowed to report either or both of them.

If we want to insist that XQST0016 takes precedence then we need need to introduce some kind of concept whereby some static errors take precedence over others.

Incidentally, we usually describe errors by saying either "It is a static error if X", or "The processor raises a static error if X". The formulation for XQST0016, which uses "MUST", is unusual, and it is not clear why, or whether this carries any force in relation to the rule about multiple errors.
Comment 19 Jonathan Robie 2013-06-04 20:07:24 UTC
(In reply to comment #18)
> We state that the result must be "as if" the processor did not support
> module import, which means it "MUST raise a static error [err:XQST0016] if
> it encounters a module declaration or a module import." 
> 
> But the specification also says "It is a static error [err:XQST0059] if the
> implementation is not able to process a module import by finding a valid
> module definition with the specified target namespace." 

But where does the second paragraph occur? In the description of Schema Import. An implementation can only encounter this condition by attempting to do an import, and the specification clearly says that the implementation must not attempt to do so:

<quote>
If an XQuery implementation does not provide the Schema Aware Feature, it MUST raise a static error [err:XQST0009] if it encounters a schema import, and it MUST raise a static error [err:XQST0075] if it encounters a validate expression.
</quote>

The definition of prohibit-feature is quite clear that this means the implementation must act as though the feature were not implemented:

<quote>
A prohibit-feature option declaration provides a list of named features that must not be enabled in the module in which the option declaration occurs; if any of these QNames corresponds to a feature that the implementation supports, it must disable that feature, acting as though the feature were not implemented, or raise a static error [err:XQST0131].
</quote>

An implementation that does not implement schema import does not try to import a schema, so it does not raise [err:XQST0059], which can only occur if an attempt to import a schema is made.
Comment 20 Jonathan Robie 2013-06-04 20:11:11 UTC
(In reply to comment #18)

> And the specification also states "If more than one error is present, or if
> an error condition comes within the scope of more than one error defined in
> this specification, then any non-empty subset of these errors may be
> reported." 

But that's not what we're looking at here. Only one error may be reported, because the implementation is quite clearly prohibited from doing a schema import. Therefore, the implementation never encounters errors by doing a schema import. The condition does not come within the scope of more than one error defined by the specification.
 
> So a processor that does not support module import is allowed to report
> either or both of these errors, and therefore a processor for which "import
> module" has been disabled is also allowed to report either or both of them.

In this case, I think you're actually arguing that if an implementation encounters errors as a result of doing something it is prohibited from doing, it should be allowed to report these spurious errors instead of the errors the specification provides. I don't think that would be a good move.
Comment 21 Michael Kay 2013-06-04 20:22:31 UTC
You say "the specification clearly says that the implementation must not attempt to do so [import the schema or module]". But it says nothing of the kind. It only says that it's a static error if it finds an import module/schema declaration. There is nothing that prevents it detecting other errors in this declaration, for example the presence of a duplicate namespace prefix or an invalid URI, or even the fact that the module/schema being imported is invalid.

You might regard it as implausible that an implementation that doesn't support import schema would waste its time looking for such errors, but there is nothing in the spec that says it is not allowed to do so; and in the specific case where the implementation doesn't yet know that it doesn't support schema/module import, because it hasn't yet reached the declaration that informs it of this fact, it is entirely plausible that it should validate the declaration as far as it is able during initial parsing.
Comment 22 Christian Gruen 2013-06-05 09:43:34 UTC
Thanks for further discussing this. I agree with Michael that the current specification does not disallow implementations to parse prohibited statements and raise errors, as it does not say anything about the order in which statements in the Prolog need to be evaluated.

From a user’s perspective, it might be surprising that a module import statement may trigger static errors when it will later be prohibited. However, I also think that require/prohibit options should have been moved to the very top of the query prolog. It feels irritating to me that a statement can be placed in a query, and will later be prohibited. It was for some reason that the version declaration was placed before all other constructs in a query, as this makes it straightforward for both the user and the parser to see/detect what is allowed and disallowed in a query. I believe the same should have happened with the require/prohibit option: module imports and other statement will be encountered by a parser before they will later be prohibited.
Comment 23 Tim Mills 2013-06-05 11:26:58 UTC
(In reply to comment #22)
> However, I also think that require/prohibit options should have been moved
> to the very top of the query prolog. It feels irritating to me that a
> statement can be placed in a query, and will later be prohibited. 

I completely agree with this and pointed it out some time ago, however it's not easy to fix because of the handling of namespace declarations in the prolog.
Comment 24 Christian Gruen 2013-06-05 12:08:22 UTC
I think this should be fixed before finalizing XQuery 3.0. It know I’m late with this, but I believe that the require/prohibit feature is more than just an option. Maybe we could add the grammar rule...

  FeatureDecl ::= "declare" "feature" ("required" | "prohibited") StringLiteral

...which is only allowed in the first part of the prolog...

  Prolog ::= ((FeatureDecl | DefaultNamespaceDecl | NamespaceDecl |
    Import | Setter ) Separator)*
    ((ContextItemDecl | AnnotatedDecl | OptionDecl) Separator)*

..and add an extra-grammatical constraint which requires FeatureDecl to appear before Import (and maybe Setters)? This solution would both be 1.0 compliant and allow namespaces as feature names.
Comment 25 Jonathan Robie 2013-06-05 12:25:30 UTC
(In reply to comment #23)
> (In reply to comment #22)
> > However, I also think that require/prohibit options should have been moved
> > to the very top of the query prolog. It feels irritating to me that a
> > statement can be placed in a query, and will later be prohibited. 
> 
> I completely agree with this and pointed it out some time ago, however it's
> not easy to fix because of the handling of namespace declarations in the
> prolog.

Yes, if this were easy to do with our grammar, we would have done so. But I think this is only irritating for implementers, users will not be irritated by getting the prohibit-feature error. 

In any implementation that does static analysis first, the static error for prohibit-feature will be raised before the dynamic-error ever occurs.  In an implementation that at least parses the query first, it's easy enough to detect the prohibited features before evaluation.
Comment 26 Michael Kay 2013-06-06 06:15:50 UTC
Another little twist on this. Consider:

import schema namespace v="http://www.w3.org/2012/xquery"
at "http://www.w3.org/2003/05/soap-envelope/";
declare option prohibit-feature "v:schema-aware";

in the case of a processor that is capable of being schema-aware, and is also capable of pretending not to be schema-aware.

It seems that such a processor must bind the prefix v to the target namespace of the schema before it can discover than it is not supposed to be schema-aware and therefore should not have done so. At the very least we must therefore allow such a processor to report problems with the namespace binding, e.g. if v is a duplicate binding. This means that the processor does NOT behave precisely in the way that a non-schema-aware processor behaves; it cannot possibly do so.
Comment 27 C. M. Sperberg-McQueen 2013-06-11 16:56:24 UTC
The WGs discussed this issue at some length on today's call.  Eventually, we converged on a view similar to that outlined in comment 15 and comment 16:  we do not wish to constrain tightly the choice of errors raised by processors which can support a given feature but which can also behave as if they did not support it.

We resolved that part of the issue by changing sec. 4.20 paragraph 3 of the spec, replacing the words

    if any of these QNames corresponds to a feature that the implementation
    supports, it must disable that feature, acting as though the feature 
    were not implemented, or raise a static error [err:XQST0131].

to

    if any of these QNames corresponds to a feature that the implementation 
    supports, it must disable that feature, acting (where possible) as though 
    the feature were not implemented, or raise a static error [err:XQST0131].

This change is to be taken as a clarification of the meaning of "disable that feature":  a processor that disables a feature should behave as if the feature were not implemented at all, but is not required to do so when doing so would entail logical contradiction or other impossibility.  (Those who believe the old wording did require particular behavior can take this change as a weakening of the conformance requirement to a recommendation.)

  --------

We also discussed how to deal with the interaction between declarations which affect namespace bindings and prohibit-feature, as illustrated by Michael Kay's example in comment 26.  We considered three options:  (1) require that all processors handle the namespace bindings effected by import schema and import module, even if these features are not supported; (2) say that option declarations must not rely on such namespace bindings (or simply that such namespace bindings are not considered when processing option declarations); (3) do nothing.

We adopted the third option, largely in view of the fact that the other options would involve changes to the spec and to implementations (and the second option would have 1.0 compatibility issues).  We reasoned that the example in comment 26 should always raise an error, and we decided (for reasons already laid out in this discussion) not to try to prescribe narrowly which error is raised.  The reasoning should probably be recorded here.

- If a processor is not schema-aware, it will raise an error on the schema import.

- If a processor is schema-aware and cannot turn schema-awareness off, it will raise an error on the prohibit-feature declaration.

- If a processor can operate in either fashion, the simplest analysis is to imagine that it processes the module from top to bottom; seeing the schema import, it shifts to schema-aware mode, and then sees that schema-awareness is prohibited; it then backtracks, processes the schema import again, and this time it raises an error on the schema import.  For processors that don't actually want to backtrack, this amounts to a requirement that when they process a schema import, they must remember that they have done so, so that attempts to prohibit schema-awareness will cause errors to be raised.  (When it sees a prohibit-feature, the processor must be in a position to raise an error if it is already in possession of guilty knowledge.)  Other analyses of this case are also possible, but those on the call all agreed (eventually) that there is no logically consistent way of handling the example that does not involve raising one error or another.  Processors must raise an error, but they do not need to agree on their logical analysis of this conundrum, and thus they do not need to agree in their error codes.

Accordingly, the WG is closing this issue report (again) and marking it RESOLVED / FIXED.  

If my reading of the historical record is correct, Michael Kay is the most recent re-opener of the issue; Michael, if you would, please review the resolution and indicate your assent by closing the bug, or your dissent by reopening it again.
Comment 28 Michael Kay 2013-06-11 17:31:19 UTC
This seems a good resolution, thanks for the lengthy rationale.