Re: SPARQL Update 1.1 review part1

as for how you addressed my original comments, nothing critical, just some remarks to keep in mind when going to LC.
As for checking the formal model part and giving the final ok, I hope I can send my comments later on today, need to run for a talk now.

> > 3) "For the rationale behind this language, see the original SPARQL New Features and Rationale document."
> >
> > This doc is not rec track, I don't think we should reference it in a rec track doc. opinions?
> 
> That's just a reference, as we do reference other non-REC not non-W3C documents.
> However, it may be more suited in the overview document that on the Update one.
> I'd suggest to keep it there and remove when the overview document is there

fine, we can do that in the next iteration.

> > 5) "SPARQL 1.1 Update is not:
> >
> >    * A replacement for RSS or Atom, which
> >        may be used to advertise changes."
> >
> > RSS and Atom should both be included in the informative references of the document.
> 
> Actually, I'm not sure that sentence is required (esp. as they are now more things to advertise changes in RDF graphs, so the list is incomplete).
> If no objects, I'll remove it.


you mean removing the whole subsection... I wouldn't have objections against this.


> > 13)
> > "A Graph Store need not be authoritative for the graphs it contains."
> >
> > *authoritative* is not defined anywhere. Either define it, or drop that sentence all-together
> 
> Rephrased to:
> 
> "A Graph Store needs not be authoritative for the graphs it contains, i.e. there can be local copies of RDF graphs defined elsewhere on the Web.
> (I cannot see how to define authoritative more clearly, imo it speaks for itself but suggestion is welcome if not clear enough)

hmmm, ok for now... maybe that'd be even clearer:

"A Graph Store needs not be authoritative for the graphs it contains, i.e. the graph URIs do not need to be in the same pay level 
domain as the endpoint; that means a graph store can keep local copies of RDF graphs defined elsewhere on the Web and modify those 
copies independently of the original graphs."

15)
> Done - I rephrased the paragraph to
> 
> "In the case of two different update services, whose respective graph stores contain graphs with the same names, there is no presumption that the updates done through one service will be propagated to the other, as the store are independant entities.
> The behaviour of these services with respect to each other (such as automatic syncronisation after updates) is implementation dependent."


minor typo correction (store -> stores):

"In the case of two different update services, whose respective graph stores contain graphs with the same names, there is no presumption that the updates done through one service will be propagated to the other, as the stores are independant entities.
The behaviour of such services with respect to each other (such as automatic syncronisation after updates) is implementation dependent."

> > 23)
> > on DELETE DATA, I have two questions:
> > a) it should be mentioned what happens if I try to delete triples not existent in that graph (I guess nothing, and still return with success, just as for DELETE, right? but I think we also have to mention it for DELETE DATA)
> 
> Already there:
> 
> (1) in the DELETE/INSERT section "Deleting triples that are not present, or from a graph that is not present will have no effect and will result in success. "
> (2) in the DELETE one "The DELETE operation is similar to the DELETE/INSERT operation without an INSERT section."
> 
> You think it needs emphasis ?


I think it needs to be made clear in the DELETE DATA section as well.


> > b) it should be mentions what happens if graph_triples contains bnodes.
> >
> > concerning b), I am not sure whether we have discussed this in particular in the context of DELETE DATA, but
> > please note resolution  http://www.w3.org/2009/sparql/meeting/2010-03-09#resolution_2
> > I haven't found any later resolutions on this, but I am not sure whether we intended this behaviour on DELETE DATA.
> > since actually it means that one can encode a query deletion into DELETE DATA, by using bnodes:
> >
> >   e.g. if we follow this resolution here, then
> >       DELETE DATA { [] :p [] }
> >   has the same behaviour as:
> >       DELETE WHERE { ?S :p ?O }
> >
> > if I see it correctly. If the editors think the same, than I am afraid we have to reopen the issue of bnodes in DELETE clauses, IMO.
> > (actually, we didn't really ever have an ISSUE on that open, as far as I can see it now.
> >
> 
> @@TODO - will check in more details later w/ Paul

should be marked at least in the document with a TODO.


> > 24) Can someone help me when the USING syntax was agreed? It seems that, since we no longer have DELETE FROM/INTERST INTO (since we now use GRAPH patterns to indicate the affected graphs), there is no need anymore to avoid FROM and FROM NAMED, so I strongly suggest to switch back to FROM/FROM NAMED also in update queries.
> >
> > IMHO, USING just creates more confusion than it solves ... another new keyword, and not easy to explain to anyone why FROM and FROM NAMED don't work here the same way that they work in Query...
> 
> IIRC, decision was let to editors and Paul made the change.
> Is that a major issue for you, Axel ?

Frankly, I don't like it, since as we removed DELETE FROM, it seems unnecessary to introduce a new keyword.
would be good to mark with an editor's note:

 "The group is currently discussing whether own keywords USING/USING NAMED are necessary, or whether FROM/FROM NAMED can just be used analogously to SPARQL1.1 Query" 

> 25) - 27)

should be marked at least in the document with TODOs, if still open?


> > 28)
> > "If no data is to be inserted, then no graph will be created, even if another dataset would result in data being inserted."
> >
> > Why do we need that? A store may decide to drop empty graphs anyways, so this seems to be redundant and introduce an extra burden for
> > graph aware stores that they have to check whether the insert has any results before creating the graph.
> 
> I don't see the issue here.
> Why would stores check something ?

What I am saying is that this sentence can safely be dropped. I think it is ok to insert an empty graph in that case, or 
to drop that empty graph immediately. I.e., what you write here is more restritive then what is said in section 3.1, I 
would rather simply reformulate this to:

"If no data is to be inserted, an empty graph MAY be created, see section 3.1 Graph update."


> > 29)
> > "If a graph must be created regardless of the data to be inserted"
> > -->
> > "If the user intends to create a graph regardless of the data to be inserted"
> 
> Used "If an operation intends to create a graph regardless of the data to be inserted"

note that if you do the above suggested reformulation, this one is void and should be removed.


30) 

we can resort this in the next version.

> > 31)
> > "Refer also to the final INSERT example, which demonstrates multiple operations, including a DELETE."
> >
> > is this a TODO or what should this sentence tell the reader?
> > maybe better:
> >
> > "For another example involving DELETE, we refer the reader also to the final example in the following section, which demonstrates multiple operations, combining an INSERT with a DELETE."
> 
> Done - rephrased to:
> 
> "Another example of <code>DELETE</code>, is provided in the <a href="#example_h">final example</a> in the following section, which demonstrates multiple operations, combining an <code>INSERT</code> with a <code>DELETE</code>."


too many commas here, I think:

"Another example of <code>DELETE</code> is provided in the <a href="#example_h">final example</a> in the following section 
 which demonstrates multiple operations combining an <code>INSERT</code> with a <code>DELETE</code>."


> > 33) Section 4.1.8 CLEAR
> >
> > I think "ALL NAMED" would be clearer than "NAMED"
> 
> I'd rather keep NAMED, and I think that's explicit enough (also defined in the formal model).

hmmm, I find ALL NAMED really more intuitive, to be honest... shall we put that up for discussion in a separate mail?


37) see 33) ...

I think non of these prevent us from publishing, i.e. if you can address them or add editor's notes, great, but not critical as long as we don't forget them... but would you mind putting all remaining issues we couldn't agree upon or which are left as TODO up for discussion in a separate email to the group?

thanks,
Axel


On 3 Oct 2010, at 06:06, Passant, Alexandre wrote:

