specification icon indicating copy to clipboard operation
specification copied to clipboard

Drafting SPARQL Update support

Open kjetilk opened this issue 3 years ago • 15 comments

Here is a first pass over #125 with a suggested BNF and some text.

I'll mark it WiP, because I have a feeling we should be able to simplify it further especially around GroupGraphPattern, which shouldn't need that, it seems.

kjetilk avatar Oct 11 '21 23:10 kjetilk

As an open question (that is perhaps answered somewhere else): is BNF the most effective way to capture a subset of SPARQL? Both now and moving forward?

Because I expect that we might have multiple subsets in the future, and the BNF might become repetitive (and in essence already might be compared to the SPARQL spec itself).

RubenVerborgh avatar Oct 12 '21 11:10 RubenVerborgh

As an open question (that is perhaps answered somewhere else): is BNF the most effective way to capture a subset of SPARQL? Both now and moving forward?

Because I expect that we might have multiple subsets in the future, and the BNF might become repetitive (and in essence already might be compared to the SPARQL spec itself).

Right. I was quite confident about using a BNF grammar to define the subset as it is very concise and would be sure to capture the same semantics and thus make reuse of the algebra defined in the spec. For that reason, I honestly didn't think of a different way to do it as every other way would need to be more verbose.

Unfortunately, the grammar isn't as well defined as I hoped, in particular, it doesn't actually define INSERT DATA and DELETE DATA without patterns, like other parts of the spec does. So, I am certainly open to other ideas in this area, but perhaps not in the immediate term?

kjetilk avatar Oct 12 '21 14:10 kjetilk

Should we already mention something about atomicity?

This text precludes one way to provide atomic updates over multiple Resources (e.g. to keep them in a mutually-consistent state):

except that servers MUST NOT allow a request with a PATCH method to change other resources than the target resource. [Source]

@RubenVerborgh , is that what you meant by "atomicity"?

(Precluding could be a good thing 'cause it leaves us room to spec it later.)

ericprud avatar Oct 12 '21 20:10 ericprud

FWIW, HTTP, LDP specs use ABNF. wac-allow, Accept-Put is also in ABNF.

csarven avatar Oct 12 '21 20:10 csarven

The particular subset looks good to me; I'm just not sure whether BNF is the most sustainable way to capture this (see other comment).

Should we already mention something about atomicity? Note that NSS implements a subset of SPARQL 1.1 with a different semantics, namely it requires exactly one match for the WHERE clause.

I need to re-review the NSS code on this point. I am aware of the willful violation of SPARQL when it comes to the semaphore mechanism, but I thought the matching was connected to that.

kjetilk avatar Oct 12 '21 21:10 kjetilk

except that servers MUST NOT allow a request with a PATCH method to change other resources than the target resource. [Source]

@RubenVerborgh , is that what you meant by "atomicity"?

No; I'm talking about semaphore behavior within a single document (so perhaps I used the wrong term). The relevant issues are https://github.com/solid/specification/issues/139 and https://github.com/solid/solid-spec/pull/193

RubenVerborgh avatar Oct 13 '21 07:10 RubenVerborgh

@RubenVerborgh , is that what you meant by "atomicity"?

No; I'm talking about semaphore behavior within a single document (so perhaps I used the wrong term). The relevant issues are #139 and solid/solid-spec#193

Right, among the ACID properties, it is really isolation that we're talking about there. It is a mechanism that is supposed to be helping servers do the right thing when they can't make sure each query doesn't run in isolation.

And yes, I think it is within our immediate scope to specify that behavior even though I see large problems with. The main issue for it was https://github.com/solid-archive/query-panel/issues/3 .

In the short term, my main concern isn't so much that we have that behavior, it is that if we need to change the semantics of SPARQL, it is likely that you cannot just put in a SPARQL engine if you have one and so, it creates a tension between those that implement the minimal subset and those who have a SPARQL engine. I think it is important to avoid that.

kjetilk avatar Oct 13 '21 09:10 kjetilk

From reading the NSS source code, I have concluded that what it does is this:

When the request body of a PATCH request has a SPARQL Update query that contains an INSERT keyword, the server must treat the request as an Append operation. When the query contains a WHERE keyword, the server must treat the request as a Read operation. When the query contains a DELETE keyword, the server must treat the request as a Read and Write operation.

So, that's what I'll be putting in the spec shortly. Even if it is a pretty blunt way of doing it.

kjetilk avatar Oct 13 '21 13:10 kjetilk

I'm see no reason for the NSS approach should become the spec—no one really ever signed off on this.

But also, the above sounds like the WAC part of it? Which should not be this spec?

RubenVerborgh avatar Oct 13 '21 13:10 RubenVerborgh

I'm see no reason for the NSS approach should become the spec—no one really ever signed off on this.

Yes, the Solid Editors, on the request of @timbl that we would first produce 0.9 specs, which documents NSS and also to a great extent CSS behaviour: https://hackmd.io/ek6PILIyStm6b7IN-cbn3g That's also the reason why the current milestone is named as such.

But also, the above sounds like the WAC part of it? Which should not be this spec?

No, that's why I define this in terms of operations, not in terms of access modes. Thinking in terms of operations needs to be a common pattern between all authz systems. We're not quite there yet, but it is the direction that has been discussed.

kjetilk avatar Oct 13 '21 21:10 kjetilk

I added another commit with the above with links and all. That would close #220 too.

kjetilk avatar Oct 14 '21 00:10 kjetilk

I opened #322 to understand the semaphore mechanism in further depth.

kjetilk avatar Oct 14 '21 23:10 kjetilk

I have now made three PRs to this branch that can be reviewed more superficially for inclusion into this PR.

They are

  • #321 which I think simplifies the BNF and makes it clearer.
  • #323 which considers the other SPARQL Update operations. It is less critical to get that in, but I think it is OK to have.
  • #324 A wording around the semaphore mechanism that I think, based on what I wrote in #322, will legitimize the current behavior but with a very minor change to SPARQL.

I hope you will all have a look at this. If we can get #321 and #324 merged into this branch, I think this PR can go from draft to actual review. I'd like that to happen ASAP.

kjetilk avatar Oct 15 '21 12:10 kjetilk

why containing a DELETE implies READ.

Because the semaphore mechanism #322 currently lets DELETE requests only succeed if there was exactly one match, giving away the existence of certain triples.

Which makes me wonder… is the semaphore mechanism the only reason? If so, Read should probably be dropped until we agree on the semaphore.

RubenVerborgh avatar Oct 18 '21 16:10 RubenVerborgh

Which makes me wonder… is the semaphore mechanism the only reason? If so, Read should probably be dropped until we agree on the semaphore.

Yes, it is the only reason, AFAICT. Standard SPARQL doesn't have this problem since it always succeeds. But the semaphore is required for 0.9, so we might as well have it.

kjetilk avatar Oct 18 '21 21:10 kjetilk