ADQL-2.1 PR now available

[Mark Taylor]

On Mon, 15 Jan 2018, Marco Molinaro wrote:

> Dear IVOA members,
> a new Proposed Recommendation
> is available for you at the IVOA Doc Repo: ADQL-2.1.
> 
> http://ivoa.net/Documents/ADQL/20180112/index.html
> 
> Please read it and comment (on the DAL list).

Mostly looks good, but some comments:

sec 4.2.1:
   "ADQL provides a the following..." -> "ADQL provides the following..."

sec 4.2.4:
   Discussing the deprecated first (coordsys) argument of geometry
   functions, it mentions in passing that it's optional, but then says:

      "Services are permitted to ignore this parameter and clients
       are advised to pass an empty string here."

   I suggest it would be better to advise something like:

      "Services are permitted to ignore this parameter if present,
       and clients are advised to omit it from function invocations.",

sec 4.2.7:
    I drafted this text in the first place:

       "The purpose of this section is to recommend a preferred
        form of ADQL to use for sky crossmatches."

    but in context it sounds a bit clumsy.  Maybe better:

       "A preferred form of ADQL for sky crossmatches is therefore
        recommended."

sec 4.2.12, 4.2.16, 4.2.17:
    These sections about functions combining geometry contain text
    like:

       "If the geometric arguments are expressed in different
        coordinate systems, the XXX function is responsible for
        converting one, or both, of the arguments into a different
        coordinate system. If the XXX function cannot perform
        the required conversion then it SHOULD throw an error.
        Details of the mechanism for reporting the error condition
        are implementation dependent."

    Two comments.  Firstly, this is a bit confusing if you come
    to the specification without knowing ADQL 2.0, since the
    deprecated optional geometry function coordinates are not
    much discussed elsewhere in this document, and are not present
    in any of the examples, so it may not be clear to readers
    how a geometry can be expressed in differing coordinate systems.

    Secondly, this is somewhat at odds with the statement in sec 4.2.4
    "Services are permitted to ignore this parameter".  It's not
    quite a contradiction though, since the sections here only
    prescribe a SHOULD.

    I'm not necessarily proposing a change here, but there might
    be scope for wording it better.

sec 4.2.15:
    Since the deprecated, optional coordsys parameter has been
    excised from all the examples, the COORDSYS function is now
    rather pointless, and in particular the example in this section:

        COORDSYS(
            POINT(
                25.0,
               -19.5
               )
           )

    looks a bit silly or incomprehensible.  I'd suggest using an example
    with a non-empty coordsys argument to POINT (e.g. as in the v2.00
    document), but note again that this usage is deprecated.

sec 4.2.16:
    "the distance between to points" -> "the distance between two points"

sec 4.3.1:
    A reserved namespace is proposed:

       "The ivo prefix is reserved for functions that have been
        defined in an IVOA specification."

    I think that's not a bad idea, but I believe that in
    at least some cases current practice violates it; the DaCHS
    service at http://dc.g-vo.org/tap defines e.g., ivo_healpix_center,
    ivo_interval_overlaps, ivo_healpix_index, ivo_apply_pm.
    I've heard Markus defend that practice elsewhere, so I guess
    he should comment.

sec 4.3.2:
    This section is repeating text from TAPRegExt (would it be better
    just to reference it?), but the context in this document raises
    a couple of points.
    First:

        arglist ::= "(" <arg> { "," <arg> } ")"

    I think should (according to the notation defined in sec 2 of
    this document) read:

        arglist ::= "(" <arg> { "," <arg> }... ")"

    That copied text is not *necessarily* a mistake in the TAPRegExt
    document, since TAPRegExt doesn't define its BNF notation.

    Second:

        <form>match(pattern TEXT, string TEXT) -> INTEGER</form>

    having gone to the trouble of defining types in sec 3, shouldn't
    they be used, at least by way of example, rather than the
    undefined token "TEXT" here?  Though I'm not sure I've grasped
    the purpose of the type system, so I may be missing the point.

sec 4.7.1:
       "The CAST() function returns the value of the first argument
        converted to the datatype specified by the second argument."

    I can't tell from this *how* you are supposed to specify the
    datatype, e.g. is it quoted or unquoted?  CAST doesn't appear
    in the BNF either, so I can't find out from that.
    An example would be useful.

sec 4.8.1:
    The PDF includes the text:

       "... formatting defined in the VOUnits specification (Derriere
        and Gray et al. (2014))."

    but in the HTML it looks like:

       "... formatting defined in the ."

    I haven't checked if there are other similar instances of
    missing references in the HTML.

Appendix A:
    The <trig_function> production includes the COT function, which
    is not mentioned in sec 2.3.

    CAST and IN_UNIT appear in the BNF only as reserved words,
    no clue to the syntax - are they supposed to be there, or
    are they omitted because they're optional?

    There may be other instances of new constructions introduced
    at this version that are not reflected in the BNF, I haven't
    checked exhaustively.

Mark

--
Mark Taylor   Astronomical Programmer   Physics, Bristol University, UK
m.b.taylor at bris.ac.uk +44-117-9288776  http://www.star.bris.ac.uk/~mbt/