Skip to content
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

Silently ignore statements that cannot be deleted #573

Closed

Conversation

smessie
Copy link
Contributor

@smessie smessie commented Sep 2, 2022

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.

deletion of non-existing triples should have no effect
@TallTed
Copy link
Contributor

TallTed commented Sep 2, 2022

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
Copy link
Contributor

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
Copy link
Contributor Author

smessie commented Sep 3, 2022

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
Copy link
Contributor

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
Copy link
Contributor

bourgeoa commented Sep 3, 2022

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
Copy link
Contributor

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
Copy link
Contributor Author

smessie commented Sep 5, 2022

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.

@smessie smessie closed this Sep 5, 2022
@jeff-zucker
Copy link
Contributor

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
Copy link
Contributor Author

smessie commented Sep 5, 2022

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
Copy link
Contributor

Even better :-).

@bourgeoa
Copy link
Contributor

bourgeoa commented Sep 5, 2022

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 :

  • we are not discussing RDF representation but about QUERY, N3 query can act differently than SPARQL query or any other one. They are not RDF replacement ones
  • expecting to change SPARQL seems a no go

@jeff-zucker
Copy link
Contributor

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants