-
Notifications
You must be signed in to change notification settings - Fork 2
EXISTS: scoping restriction #272
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
99d07fc
to
e87de02
Compare
e87de02
to
b983250
Compare
@kasei - you had some comments about text for scoping beyond just EXISTS. This could be a good place to discuss those. |
<td>Group <code>{ P1 P2 ... }</code></td> | ||
<td><code>v</code> is in-scope if it is in-scope in one or more of P1, P2, ...</td> | ||
<td>Group `{ P1 P2 ... }`</td> | ||
<td>`v` is in-scope if it is in-scope in one or more of P1, P2, ...</td> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<td>`v` is in-scope if it is in-scope in one or more of P1, P2, ...</td> | |
<td>`v` is in-scope if it is in-scope in one or more of `P1`, `P2`, ...</td> |
<td><code>GRAPH term { P }</code></td> | ||
<td><code>v</code> is <code>term</code> or <code>v</code> is in-scope in P</td> | ||
<td>`GRAPH term { P }`</td> | ||
<td>`v` is `term` or `v` is in-scope in P</td> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<td>`v` is `term` or `v` is in-scope in P</td> | |
<td>`v` is `term` or `v` is in-scope in `P`</td> |
<td><code>{ P1 } UNION { P2 }</code></td> | ||
<td><code>v</code> is in-scope in P1 or in-scope in P2</td> | ||
<td>`{ P1 } UNION { P2 }`</td> | ||
<td>`v` is in-scope in P1 or in-scope in P2</td> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<td>`v` is in-scope in P1 or in-scope in P2</td> | |
<td>`v` is in-scope in `P1` or in-scope in `P2`</td> |
<td><code>OPTIONAL {P}</code></td> | ||
<td><code>v</code> is in-scope in P</td> | ||
<td>`OPTIONAL {P}`</td> | ||
<td>`v` is in-scope in P</td> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<td>`v` is in-scope in P</td> | |
<td>`v` is in-scope in `P`</td> |
<td><code>SERVICE term {P}</code></td> | ||
<td><code>v</code> is <code>term</code> or <code>v</code> is in-scope in P</td> | ||
<td>`SERVICE term {P}`</td> | ||
<td>`v` is `term` or `v` is in-scope in P</td> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<td>`v` is `term` or `v` is in-scope in P</td> | |
<td>`v` is `term` or `v` is in-scope in `P`</td> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not part of this PR, but "v is term" here seems really strange? Am I not meant to read "term" as indicating the use of the SERVICE IRI
form (which is the only one that has evaluation semantics)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the word "term" is misleading here. This part of the table should better be:
| SERVICE
x {
P }
| v
is x or v
is in-scope in P |
Same issue for the case of GRAPH
a bit earlier in the table.
There are two entry points into the grammar: `QueryUnit` for the SPARQL query language | ||
and `UpdateUnit` for the SPARQL update language. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are two entry points into the grammar: `QueryUnit` for the SPARQL query language | |
and `UpdateUnit` for the SPARQL update language. | |
There are two entry points into the grammar: `QueryUnit` for the SPARQL Query language | |
and `UpdateUnit` for the SPARQL Update language. |
I find this change to be VERY confusing. The existing scoping rules are all based on syntax, and can be determined statically. The new rule is:
If I'm reading this correctly (?), this is based on runtime information about the solution mapping being used in the FILTER (or BIND, etc.) expression evaluation, which would make this rule very different from all the others. I think what we actually want is to say that v is in-scope anywhere inside of an EXISTS AST, with the understanding that this will be true because of the translation that injects the solution mapping by way of the initial empty BGP inside of GGPs. This would be very different than all the other scoping rules which are all bottom-up, but… 🤷♂️ Either way, the "EXISTS and NOT EXISTS" in the "Syntax Form" column of the scoping rules table probably needs expansion to provide the details that this isn't about causing |
<tr> | ||
<td>`EXISTS` and `NOT EXISTS`</td> | ||
<td> | ||
`v` is in-scope if it is in-scope for the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This formulation is a bit weird to me because it's not defining the set of in-scope variable in the pattern like the other cases but the variable in scopes inside of any sub-pattern of the pattern.
I would rephase the sentence to something like v
is in-scope of all graph pattern contained in P
if v
is in-cope for a solution mapping where the expression containing EXISTS P
or NOT EXISTS P
is applied.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I find that weird as well. I would not include such a row in this table because the intended way to read this particular row would be very different from the way all the other rows are meant to be read.
`(expr AS v)` form. The scoping for `(expr AS v)` | ||
applies immediately in `SELECT` expressions. | ||
</p> | ||
<p>In `BIND (expr AS v)` requires that the variable `v` is not |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<p>In `BIND (expr AS v)` requires that the variable `v` is not | |
<p>`BIND (expr AS v)` requires that the variable `v` is not |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
or
<p>In `BIND (expr AS v)` requires that the variable `v` is not | |
<p>In `BIND (expr AS v)`, the variable `v` is required to not be |
Keywords are matched in a case-insensitive manner with the exception of | ||
the keyword '`a`' which, in line with Turtle and N3, is used | ||
in place of the IRI `rdf:type` (in full, | ||
<code><a href="http://www.w3.org/1999/02/22-rdf-syntax-ns#type">http://www.w3.org/1999/02/22-rdf-syntax-ns#type</a></code>). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The boolean sugars -- true
and false
-- are also case sensitive.
(I hate that these exist. We're well and truly stuck with a
, which I hate almost as much, but I truly wish that true
and false
were treated just like any other untyped literal -- and just like any other capitalization of those words would be. Talk about a foot-gun...)
This PR extracts proposed scoping changes for EXISTS/NOT EXISTS.
The only content changes are in the grammar notes (9 and new note 15), and in a new entry in the table in 18.3.1 Variable Scope. This can be seen in the diff. The rest is HTML changes that make no appearance changes.
Preview | Diff