SPARQL Update 1.1 review part1

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. 

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.

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?

4) "This document is related to these other specification documents:"
-->
"This document is related to the following other specification documents:"

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.

  
6) "Language forms are show as:
    [...] 
   [] indicates [...]"
-->
"Language forms are show for instance as follows:
    [...] 
  Here, [] indicates [...]"

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."

Note that this is actually still not 100% consistent, since you also use italics for new grammar elements specific to update.

8)
"Examples are shown as:"
-->
"Examples are shown as follows:"

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

in the bullet list following this sentence, use fixed width Italic font for the non-terminals from the grammar productions 
(TriplesBlock, ConstructTriples, GroupGraphPattern)

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.

Section 3:

12) 
"Like an RDF Dataset operated on by SPARQL"
-->
"Like an RDF Dataset operated on by the SPARQL1.1 Query Language [ref]"

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

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)

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

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?

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]"

18)
"[...] resulting in the statements not being affected."
-->
"[...] resulting in the statements not being affected of deletions."

19)
"If an inconsistency is detected, the store should raise an exception."

not clear to me what "raise an exception" means.

Section 4:

20) The last paragraph before section 4.1 should reference SPARQL1.1 Protocol

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"

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."

22)
for both DELETE DATA and INTERST DATA an example of insertion/deletion on a named graph would be good.

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)
 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.

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... 

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.

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."

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.

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.

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"
 
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.
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)

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."

32)
section 4.1.6 ... see above, should be merged into 4.1.4

33) Section 4.1.8 CLEAR

I think "ALL NAMED" would be clearer than "NAMED" 

34)
"but is a clearer expression of emptying a graph."

I think this should be dropped.

35) 
Why don't we have SILENT for LOAD?

36) Section 4.2.2
"This DROP operation"
-->
"The DROP operation"

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


a detailed review of Section 5 will follow in part 2



Axel

Received on Thursday, 30 September 2010 22:10:50 UTC