RE: Review of SPARQL 1.1

CVS to revision 119

> -----Original Message-----
> From: public-rdf-dawg-request@w3.org [mailto:public-rdf-dawg-request@w3.org]
> On Behalf Of Birte Glimm
> Sent: 02 October 2009 22:01
> To: SPARQL Working Group
> Subject: Review of SPARQL 1.1
> 
> Hi all, Andy, Steve,
> here is my review for the FPWD of SPARQL 1.1. I'll not be able to
> attend the telecon on Tuesday since I'll be travelling, but I hope we
> can sort out all questions before.
> Birte
> 
> 
> I am not sure what will happen with all the @@ comments, some might
> stay, but will, for example, the Abstract be rewritten before FPWD or
> will it just be annotated with @@ Old intro - rewrite?

We have removed abstract.  The introduction should suffice.

> I understand that the doc will eventually b merged with the SPARQL 1.0
> doc, but in several places you rely on definitions from Sec 12 of
> SPARQL 1.0. If after the merge your sections will be before Sec 12,
> you will have lots of forward references. Another way of merging would
> be to have all the examples in an earlier section and put the
> definitions into the section that is now Sec 12 of SPARQL 1.0. This
> would then follow the way that the SPARQL 1.0 doc is organised (all
> features introduced by motivation and examples and one section with
> fomal definitions). This is just something to keep in mind for the
> doc.

Yes - the plan is to merge this content into the main document, with explanation and formal definition arranged in the style as before.  When the content is merged, we can sort out references.

> In general, I would prefer to see some introductory sentence at the
> beginning of the sections. In Sec 2 (and others), for example, you
> directly start with an example without saying anything of what
> aggregate functions are, what the example will demonstrate, etc. One
> can just guess from the section heading that the example will
> demonstrate how aggregate functions work.

We hope to add some introductory text if there is time available.

> I woud also prefer if each grammar bit is preceeded with a short
> explanation of which parts of the grammar are new, which ones extend
> previous definitions and which ones replace previous definitions.

The grammar fragments are in the style of the original spec and are indicative of changes.  Until we have the complete grammar, the fragments are the new parts, tied to where they occur.  

We'll explain this.

> 
> Intro, 1st par: ... the backgraound to *the* choice

Done

> 
> Sec 2:
> Add headings Data:, Query:, and Query Result/Resuts: to the example to
> be consistent with the rest of SPARQL 1.0 and 1.1.

Done - and we'll make things consistent when merging.  The heading don't get repeated every time after a few 

> The @@ comment for ProjectValues refers to the Project rule, but that
> is only introduced later in the doc and is not from SPARQL 1.0, please
> add a link.

The grammar fragments have been updated.

> Algebra Operators:
> I suggest a complete sentence at the beginning, e.g.:
> The operators that make up aggregates consist of three functions: Key,
> Partition, and Aggregations.
> and then continue to define the three. Since the definitions are quite
> long, it is otherwise hard not to loose the context.
> Key is lowercase, but partition and aggregation are uppercase.
> I would also prefer complete sentences above the definitions, e.g.,
> Key *is* as function...
> I do understand what you mean by single-valued? Is it injective (a
> unique argument produces a unique result)? I don't think it is. Is the
> result always a singleton set? Also not really I think.
> Further, I think varlist in partition is not the same as varlist in
> key and I would prefer to name them something like namedVars and
> groupVars because for key it refers to the named variables from the
> select part of a query and for partition it refers to the named
> variables from the group by part of the query. I do not understand the
> def of partition and I am not sure whether or not you would need both
> var lists in partition. Here is my understanding of partition by means
> of an example (about books and their avarage price per publisher):
> Data:
> :a :publishes :a1 .
> :a :publishes :a2 .
> :b :publishes :b1 .
> :a1 :costs 12 .
> :a2 :costs 20 .
> :b1 :costs 4 .
> Query:
> SELECT ?p, AVG(?price) AS avg WHERE { ?p :publishes ?book . ?book
> :costs ?price . } GROUP BY ?p
> Result:
> p/:a, avg/16
> p/:b, avg/4
> Now I have solution mappings
> m1={ ?p/:a, ?book/:a1, ?price/12 }
> m2={ ?p/:a, ?book/:a2, ?price/20 }
> m3={ ?p/:b, ?book/:b1, ?price/4 }
> and omega = { m1, m2, m3 }
> I expect a partition
> { {m1, m2}, {m3} } or if you want to project out variables there already
> { { key(namedVars, m1), key(namedVars, m2) }, { key(namedVars, m3) } }
> For the first, I would define partition as follows:
> partition(groupVars, omega)={ { m_1, ..., m_n } | m_1, ..., m_n in
> omega and key(groupVars, m_1) = ... = key(groupVars, m_n) }
> for the latter
> partition(groupVars, namedVars, omega)={ { key(namedVars, m_1), ...,
> key(namedVars, m_n) } | m_1, ..., m_n in omega and key(groupVars, m_1)
> = ... = key(groupVars, m_n) }