> Hi,
> 
> Document was not yet ready for review, but I incorporated most of your change, see below (a few TODOs left).
> Will send the request for review soon, just going through it one last time.
> 
> Alex.
> 
> On 30 Sep 2010, at 23:10, Axel Polleres wrote:
> 
> > Hi all,
> >
> > sorry for the late review due to traveling/non-connectivity... (also working on remaining actions such as update test vocabulary, etc.)
> >
> > Now I realise that my print version I had for review is based on an old version and will only list those comments still valid.
> > I will check on the new parts in a part 2 of this review.
> >
> > ==================
> >
> >
> > Section 1:
> >
> > 1)  We use both terms "RDF Store" and "Graph Store" in the document, but only the latter is formally defined. I'd recommend to use "RDF Store" uniformly (or Graph Store, since it seems to be in use quite long already, but I like "RDF store" more) unless those terms are menat to be really different. If so, we need to define what we mean by RDF Store.
> >
> > Also, I'd suggest to *always* link to the definition, when the terms *graph store* or *rdf store* are used.
> 
> Done - replaced uniformly to Graph store
> 
> >
> > 2)  "The reuse of the SPARQL syntax, in both
> >      style and detail, reduces the learning curve for developers and
> >      reduces implementation costs."
> >
> >   I think this sentence doesn't add anything and recommend to delete it.
> >
> 
> Done
> 
> > 3) "For the rationale behind this language, see the original SPARQL New Features and Rationale document."
> >
> > This doc is not rec track, I don't think we should reference it in a rec track doc. opinions?
> 
> That's just a reference, as we do reference other non-REC not non-W3C documents.
> However, it may be more suited in the overview document that on the Update one.
> I'd suggest to keep it there and remove when the overview document is there
> 
> >
> > 4) "This document is related to these other specification documents:"
> > -->
> > "This document is related to the following other specification documents:"
> >
> 
> Done
> 
> > 5) "SPARQL 1.1 Update is not:
> >
> >    * A replacement for RSS or Atom, which
> >        may be used to advertise changes."
> >
> > RSS and Atom should both be included in the informative references of the document.
> 
> Actually, I'm not sure that sentence is required (esp. as they are now more things to advertise changes in RDF graphs, so the list is incomplete).
> If no objects, I'll remove it.
> 
> >
> >
> > 6) "Language forms are show as:
> >    [...]
> >   [] indicates [...]"
> > -->
> > "Language forms are show for instance as follows:
> >    [...]
> >  Here, [] indicates [...]"
> 
> Done
> 
> >
> > 7)
> > "Italics indicate an item is some syntax element derived from SPARQL Query. BOLD indicates literal text."
> > -->
> > "Italics indicate syntactic items derived from the SPARQL Query grammar. BOLD indicates language keywords."
> 
> Done
> 
> >
> > Note that this is actually still not 100% consistent, since you also use italics for new grammar elements specific to update.
> 
> Done
> 
> >
> > 8)
> > "Examples are shown as:"
> > -->
> > "Examples are shown as follows:"
> 
> Done
> 
> >
> > 10)
> > In section 1.2.2
> >
> > "The following terms are also used in this document, and are defined in the SPARQL 1.1 Query Language"
> > -->
> > The following terms are also used in this document as defined in the SPARQL 1.1 Query Language
> 
> Done
> 
> >
> > in the bullet list following this sentence, use fixed width Italic font for the non-terminals from the grammar productions
> > (TriplesBlock, ConstructTriples, GroupGraphPattern)
> 
> Done
> 
> >
> > Section 2:
> >
> > 11) Section 2.1
> >
> > http://www.w3.org/2009/sparql/track/issues/37 is closed.
> >
> > for Alex (re: his last mail), as far as I remember, we simply resolved this a non-issue, since the semantics should be clear.
> > Check http://www.w3.org/2009/sparql/meeting/2010-08-03#resolution_4
> > Paul also agreed that it shouldn't have any problems, but wanted to add some explaining words.
> > I assume that the only issue would be when the SERVICE queried in an update operation is the same as the
> > service updated, but even that seems to be a non-issue, since this is just the standard behavior.
> > As I see it in the notes, Steve mentioned "feedback effects" in the meeting, maybe he can clarify?
> >
> > In total, I'd hope Section 2 can simply be dropped entirely.
> 
> Done - ISSUE-37 closed, and section dropped + see last e-mail on the topic for clarification
> 
> >
> > Section 3:
> >
> > 12)
> > "Like an RDF Dataset operated on by SPARQL"
> > -->
> > "Like an RDF Dataset operated on by the SPARQL1.1 Query Language [ref]"
> 
> Done
> 
> >
> > 13)
> > "A Graph Store need not be authoritative for the graphs it contains."
> >
> > *authoritative* is not defined anywhere. Either define it, or drop that sentence all-together
> 
> Rephrased to:
> 
> "A Graph Store needs not be authoritative for the graphs it contains, i.e. there can be local copies of RDF graphs defined elsewhere on the Web.
> (I cannot see how to define authoritative more clearly, imo it speaks for itself but suggestion is welcome if not clear enough)
> 
> >
> > 14)
> > "A formal definition for graph stores and how SPARQL 1.1 Update affects them is described in the SPARQL 1.1 Update Definition section."
> > -->
> > "A formal definition for graph stores and how SPARQL 1.1 Update affects them is described in the SPARQL 1.1 Update Formal Model section."
> > (and fix the anchor, which doesn't work at the moment)
> 
> Done
> 
> >
> > 15)
> > "[...]It is recommended that such deployment scenarios are avoided."
> > The whole paragraph would benefit from  clearer wording, but that last sentence is totally unclear to me. What now is to be avoided exactly?
> > This needs clarification, IMO
> 
> Done - I rephrased the paragraph to
> 
> "In the case of two different update services, whose respective graph stores contain graphs with the same names, there is no presumption that the updates done through one service will be propagated to the other, as the store are independant entities.
> The behaviour of these services with respect to each other (such as automatic syncronisation after updates) is implementation dependent."
> 
> However, I cannot see why it should be avoided, so I dropped that last sentence.
> 
> >
> > 16)
> > "An implementation must target SPARQL queries on updated graphs if the SPARQL and
> > SPARQL 1.1 Update end points are the same."
> >
> > also not clear to me what that should say. esepacially what "must target" means here?
> > plus, should the *must* be bold here, i.e. is the RFC sense meant here?
> 
> It means that if you've got a single endpoint for query and update, queries should be ran on the updated graphs.
> But that's imo obvious, I removed it.
> 
> >
> > 17)
> > "If the store is capable of calculating entailed statements"
> > -->
> > "If the store is capable of inferring entailed statements, cf. SPARQL1.1 Entailment Regimes [ref]"
> >
> 
> Done
> 
> > 18)
> > "[...] resulting in the statements not being affected."
> > -->
> > "[...] resulting in the statements not being affected of deletions."
> >
> 
> Done
> 
> > 19)
> > "If an inconsistency is detected, the store should raise an exception."
> >
> > not clear to me what "raise an exception" means.
> 
> Done
> Based on text from the entailment doc, I rephrased to
> "may generate an error or warning" (also note should -> may)
> 
> >
> > Section 4:
> >
> > 20) The last paragraph before section 4.1 should reference SPARQL1.1 Protocol
> 
> Done - rephrased
> 
> "This document does not stipulate the exact form of the result, as that will be dependent on the interface being used, for instance the HTTP or a programatic API"
> 
> =>
> 
> "This document does not stipulate the exact form of the result, as that will be dependent on the interface being used, for instance the SPARQL 1.1 protocol via HTTP or a programatic API"
> 
> >
> > 21)
> > "The INSERT DATA operation adds some triples, which are inline in the
> > request, into a graph"
> > -->
> > "The INSERT DATA operation adds some triples, given inline in the
> > request, into a graph"
> >
> 
> Done
> 
> > likewise:
> >
> > "The DELETE DATA operation removes some triples, which are inline in
> > the request."
> > -->
> > "The DELETE DATA operation removes some triples, given inline in
> > the request, if the respective graph contains those."
> 
> Done
> 
> >
> > 22)
> > for both DELETE DATA and INTERST DATA an example of insertion/deletion on a named graph would be good.
> 
> There is one covering both in the DELETE DATA section.
> I've just added a link to it from the INSERT DATA section to make it more apparent.
> (note also that I included graphs before / after for all examples)
> 
> >
> > 23)
> > on DELETE DATA, I have two questions:
> > a) it should be mentioned what happens if I try to delete triples not existent in that graph (I guess nothing, and still return with success, just as for DELETE, right? but I think we also have to mention it for DELETE DATA)
> 
> Already there:
> 
> (1) in the DELETE/INSERT section "Deleting triples that are not present, or from a graph that is not present will have no effect and will result in success. "
> (2) in the DELETE one "The DELETE operation is similar to the DELETE/INSERT operation without an INSERT section."
> 
> You think it needs emphasis ?
> 
> > b) it should be mentions what happens if graph_triples contains bnodes.
> >
> > concerning b), I am not sure whether we have discussed this in particular in the context of DELETE DATA, but
> > please note resolution  http://www.w3.org/2009/sparql/meeting/2010-03-09#resolution_2
> > I haven't found any later resolutions on this, but I am not sure whether we intended this behaviour on DELETE DATA.
> > since actually it means that one can encode a query deletion into DELETE DATA, by using bnodes:
> >
> >   e.g. if we follow this resolution here, then
> >       DELETE DATA { [] :p [] }
> >   has the same behaviour as:
> >       DELETE WHERE { ?S :p ?O }
> >
> > if I see it correctly. If the editors think the same, than I am afraid we have to reopen the issue of bnodes in DELETE clauses, IMO.
> > (actually, we didn't really ever have an ISSUE on that open, as far as I can see it now.
> >
> 
> @@TODO - will check in more details later w/ Paul
> 
> > 24) Can someone help me when the USING syntax was agreed? It seems that, since we no longer have DELETE FROM/INTERST INTO (since we now use GRAPH patterns to indicate the affected graphs), there is no need anymore to avoid FROM and FROM NAMED, so I strongly suggest to switch back to FROM/FROM NAMED also in update queries.
> >
> > IMHO, USING just creates more confusion than it solves ... another new keyword, and not easy to explain to anyone why FROM and FROM NAMED don't work here the same way that they work in Query...
> 
> IIRC, decision was let to editors and Paul made the change.
> Is that a major issue for you, Axel ?
> 
> >
> > 25)
> > "Any remaining portions of the GroupGraphPattern which are not assigned a dataset will be matched against the graph specified in the WITH clause, if present, or the default graph otherwise."
> > -->
> > "Any remaining portions of the GroupGraphPattern which are not assigned a dataset will be matched against the graph specified in the WITH clause, if present, or the default graph of the graph store otherwise."
> >
> > ??? not sure here... actually, it seems that the default *dataset* to be used for the query part could be different from the *graph store*, or do we fix that to be the same (I think for update it would make sense, but I also remember we had some discussions around that...)
> > anyways, this needs clarification... also with respect to whether WITH also scopes the WHERE part.
> >
> > In cae I had missed any discussions on this, I apologies and would kindly ask the editors for the resp. pointers.
> 
> @@TODO - will check in more details later w/ Paul
> 
> >
> > 26)
> > similarly:
> > "if omitted, the INSERT or DELETE clauses applies to the graph specified by the
> > WITH clause, or the default graph if no WITH clause is present."
> > -->
> > "if omitted, the INSERT or DELETE clauses applies to the graph specified by the
> > WITH clause, or the default graph of the graph store if no WITH clause is present."
> 
> @@TODO - will check in more details later w/ Paul
> 
> >
> > 27)
> > "Using a new blank node in a delete template will lead to nothing being deleted, as the new blank node cannot match anything that already exists."
> >
> > this seems to contradict resolution http://www.w3.org/2009/sparql/meeting/2010-03-09#resolution_2
> > I haven't seen any resolution overriding that, but I might have missed that. Even if we decided to override that resolution, it is not entirely clear to me what "new" blank node means exactly here.
> 
> @@TODO - will check in more details later w/ Paul
> 
> >
> > 28)
> > "If no data is to be inserted, then no graph will be created, even if another dataset would result in data being inserted."
> >
> > Why do we need that? A store may decide to drop empty graphs anyways, so this seems to be redundant and introduce an extra burden for
> > graph aware stores that they have to check whether the insert has any results before creating the graph.
> 
> I don't see the issue here.
> Why would stores check something ?
> 
> >
> > 29)
> > "If a graph must be created regardless of the data to be inserted"
> > -->
> > "If the user intends to create a graph regardless of the data to be inserted"
> 
> Used "If an operation intends to create a graph regardless of the data to be inserted"
> 
> >
> > 30) Section 4.1.4, Example 2
> >
> > "An example to remove all statements about anything with a first name of "Fred" from the graph http://example/addresses. No WHERE clause is present, meaning that the template also serves as the pattern to be matched."
> > -->
> > "An example to remove all statements about anything with a first name of "Fred" from the graph http://example/addresses. No DELETE template is present, meaning that the WHERE clause also serves as the template."
> >
> > BTW, this example is for DELETE WHERE and not for DELETE.
> 
> Done - I changed it to another DELETE example (w/ USING).
> I also fixed the text to fit with the next example
> 
> > I ask myself whether we really need separate DELETE and DELETE WHERE sections... it seems perfectly fine to
> > change the grammar in the beginning of section 4.1.4 as follows:
> >
> > [ WITH <uri> ]
> > DELETE { modify_template [ modify_template ]* }
> > [ USING [NAMED] <uri> ]*
> > WHERE GroupGraphPattern
> >
> > -->
> >
> > [ WITH <uri> ]
> > DELETE [{ modify_template [ modify_template ]* }]
> > [ USING [NAMED] <uri> ]*
> > WHERE GroupGraphPattern
> >
> > i.e. making the modify_template optional... and remove section 4.1.6 over all (maybe keeping some of the examples for section 4.1.4)
> >
> 
> In that case, you allow WITH <uri> DELETE WHERE { } ?
> and DELETE USING <> WHERE {} ?
> 
> I'm fine with that, but we may have to think of implications of this change.
> For instance, I cannot see how the 2nd will be used. (USING is useless here)
> 
> But merging would make sense, will think of it
> 
> > 31)
> > "Refer also to the final INSERT example, which demonstrates multiple operations, including a DELETE."
> >
> > is this a TODO or what should this sentence tell the reader?
> > maybe better:
> >
> > "For another example involving DELETE, we refer the reader also to the final example in the following section, which demonstrates multiple operations, combining an INSERT with a DELETE."
> 
> Done - rephrased to:
> 
> "Another example of <code>DELETE</code>, is provided in the <a href="#example_h">final example</a> in the following section, which demonstrates multiple operations, combining an <code>INSERT</code> with a <code>DELETE</code>."
> 
> >
> > 32)
> > section 4.1.6 ... see above, should be merged into 4.1.4
> >
> 
> Cf previous comment
> 
> > 33) Section 4.1.8 CLEAR
> >
> > I think "ALL NAMED" would be clearer than "NAMED"
> 
> I'd rather keep NAMED, and I think that's explicit enough (also defined in the formal model).
> 
> >
> > 34)
> > "but is a clearer expression of emptying a graph."
> >
> > I think this should be dropped.
> 
> Done
> 
> >
> > 35)
> > Why don't we have SILENT for LOAD?
> 
> Probably because we didn't specify return status of the LOAD clause.
> I've added:
> 
> "<p>In case no RDF data can be retrieved from <code>documentURI</code>, the SPARQL 1.1 Update service is expected to return failure. In any other case, it will always return success. If <code>SILENT</code> is present, the result of the operation will always be success.</p>"
> 
> + support for SILENT
> 
> In the previous sentence, should we mention GRDDL, etc. ? I don't think we already discussed that, but some store allow to LOAD XHTML+RDFa or GRDDL-ed data (4store through librdf IIRC ?)
> 
> 
> >
> > 36) Section 4.2.2
> > "This DROP operation"
> > -->
> > "The DROP operation"
> 
> Done
> 
> >
> > 37) in Section 4.2.2 as well,  a similar risk remark should be added here as in Section 4.1.8
> > particularly, I would be interested what DROP means if the default graph is the union of the named graphs.
> > what does DROP DEFAULT actually mean.
> >
> > Again NAMED should be replaced by ALL NAMED, i think this is clearer
> 
> Same answer as for 33)
> 
> >
> >
> > a detailed review of Section 5 will follow in part 2
> 
> Can you wait that I commit with the formal model ?
> 
> >
> >
> >
> > Axel
> >
> >
> >
> >
> >
> >
> 
> --
> Dr. Alexandre Passant
> Digital Enterprise Research Institute
> National University of Ireland, Galway
> :me owl:sameAs <http://apassant.net/alex> .
> 
> 
> 
> 
> 
> 
>