Skip to content

1019 XQFO: Unknown option parameters#1059

Merged
ndw merged 2 commits intoqt4cg:masterfrom
ChristianGruen:1019
Mar 26, 2024
Merged

1019 XQFO: Unknown option parameters#1059
ndw merged 2 commits intoqt4cg:masterfrom
ChristianGruen:1019

Conversation

@ChristianGruen
Copy link
Contributor

@ChristianGruen ChristianGruen commented Mar 4, 2024

Issue: #1019

@michaelhkay
Copy link
Contributor

Thinking about this further, I'm unconvinced.

If someone uses saxon:double-space="x" as a serialization parameter, and then runs the query under BaseX, do you really want to require the implementation to throw an error? This would have a number of consequences:

(a) queries that currently run successfully would now fail.

(b) an implementation is forced to look at all options supplied, not only those it cares about.

(c) portability even across different versions/variants of the same product would become more difficult.

While I agree that the change would ensure that misspellings are caught, I'm not sure that it's a net win.

@ChristianGruen
Copy link
Contributor Author

ChristianGruen commented Mar 5, 2024

If someone uses saxon:double-space="x" as a serialization parameter, and then runs the query under BaseX, do you really want to require the implementation to throw an error?

I definitely would:

  • As the user of another processor, I wouldn’t know whether the key saxon:double-space="x" changes the result.
  • A proprietary option can lead to incompatible results and affect subsequent operations; that’s something that should not go unnoticed.
  • Larger projects nearly always rely on vendor-specific extensions. When such projects are converted to another target processor, it’s mandatory to check proprietary options, and it’s tedious to track them down when no errors are raised.

(a) queries that currently run successfully would now fail.

…provided that the current result is what a user believes it is.

(b) an implementation is forced to look at all options supplied, not only those it cares about.

…which shouldn’t be a big deal (we do it anyway), and can possibly be simplified even more when options are defined via records.

@ChristianGruen
Copy link
Contributor Author

If the proposed change seems to strict, we could change MUST to SHOULD or MAY (and thus make it completely implementation-defined):

If an option is neither described in the specification nor supported by the implementation, a type error FORG0013 SHOULD be raised.

@ChristianGruen
Copy link
Contributor Author

As motivated by @ndw (thanks), I’ve rewritten the rules to clarify that only those unknown options that are in no namespace must be rejected.

@michaelhkay pointed out that we could use plausibility checks. I guess the general rules are described in 2.4.6 Implausible Expressions. I’ve sticked with the existing presentation, as I’m not sure how this would need to be incorporated into the text.

@ndw
Copy link
Contributor

ndw commented Mar 25, 2024

Looks good to me. I'm tempted to mark this as "merge without discussion" but in any event, I hope the discussion can be brief this time around :-)

@ndw
Copy link
Contributor

ndw commented Mar 26, 2024

The CG agreed to merge this PR at meeting 071

@ndw ndw merged commit d8b42ce into qt4cg:master Mar 26, 2024
ChristianGruen added a commit to qt4cg/qt4tests that referenced this pull request Mar 26, 2024
@michaelhkay michaelhkay added XQFO An issue related to Functions and Operators Tests Needed Tests need to be written or merged labels Mar 26, 2024
@ChristianGruen ChristianGruen deleted the 1019 branch March 27, 2024 11:47
@ChristianGruen ChristianGruen added Tests Added Tests have been added to the test suites and removed Tests Needed Tests need to be written or merged labels Jan 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Tests Added Tests have been added to the test suites XQFO An issue related to Functions and Operators

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants