-
Notifications
You must be signed in to change notification settings - Fork 2
Correlated EXISTS #257
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?
Correlated EXISTS #257
Conversation
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.
Incomplete, but better than no review... Some clear fixes; some less clear questions.
@@ -8849,16 +8849,27 @@ <h4>Variable Scope</h4> | |||
<td><code>VALUES varlist { values }</code></td> | |||
<td><code>v</code> is in-scope if <code>v</code> is in <code>varlist</code></td> | |||
</tr> | |||
<tr> | |||
<td>`EXISTS` and `NOT EXISTS` filters</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>`EXISTS` and `NOT EXISTS` filters</td> | |
<td>`FILTERs` using `EXISTS` and `NOT EXISTS`</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.
First, to @TallTed's edit suggestion: since FILTERs
is not a keyword in SPARQL, I would not code-fence it. Additionally, for this rephrasing, it should be "or" instead of "and". So, better something like:
<td>`EXISTS` and `NOT EXISTS` filters</td> | |
<td>`FILTER` statements using `EXISTS` or `NOT EXISTS`</td> |
or:
<td>`EXISTS` and `NOT EXISTS` filters</td> | |
<td>`FILTER` containing `EXISTS` or `NOT EXISTS`</td> |
Now, more generally: Is this really only about filters? Shouldn't BIND
with EXISTS
or NOT EXISTS
be considered as well?
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.
In fact, I wonder why EXISTS
and NOT EXISTS
need to be mentioned at all here.
For every FILTER expr
, it holds that no variable is in-scope, no matter whether expr
contains EXISTS
/NOT EXISTS
or not. The notion of in-scope variables is about variables that may be in a solution mapping produced for a query construct and a FILTER
itself does not result in solution mappings. So, no variable can be in-scope for a FILTER
. Of course, there may be variables that are in-scope for a group pattern that contains a FILTER
, but that case is covered by the Group-related row above in this table.
<p>The variable <code>v</code> must not be in-scope at the point of the | ||
<code>(expr AS v)</code> form. The scoping for <code>(expr AS v)</code> | ||
applies immediately in <code>SELECT</code> expressions. | ||
</p> | ||
<p>In <code>BIND (expr AS v)</code> requires that the variable <code>v</code> 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 <code>BIND (expr AS v)</code> requires that the variable <code>v</code> is not | |
<p><code>BIND (expr AS v)</code> requires that the variable <code>v</code> is not |
<p>In <code>SELECT</code>, the variable <code>v</code> must not be in-scope in the graph | ||
pattern of the <code>SELECT</code> clause, nor used in another select expression earlier in |
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 appears to be non-normative text. I am therefore writing to replace the lowercase RFC2119 language.
<p>In <code>SELECT</code>, the variable <code>v</code> must not be in-scope in the graph | |
pattern of the <code>SELECT</code> clause, nor used in another select expression earlier in | |
<p>In <code>SELECT</code>, the variable <code>v</code> cannot be in-scope in the graph | |
pattern of the <code>SELECT</code> clause, nor have been used in another select expression earlier in |
<div class="ednote" id="note-BindingInScope"> | ||
<p> | ||
One way to provide `BindingInScope` is to change `eval` to also have the current row | ||
as an argument or a "null" token. . Normally this the "null" token. This would be set in |
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.
as an argument or a "null" token. . Normally this the "null" token. This would be set in | |
as an argument or a "null" token, and is normally the "null" token. This would be set in |
Another way is to have a global (to the execution) variable. In eval-exists, the | ||
old value is recorded, the global set to the new value, and reset on exit - this forms a | ||
stack for nested EXISTS. |
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.
It's not clear what is reset on exit
. I think, maybe, replace that phrase with the global reset to the old value on exit
, as suggested below. The hyphen should become either a full stop (as suggested below) or an em-dash (which I think doesn't work as well, here).
Another way is to have a global (to the execution) variable. In eval-exists, the | |
old value is recorded, the global set to the new value, and reset on exit - this forms a | |
stack for nested EXISTS. | |
Another way is to have a global (to the execution) variable. In eval-exists, the | |
old value is recorded, the global set to the new value, and the global reset to the old value on exit. This forms a | |
stack for nested `EXISTS`. |
<p> | ||
The outcome of `PrjMap` is independent of the order of replacement | ||
(e.g. bottom-up or top-down). | ||
Replacements may happen several times, depending on recursive order |
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.
Replacements may happen several times, depending on recursive order | |
Replacements may happen several times, depending on recursive order, |
The outcome of `PrjMap` is independent of the order of replacement | ||
(e.g. bottom-up or top-down). | ||
Replacements may happen several times, depending on recursive order | ||
but each time a replacement is made, the variable not used anywhere else. |
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 don't quite understand what's trying to be said here. It might just need the is
I've added, but a larger change may be needed.
but each time a replacement is made, the variable not used anywhere else. | |
but each time a replacement is made, the variable is not used anywhere else. |
<div class="note"> | ||
<p> | ||
A variable inside a project expression that is not in the variables projected | ||
is not affected by the values insertion operation because it is renamed apart. |
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.
is not affected by the values insertion operation because it is renamed apart. | |
is not affected by the value insertion operation because it is renamed separately. |
@@ -10502,6 +10623,12 @@ <h3>Evaluation Semantics</h3> | |||
</div> | |||
<div class="defn"> | |||
<p><b>Definition: <span id="defn_evalFilter">Evaluation of Filter</span></b></p> | |||
|
|||
<p class="ednote">@@ Make current μ available. Or use current language in Exist. | |||
Long term: change eval() to be arity three - 3rd argument is "current row". |
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.
Long term: change eval() to be arity three - 3rd argument is "current row". | |
Long term: change eval() to be arity three — 3rd argument is "current row". |
@@ -10994,52 +11125,95 @@ <h3>Grammar</h3> | |||
section 6 <a data-cite="xml11#sec-notation">Notation</a>.</p> | |||
<p>Notes:</p> | |||
<ol> |
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.
Order doesn't seem to be important in this <ol>
. If there is nothing that refers to these notes by number, then this <ol>
should be changed to an <ul>
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 numbering is essential so people can talk about the notes conveniently. It is a long list - saying the "8th bullet" is convenient. Counting manually is not convenient.
@afs do you already want feedback on this one? (I am asking because it still has WiP status in the GitHub UI and you didn't say anything in the description.) |
@hartig - I was going to bring it up at the next SPARQL TF. I use marking as "draft" to indicate its still changing so it's more like "FYI" in this state and not a request for feedback but feedback on the design is welcome. (here - the areas are marked On this PR, one area that might be of interest is whether "eval(pattern, D, G)" could be useful generalized to take a correlation |
Preview | Diff