Re: Update review

On Fri, Oct 16, 2009 at 10:32 AM, Luke Wilson-Mawer
<luke.wilson-mawer@garlik.com> wrote:
<snip/>
> 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.

Most of them seem fine. I have a couple of questions and comments below:

> PRE FPWD
>
> General Points
>
>   * Unwanted red HTML output is still there, although I know this will
>     be fixed in the publication process.

This is just because <i></i> and <b></b> are not permitted. I've been
replacing <i> with <em> and it looks better.

>   * Subheadings are inconsistent.  Issues headings aren't graded
>     particularly clearly; it's hard to see where an issue begins and ends.

I'll look at this, but specific suggestions would be appreciated.

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

Agreed. The attached ISSUE should be the appropriate place for this.

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

It looks like the word "contains" was missing.

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

I presumed that it meant that if you get a local copy of a graph, then
you may have updated it. Either that, or it may have fallen behind
revisions if you grabbed a snapshot of a graph that was still under
active development. For instance, the authoritative graph for
<http://example.com/data/foo.rdf> may be the document found at that
location. But it's also possible to get a copy of that graph (as an
RDF/XML document) and load it up in a graph store. This would improve
efficiency by avoiding network overhead, and indexing the data
locally.

That's just my reading of it. If that doesn't make sense, or the
phrasing is still obscure then we can try again.

> 4.1
>
>   * The first paragraph is confusing - I don't think it is clear what
>     it is trying to say.

It appears to be trying to describe the default graph without saying
it outright. In particular, it's trying to cover the case for a
default graph that is a union of the other graphs in the store.

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

I've always liked the idea of an update service being able to do
queries in general, though I don't know if there is support for this.
If it's not, then I suggest that a SELECT and an INSERT should *not*
be allowed in the same query. I'd like to hear more comments before
changing this (though I agree that the style is not right).

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

Actually, no. This is the name of the element in the syntax from the
SPARQL/Query definition:
  http://www.w3.org/TR/rdf-sparql-query/#rGraphGraphPattern

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

I don't believe that file:// URIs are being explicitly excluded, no.
I'll certainly be supporting them myself. :-)

I updated this to document URI.

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

It seems to me that the prose uses lower case to refer to the action
in general terms, as opposed to explicitly referencing the command.

>   * Sometimes issue names such as 'ISSUE-20' are linked and sometimes
>     not and the issue number is repeated for each issue.

There is a pattern to each mention of an issue. Anything in braces in
the following template is different for each issue:
  Issue (ISSUE-{##}):
  {Description}
  Source: ISSUE-{##}
  Resolution:
  {Description of resolution - typically "None recorded"}

The only times that issues are ever linked is from "Source" and they
are linked consistently.

I'm all for changing the above (ISSUE-## comes up twice, for instance,
and it's hard to see where one issue ends and the next starts), but
the usage is currently consistent.

>   * 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?

It doesn't belong here, no. I guess that it should be in HTTP-Update.
I'll mention it in a separate email.

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

This implies interchangeability to me, which I don't think is the
case, is it? (Is it legal to have MODIFY without both INSERT and
DELETE?)

>   * In my opinion, a more wordy introductory paragraph, rather than a
>     list of specific descriptions of the various verbs, would be
>     better here.

Will think on this.

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

I've added a note here. The implications warrant further discussion.

Regards,
Paul Gearon

Received on Friday, 16 October 2009 21:56:00 UTC