The Partition definition was broken copying over from the wiki.  We need to fix this and make it clear.

> I don't think you want the later though because you still need the
> whole solution mapping to compute the average.
> I am not sure why you keep omega as part of the mappings in a
> partition and in the definition of aggregation it seems that partition
> returns a different omega then what it got as input, but this is not
> clear from the definition of partition. Maybe that is because you
> write omega in omega and that should be something else?
> For the aggregation, it remained completely unclear why it is defined
> as it is and how it is supposed to work. I would assume that I have in
> the example, AVG as agg, and I would need the partioning and then I
> compute a mapping that contains the aggregated values?
 
See above - we'll work though the definitions after fixing the error above.

> What does AST stand for in the section headings (Mapping from AST to
> Algebra)? Later in Sec 3 it says ASP. Is that intended?

Fixed.

> Below the example just before section 3, you use BGP(triple pattern).
> That is not really defined neither in SPARQL 1.0 nor here. SPARQL 1.0
> mentions that briefly in 12.2.1, but I suggest a more precise
> definition somewhere.

BGP(triple pattern) is used in examples in SPARQL 1.0 but this whole section is not yet fully written and may not be completed by FPWD.

> Will the wiki references stay in the doc? If so, I suggest to give
> them a heading such as "Further Material", "Relevant Links/Material".

These were part of originally creating the content.  They are not intended for the FPWD.

> Sec 3: Add "Query:" above the query in line with the SPARQL 1.0 doc.
> 
> Will the comments such as -- Andy at the top of Sec 4 stay?

Removed.

> Sec 4, 1st par: It *it* does

Fixed.

> 
> Sec 4, 2nd par: ...a fil*t*er expression

Fixed

> 
> Sec 4, Syntax, text below grammar rules for FILTER: ... and
> NotExistsPattern *s*but ...
> following note: equivalence to -> equivalent to
> 
> I would prefer complete sentences, e.g., "To facilitate this, "
> followed by a definition with a border around it looks awkward.
> Similarly in the paragraph below the definition the last sentence is
> just: See below. I would delete that.
> 
> I would not say "We define:" in the definition because it has already
> the heading "Definition: term to be defined"

Will revise.

> The subsection "Mapping from AST to Algebra" in Sec 4 starts directly
> with some data, no introductory sentence and after the data part of a
> sentence starts in lowercase "becomes a filter..." Also, in the given
> data, are <t> and <p> supposed to be a full URIs/IRIs? Is <p> the same
> as :p? The forms <p>, <t> are not used anywhere in SPARQL 1.0 other
> when t or are fullly qualified IRIs.

Will expand if time.

> 
> The section needs more explanation.

Will do if time.

> 
> Sec 5, Definition Extend: an RD*F* *T*erm. Again, I would prefr not to
> say "We define " in the definition.
> Note below syn*ta*ctic

 Andy

Received on Monday, 5 October 2009 17:25:31 UTC