w3c / data-shapes

RDF Data Shapes WG repo
89 stars 33 forks source link

Potential problem with tests/sparql/pre-binding/unsupported-sparql-006.ttl #101

Closed ashleysommer closed 6 years ago

ashleysommer commented 6 years ago

Test in question: https://github.com/w3c/data-shapes/blob/gh-pages/data-shapes-test-suite/tests/sparql/pre-binding/unsupported-sparql-006.ttl

I'm currently trying to get all of the unsupported-sparql-xxx tests to pass in pySHACL.

This test expects the SHACL implementation to throw a Validation Failure due to the use of BIND (true AS ?value) pre-bound variable reassignment in the ASK Validator SPARQL query.

However in our pySHACL implementation, this test is generating a different error, due to the rule that validators of type SPARQLAskValidator cannot be used on SHACL PropertyShapes.

Some facts I've pulled from the ttl file:

In the Spec:

Section 6.2.3 Validator rules: Rule 2: For property shapes, use one of the values of sh:propertyValidator, if present.

Section 6.2.3.1 SELECT-based Validators The values of sh:propertyValidator must be SELECT-based validators

Based on those rules, the pySHACL implementation is throwing an internal ConstrainLoad error, rather than a formal SHACL Validation Failure. Is that correct? Is the test wrong? If not, what are we doing wrong?

Thanks

HolgerKnublauch commented 6 years ago

Thanks for your report, Ashley. This test case does indeed have a glitch. The issue is that the use of an ASK validator as value of sh:propertyValidator is unsupported by SHACL, which technically means that the shapes graph is invalid which means that the result of validation is undefined. Therefore, the test case cannot assume that a Failure gets reported.

While I am sure there is a formal process to update these tests, I went ahead and fixed it on github: https://github.com/w3c/data-shapes/commit/a5582ac559bfa0c24fb03e651ec880fe966f6444 switching to sh:validator.

I have verified that the TopBraid test result from the report is still accurate (it still produces the failure).

Could you kindly double-check whether you agree that this fixes the issue with the test? I would then announce this to the SHACL CG so that those who have reported test results on this in the past can verify that they still produce the same results.

ashleysommer commented 6 years ago

Hello Holger, Yes, that change fixes the problem with that test case, and that test case is now passing for me. Go ahead and announce the change.

HolgerKnublauch commented 6 years ago

Thanks, I have announced this and other recent changes: https://lists.w3.org/Archives/Public/public-shacl/2018Sep/0013.html