Re: Update review

Hi,

I've gone through the update document and added some comments below. 

I've tried to split them into two categories, ' PRE FPWD' and 'POST 
FPWD'.  I think that the document will be suitable for publication once 
the comments marked 'PRE FPWD' are addressed (or dismissed).

There are quite a few points, but most are tiny non-normative changes 
relating to language.  I've tried to be as specific as possible to save 
time on any changes that need to be made, but please feel free to 
challenge any of my suggestions - some of them are quite pernickety.

Thanks,

Luke


PRE FPWD

General Points

    * Unwanted red HTML output is still there, although I know this will
      be fixed in the publication process.
    * Subheadings are inconsistent.  Issues headings aren't graded
      particularly clearly; it's hard to see where an issue begins and ends.

Abstract

    * 'It uses a syntax derived form SPARQL' should read 'It uses a
      syntax derived /from/ SPARQL'.

1.

    * '1.1 1.1 Scope and Limitations' should read '1.1 Scope and
      Limitations'

2.4.

    * In the phrase 'See the example here.', the word 'here' is linked
      to the wiki.  I think 'See the example here' should simply be removed.

3.

    * 'DELETE pattern' ought to read 'DELETE template'
    * Modify isn't used in these examples, but in section 5.1 it says:
      'The fundamental pattern-based operation of a graph update is
      MODIFY.'  Perhaps modify ought to be added         to the examples
      where appropriate.

4.

    * I wasn't sure what this sentence meant, it seems that something's
      been chopped out of the middle of it: 'Like an RDF Dataset
      operated on by SPARQL, a Graph Store one unnamed graph and zero or
      more named graphs'
    * I'm not sure it is clear what authoritative means in this
      sentence: 'A Graph Store needs not be authoritative for the graphs
      it contains.'.

