linkeddata / rdflib.js

Linked Data API for JavaScript
http://linkeddata.github.io/rdflib.js/doc/
Other
562 stars 142 forks source link

Silently ignore statements that cannot be deleted #573

Closed smessie closed 1 year ago

smessie commented 1 year ago

Currently, an error is returned when non-existing triples are being removed, however, according to the spec it should just have no effect.

For DELETE DATA

Note that the deletion of non-existing triples has no effect, i.e., triples in the QuadData that did not exist in the Graph Store are ignored.

And for DELETE

Analogous to DELETE/INSERT, deleting triples that are not present, or from a graph that is not present will have no effect and will result in success.

From https://www.w3.org/TR/sparql11-update/#deleteData

This PR thus changes the behavior so deleting non-existing triples does not have any effect.

TallTed commented 1 year ago

It is arguable, and has been in many arenas including Solid spaces, among people including @timbl, that specifying silent success for DELETE of nonexistent triples/quads was a specification error.

Certainly, there are many scenarios where the entity performing such an attempted DELETE wants to learn that the targeted triples/quads were not present — which may be evidence of other issues, including having targeted the wrong triple/quad-store with the DELETE, failure of some other tool that had been thought to have previously INSERTed those triples/quads, and others.

Before moving further on this PR, I suggest some extra research into why rdflib has the current behavior, as I think it was intentionally implemented this way, in known contradiction with the cited specifications.

jeff-zucker commented 1 year ago

The viewpoint Ted mentions is actually part of the existing Solid Specification in the section of Solid Protocol on N3 patch:

"If the set of triples resulting from ?deletions is non-empty and the dataset does not contain all of these triples, the server MUST respond with a 409 status code."

The spec mentions this discussion for reference - Source

smessie commented 1 year ago

Alright, so if I understand it right, the function is used both in the case of a SPARQL patch (in which case no 409 should be returned) and a N3 patch (in which case a 409 should be returned).

Then it might indeed be a better idea to leave it here in rdflib as it is and handle the (non) error at the place where it is used. E.g I came across this issue because NSS is throwing a 409 while doing a SPARQL patch, so then it should be fixed in NSS so that does not happen in case of a SPARQL patch, so it is conform the spec.

If we do not want a 409 in case of a SPARQL patch as well, then I think the spec should be updated.

jeff-zucker commented 1 year ago

Alright, so if I understand it right, the function is used both in the case of a SPARQL patch (in which case no 409 should be returned) and a N3 patch (in which case a 409 should be returned).

Well actually SPARQL patch is not in the Solid Specification and only remains in the system as a historical artifact. And it is completely unworkable to think we would have a situation in which patches did different things based on which patch we used. The decision on N3 clearly shows the intent of the Solid specification writers to support the idea that deletion of non-existing triples should error. If you disagree with that, you should be raising an issue on the Solid specifications (or better on the SPARQL specs), not proposing PRs for rdflib.

bourgeoa commented 1 year ago

Well actually SPARQL patch is not in the Solid Specification and only remains in the system as a historical artifact. And it is completely unworkable to think we would have a situation in which patches did different things based on which patch we used.

I do understand your point, but don't see why it is unworkable. Non spec compliant process can be what they want.

jeff-zucker commented 1 year ago

don't see why it is unworkable. Non spec compliant process can be what they want.

So you can imagine a server which fails on an a patch in the N3 format but succeeds on the same patch in SPARQL format?

smessie commented 1 year ago

Well actually SPARQL patch is not in the Solid Specification and only remains in the system as a historical artifact. And it is completely unworkable to think we would have a situation in which patches did different things based on which patch we used. The decision on N3 clearly shows the intent of the Solid specification writers to support the idea that deletion of non-existing triples should error. If you disagree with that, you should be raising an issue on the Solid specifications (or better on the SPARQL specs), not proposing PRs for rdflib.

Okay, I based this PR on the SPARQL specs, but not on the Solid specs it seems. The Solid specs do not mention anything about SPARQL update, and the SPARQL specs say that it should return success in this case. I think the behavior of NSS is thus still not spec compliant, but this repo is not the place to fix it since it is independent of the protocol used, being SPARQL or N3.

So you can imagine a server which fails on an a patch in the N3 format but succeeds on the same patch in SPARQL format?

It would make more sense to have the same behavior in both SPARQL and N3 format, but that's against the current Solid and SPARQL specs.

jeff-zucker commented 1 year ago

SPARQL is not against the Solid spec, just not covered in it. So, given that Solid has chosen to error on delete of non-existent triple the choice is, - break one rule in the SPARQL spec or break the entire foundation of RDF - that RDF in two different formats should behave the same. I think the choice is clear - break the SPARQL spec.

smessie commented 1 year ago

I think the choice is clear - break the SPARQL spec.

Or fix the SPARQL spec 😉 but yes I'm convinced that the problem is not in rdflib.

jeff-zucker commented 1 year ago

Even better :-).

bourgeoa commented 1 year ago

Or fix the SPARQL spec 😉 but yes I'm convinced that the problem is not in rdflib.

I have questions about the arguments that have been developed :

jeff-zucker commented 1 year ago

N3 query can act differently than SPARQL query or any other one

I am talking about how the server responds to a PATCH request. To say that it will respond one way to a request that is, in RDF, the same as another, seems to me to violate the basic principle of RDF.

expecting to change SPARQL seems a no go

Possibly related to the switch from SPARQL to N3?