w3c / data-shapes

RDF Data Shapes WG repo
89 stars 33 forks source link

proposed test pre-binding-006 has wrong expected result #84

Closed pfps closed 7 years ago

pfps commented 7 years ago

It does SELECT * and thus returns only the variables that are in-scope (visible) in the pattern. The only variable that this covers is this, i.e., not the other pre-bound variables.

This is not a matter of intention. This is a matter of the definition of SPARQL.

peter

On 05/12/2017 04:19 PM, Holger Knublauch wrote:

It does SELECT * and thus returns all pre-bound variables. If in doubt, the test case has the intended semantics.

SELECT $this
WHERE {
    {
        SELECT *
        WHERE {
            FILTER ($this = ex:InvalidResource) .
        }
    }
}

Holger

On 13/05/2017 3:44, Peter F. Patel-Schneider wrote:

The proposed test pre-binding-006 has a subquery that does not return all potentially pre-bound variables, which means that the query does not meet the requirements for pre-binding in SHACL and thus that SHACL-SPARQL implementations must report a failure for the shape.

However, the expected result of the test is not a failure.

Peter F. Patel-Schneider Nuance Communications

HolgerKnublauch commented 7 years ago

Please enumerate the other variables that should be returned. I believe only $this is pre-bound in that query anyway.

pfps commented 7 years ago

From the SHACL document:

Note that the term potentially pre-bound variables includes the variables this, shapesGraph, currentShape, value (for ASK queries), and any variables that represent the parameters of the constraint component that uses the query.

So the potentially pre-bound variables here are this, shapesGraph, and currentShape.

The sub-query only returns this.

As the sub-query does not return all potentially pre-bound variables, the query is not meet the restrictions on SPARQL queries required for the definition of pre-binding in SHACL.

HolgerKnublauch commented 7 years ago

I was hoping to confirm with Andy, but he is traveling this week. My understanding of this is that SELECT * would also apply to the pre-bound variables. The definition in SPARQL 1.1 relies on the term in-scope (https://www.w3.org/TR/sparql11-query/#variableScope) which states that "... in-scope if there is a way for a variable to be in the domain of a solution mapping at that point in the execution of the SPARQL algebra..." i.e. it operates on the Algebra. With pre-binding the Algebra is modified to include the Table operator (http://w3c.github.io/data-shapes/shacl/#pre-binding) which does contain the pre-bound variables.

Even if that were not the case, support for $shapesGraph and $currentShape is optional, and we could clarify that these two variables do not need to be returned by all sub-SELECTs either. It would't have impact on any user, and is just a formality on how we have defined the syntax rule.

pfps commented 7 years ago

SELECT * is handled during the translation to the SPARQL algebra, see 18.2.4.4 SELECT Expressions. So it is handled before the modifications done by pre-binding.

HolgerKnublauch commented 7 years ago

Ok, this looks plausible. We had already stated in 5.3.1 that $shapesGraph and $currentShape are optional, and it may therefore be reasonable to clarify that they are optional in this syntax rule, too. Otherwise it would even trigger the failure rule in 5.3.1 for some implementations. Do you see any problems with changing the syntax rule sentence to

Subqueries must return all potentially pre-bound variables, except shapesGraph and currentShape which are optional

to clarify this fact? Looks like a minor editorial change to me.

pfps commented 7 years ago

Optional syntax? How would that work? The valid syntax of SHACL depends on the SHACL implementation? I don't see how that can be.

In any case, any change to the syntax of SHACL is definitely a non-editorial change.

HolgerKnublauch commented 7 years ago

We already said in 5.3.1 that these are optional, so of course this also applies to this particular rule. This was always the intention. It would otherwise be a contradiction to the failure rule in 5.3.1. Thus this is only an editorial clarification (repetition of what's already said). I'll get that topic discussed with the WG. Like with all the formal objections that you have raised, the W3C management will decide, not you.

pfps commented 7 years ago

This change the the SHACL language makes it again possible that a pre-bound shapesGraph or currentShape is not available when needed.

For example the following query will return no solutions:

SELECT $this WHERE { SELECT $this WHERE { } FILTER ( bound ( $shapesGraph ) ) } with this and shapesGraph pre-bound.

pfps commented 7 years ago

Even with the change this test remains illegal. There are no in-scope variables in the inner SELECT.

HolgerKnublauch commented 7 years ago

On your first comment, yes of course the variables may not be visible when needed, but that's the user's choice. Nobody is forced to use sub-selects in such a query, and they can easily just return $shapesGraph from the query if they want to.

On your second comment, you seem to be contradicting yourself. Six days ago you said that only ?this is in-scope. So what do you believe now, and why? (Andy is still traveling but I'll be interested in his input on this topic too). Meanwhile there would be no harm in changing the test case, if that's controversial.

pfps commented 7 years ago

Yes indeed, I was wrong before. Variables in filters are not in scope.