4.1

    * The first paragraph is confusing - I don't think it is clear what
      it is trying to say.
    * I think that the note ought to be an issue, and perhaps the
      wording of 'it seems...' is too conversational for a FPWD.  Also,
      I'm not sure whether there is consensus that this actually is the
      position of the working group. (I originally added that issue to
      the UpdateIssues page and there wasn't any discussion on it then).

4.2

    * 'SPARQL/UPdate' ought to be 'SPARQL/Update'

5.1

    * Some of the 'Notes' in this section seem to be more like issues,
      but don't reference issue numbers.  E.g. 'Is DELETE too verbose?',
      'Is MODIFY syntax required?'
    * 'empting a graph.' should say 'emptying a graph.'
    * 'but on GraphGraphPatterns' - typo.  In the same line, 'The
      Working group also considers' perhaps ought to read 'The working
      group also considered'
    * It is my understanding that 'file://' URIs are allowed for LOADs. 
      So, LOAD <remoteURI> [ INTO <uri> ] ought to just be LOAD
      <fileToLoadURI> [INTO <uri>] or similar.


POST FPWD

General Points:

    * Examples seem out of context.  In my opinion they would be better
      located in section 5 next to the relevant feature ('DELETE',
      'INSERT' etc.), or elsewhere in the document where they are
      explaining a specific point, rather than lumped together at the
      front of the document.
    * Verbs such as SELECT, INSERT etc are shown in inconsistent fonts
      in the prose.
    * Sometimes issue names such as 'ISSUE-20' are linked and sometimes
      not and the issue number is repeated for each issue.
    * Examples ought to be in grey boxes like in the query language
      document.
    * 'graph store' and 'Graph Store'  are used inconsistently
      throughout the document.
    * endpoint and end point used inconsistently throughout document.

Abstract

    * 'Create a new RDF Graph to a Graph Store' perhaps ought to read
      'Create a new RDF Graph /in/ a Graph Store'
    * 'Operations are provided to change existing RDF graphs as well as
      create and remove graphs with the Graph Store.' perhaps ought to
      read 'Operations are provided to change existing RDF graphs as
      well as create and remove graphs /in/ the Graph Store.'

1.

    * 'Insert new triples to an RDF graph' perhaps would be better said
      as 'Insert new triples /into an/ RDF graph'

2.3

    * 'Basic federated update could allow to move triples between
      stores.' would maybe be better said as 'Basic federated update
      could allow the moving of triples between stores.'

3.

    * 'This section gives some example snippets in the proposed
      SPARQL-Update language that would be used to update a remote RDF
      store.' RDF store ought to be Graph Store as it is elsewhere in
      the document, but the document has already said the language works
      on Graph Stores, so maybe something shorter, like:  'This section
      gives some example snippets of the Update language '

4.

    * 'can be added or deleted to a graph store' would be better stated
      as 'can be added to or deleted from a graph store'
    * 'for update of a single graph.' perhaps should read 'for the
      update of a single graph'

4.2

    * Is it appropriate to put  ISSUE-22 in this document, or should it
      be in the http-update one?
    * 'If two different update services are managing their 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' would perhaps be better said as something
      like: '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.

5.1

    * I think it would be more accurate to say  'The INSERT operation is
      equivalent to the' rather than 'The INSERT operation is similar to
      the'.
    * In my opinion, a more wordy introductory paragraph, rather than a
      list of specific descriptions of the various verbs, would be
      better here.
    * The concept of an update service managing a graph store is used
      throughout section 4.  The category "Graph Management" here
      appears to have a subtly different meaning which could cause
      confusion.
    * 'The SPARQL-Update service generates an error if the graph
      referred by the URI already exists unless the keyword SILENT is
      present when no error is generated and execution of the sequence
      of SPARQL-Update operations continues' might read better as 'The
      SPARQL-Update service generates an error if the graph referred to
      by the URI already exists unless the keyword SILENT is present, in
      which case no error is generated and execution of the sequence of
      SPARQL-Update operations continues'
    * My colleague pointed out that having the query 'CLEAR' with no
      arguments just clear the default graph is likely to lead to
      accidents in practice - perhaps 'CLEAR DEFAULT' might be a better
      choice.  Also, this behaviour isn't explicitly mentioned in the
      prose of the document, maybe it ought to be.

Steve Harris wrote:
> Excellent.
>
> Looks like this addresses all the concerns I had, I don't think 
> there's much value in me rereading the document, but I'm happy to.
>
> - Steve
>
> On 15 Oct 2009, at 10:43, Simon Schenk wrote:
>
>> Am Mittwoch, den 07.10.2009, 12:58 +0100 schrieb Steve Harris:
>>> General Points
>>>
>>> Somewhere this document needs to say that update operations lean on
>>> SPARQL algebra, and how to resolve WHERE {} clauses w.r.t SPARQL/Query.
>>
>> Done in section 6.
>>
>>> Lots of parts of this document assume a particular outcome of
>>> ISSUE-20, ideally alternative designs would be shown, but at least the
>>> relevant sections should highlight this. I've tried to point out the
>>> obvious ones, but I may have missed some.
>>
>> Should all marked in notes.
>>
>>> I think that other reviewers might do better to wait for future
>>> revisions, as it stands there is not enough clearly finished content
>>> to warrant reviewing.
>>>
>>> All the red HTML errors need to be fixed.
>>
>> Done modulo XML-rec changes, as agreed in last telco.
>>
>>> 2.2 Aggregate functions
>>> 2.3 Service Description
>>> 2.4 Negation
>>> 2.5 BGP extensions of different entailment regimes
>>> 2.6 Basic Federated Query
>>> 2.7 Property Paths
>>> 2.8 Query Language Syntax
>>>
>>> These are all either "time permitting", or have no text in them, so it
>>> might be better if these sections were removed in the first draft.
>>
>> Removed the empty ones, kept the rest in section 2, which has been
>> renamed to "Relation to new features in SPARQL 1.1"
>>
>>> The examples in 3c,d,e have no timezone, it's not required in XSD, but
>>> it might be clearer with a timezone specifier, the examples in SPARQL/
>>> Update 1.0 have timezones.
>>
>> Done.
>>
>>> I think in these examples a description of the possible outcomes would
>>> make them more useful. E.g. in 3b, what happens if the DELETE fails to
>>> match anything?
>>
>> Done.
>>
>>> 4 The Graph Store
>>>
>>> "A Graph Store is a repository of RDF graphs managed by a single
>>> service." - what does "service" mean in this instance, I imagine it's
>>> not a service in the HTTP sense?
>>>
>>> "Unlike an RDF dataset, named graphs can be added or deleted to a
>>> graph store." does that imply that unnamed graphs cannot be added or
>>> deleted?
>>
>> Yes, because they cannot be identified.
>>
>>> 4.2 SPARQL/Update Services
>>>
>>> "SPARQL/Update requests are a sequence of operations." - SPARQL/Update
>>> requests need to be defined. there's a kind of description in 5, but
>>> it's not really sufficient.
>>
>> Added a note.
>>
>>> "If two different update services are managing their respective graph
>>> stores share
>>> a common graph..." - it sounds like this might contradict the first
>>> para of 4?
>>>
>>> "An implementation must target SPARQL queries on updated graphs if the
>>> SPARQL and
>>> SPARQL/Update end points are the same." - how can you tell? Should
>>> that be SPARQL/Query? Sounds to be at odds with 4.
>>
>> No contradiction.
>> For clarification added
>> "A Graph Store need not be authoritative for the graphs it contains."
>> to 4 and modified 4.2 as follows:
>> "If two different update services are managing their 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"
>>
>>> "Multiple operations can be packed into requests." - Using HTTP
>>> pipelining, or just lexically? Again, need definition of request.
>>>
>>> "The operations of a request are executed in order given." - lexical
>>> order? or some other?
>>
>> Clarified as follows:
>> "Multiple operations can be packed into requests. The operations of a
>> request are executed in lexical order."
>>
>>> 5.1 Graph Update
>>>
>>> "Graph update operations change existing graphs in the Graph Store but
>>> do not create or delete them." - that depends on the outcome of
>>> ISSUE-20, doesn't it?
>>
>> Added note mentioning dependence on ISSUE-20 and marking this behaviour
>> at risk.
>>
>>> 5.1.* would all be clearer with examples, <i>template</i> and
>>> <i>pattern</i> isn't really clear enough, and it shouldn't be
>>> necessary to read the grammar to broadly understand what these
>>> requests look like. Suggest you move/copy/reference the examples in
>>> section 3, for now at least. Ideally there would be more examples here.
>>
>> Done by referencing examples.
>>
>>> 5.1.4 DELETE and 5.1.5 INSERT
>>>
>>> The semantics need explaining. E.g. if multiple INTO clauses are used,
>>> is the WHERE pattern required to match for all graphs, or one graph,
>>> or just the graphs that will be inserted/deleted? It's not clear.
>>
>> Clarification:
>>
>>> 5.1.6 LOAD
>>>
>>> Needs to be mentioned in Security Issues section.
>>
>> Done.
>>
>>> 5.1.7 CLEAR
>>>
>>> Needs to reference ISSUE-20
>>
>> Done.
>>
>>> 5.2 Graph Management
>>>
>>> "The default graph in a Graph Store always exists." - earlier in the
>>> doc it says that there can be zero or one default graph.
>>
>> Fixed to one for now and added a not to 4 marking this behaviour at
>> risk.
>>
>>> 5.2.1 Graph Creation
>>>
>>> Needs to reference ISSUE-20
>>
>> Done.
>>
>> Cheers,
>> Simon
>>
>>
>> -- 
>> Simon Schenk | ISWeb | Uni Koblenz
>> http://isweb.uni-koblenz.de
>> http://www.uni-koblenz.de/~sschenk
>> Five sentences policy: http://five.sentenc.es/
>

Received on Friday, 16 October 2009 14:33:10 UTC