ADQL 2.1 Proposed Recommendation: Request for Comments
ADQL defines an SQL-like grammar adapted for astronomical purpose. It is especially used by the
TAP (Table Access Protocol) standard.
Latest version of
ADQL can be found at:
Here is a diff view of all the modifications done on the above PR document since the beginning of this RFC period:
Reference Interoperable Implementations
Implementations Validators
Two
ADQL parsing validators available in the
GitHub repository named
Lyonetia:
Both validators work offline and with no assumption on a specific database schema.
A
GitHub CI workflow is set up in this repository. It validates all test queries available in the
src/adql/ivoa directory thanks to the
ADQL validator based on VOLLT.
Two badges are visible on top of the
README.md document. The first one (named "Validator test") indicates whether the
ADQL validator based on VOLLT compiled successfully or not. The second one (named "ADQL Validation) indicates whether all test queries passed or not. An HTML report is visible by clicking on this second badge (after clicking on the badge, click on the "Master" page to see it). It shows which test file run and which tests failed.
Comments from the IVOA Community during RFC/TCG review period: 25-Apr-2023 - 6-Jun-2023
The comments from the TCG members during the RFC/TCG review should be included in the next section.
In order to add a comment to the document, please edit this page and add your comment to the list below in the format used for the example (include your Wiki Name so that authors can contact you for further information). When the author(s) of the document have considered the comment, they will provide a response after the comment.
Additional discussion about any of the comments or responses can be conducted on the WG mailing list. However, please be sure to enter your initial comments here for full consideration in any future revisions of this document
Community Review by Theresa Dower
TheresaDower (as an implementer of parts of the
ADQL 2.1 standard client and server side on odd architecture), 2023-04-27
I am so happy to see this moving forward!
- I see no show-stoppers in new features for the MAST/STScI frequently annoying special case MSSQL server back end.
- I have already, through the vollt adql library reference implementation and my tweaks to a MAST/STScI fork, added some of the features. This includes COALESCE and ILIKE and some updates to math functionality, mostly for the sake of canned queries PyVO expects for RegTAP 1.1. This has gone well so far. Thank you for the work on this reference implementation!
- I think deprecating the coordinate system arguments is ultimately good; it removes unforced errors from expectations of servers quietly converting things in the back end, which it seems no one is actually doing (we're ignoring it and defaulting to ICRS)
- ... that said, given the way a couple PyVO client expectations forcing server support have already caught us by surprise with RegTAP, I am a little nervous about the actual shuffle in operations when clients start dropping the deprecated pieces, and servers are expected to handle it. Hopefully we can improve coordination rolling out changes in the most popular clients and services that speak ADQL; we are already in a better place for global volunteer effort coordination than the past few years.
- This is all to say... should I be pulling the whole vollt ADQL 2.1 branch for MAST TAP services soon? Is there anything outstanding in it I should wait on? Happy to block some time to work on it.
- GM Answer:
- First of all, thank you for all your positive comments.
- About the optional features and how they are supported by clients and servers, both should know that supported optional features must be declared in the capabilities of the used DAL service (e.g. TAP). If an unsupported feature is used anyway, the server must raise an error in a way depending on the underlying query protocol. It is then up to the client to correctly report this error. It is also up to the client to eventually provide a way to show which features are supported, so that the use can adapt the query expression.
- [Speaking as the VOLLT developer] The ADQL-2.1 branch contains the full ADQL-library implementation. So, if you want to use only the query parsing and manipulation library, it is all OK. However, the TAP-library is not yet fully-compatible with this new version of ADQL (especially for the features declaration). That's why this branch is still a branch and is not yet merged with master. I am currently working on completing the integration of ADQL-2.1 into TAP-lib. So, in theory, the TAP service in this branch it should work, but things like the declaration of supported optional features is not yet integrated. The best is probably to wait for this branch to be merged with the master one.
- -- GregoryMantelet 2023-05-03
Community Review by Markus Demleitner
I have submitted a bunch of minor changes in
https://github.com/ivoa-std/ADQL/pull/91, too.
Larger points and ones that IMHO do not preserve meaning:
(1) p. 5: "Similarly to SQL, the
ADQL language definition is not semantically safe by design and therefore this specification defines syntactical correctness only. Type safety has been achieved as far as it can be done in in SQL." -- I don't think "semantically safe" is a well-defined term here, and I'm not sure what that passage is trying to say: that we tried to mimick a minimal type calculus in the grammar? I'd dispute that that has been a design goal (it has not been for me). Me, I'd drop the two sentences on grounds that they don't help but
may be untrue.
- GM Answer: Not being at the origin of these two sentences while not really understanding them, I agree to remove ambiguous or at least possibly untrue content. Consider it done in PR#95 ; and your PR#91 for "MAY"s removal. -- GregoryMantelet 2023-07-26
(2) "The selection list MAY include numeric, string or geometry value expressions." -- why do we say this, and with an RFC-inspiring uppercase MAY on top? Unless I'm missing something, I'd drop it for missing the triviality bar. In the PR I'm mentioning above I've also lowercased a few MAY-s that IMHO don't make sense for an RFC 2119 MAY and are just normal, colloquial mays (yes, I'm aware that we don't care about case for RFC-relevance, but still: I'd prefer if we used uppercase only to warn people aganst halfway unexpectable snags).
(3) The function tables on p. 16f: I'm in general not a big fan of tables when what something really is is a definition list or perhaps only an enumeration, and I'm almost always encouraging fewer rules within the tables. Perhaps we don't have to convert this in PR, but let's change the function enumerations to some more appropriate format in 2.2.
- GM Answer: Personally, no strong opinion here, as long as we don't loose any information. Let's do that in ADQL-2.2 then. I created the issue #93 to remember doing this. -- GregoryMantelet 2023-07-26
(4) p. 20: Yeah, I know... I certainly don't want to delay
ADQL 2.1 waiting for
anything in the wider vicinity of
STC. But can we please not cite the unfortunate 2013
STC-S WD but simply the non-normative appendix of TAP? So, I'd write
ADQL also provides support for STC-S based geometric regions. While a fully normative definition of the argument of REGION has not been written, implementations are advised to follow Section~6 in TAP 1.0 \citep{2010ivoa.spec.0327D}.
That's a lot more helpful than a reference to the WD that should have been pulled from the doc repo a long time ago. This will then also be consistent with what we write in 3.5.4 (where I'd not argue against dropping this discussion on p. 20 entriely).
- GM Answer: Agreed to replace the reference to STC-S WD by the TAP appendix. As you say, it is more consistent with 3.5.4 and it is safer to avoid citing something as unstable as a WD. This is done in PR#95. About dropping the discussion about STC-S, I would not do it in ADQL-2.1 ; it would be too soon. Let's speak about that in a future version of ADQL, when DALI will be ready to take over completely. -- GregoryMantelet 2023-07-26
(5) p. 26 There's a paragraph on COORDSYS here, but there's a proper subsection on it in 4.2.16. I'd suggest we drop the text on p. 26. A similar consideratioin would apply to the DISTANCE material on current p. 27.
- GM Answer: Do you speak about section 4.2.5 or 4.2.7? If about 4.2.5, I would keep this section ; it gives important information about the policy change in ADQL 2.1 regarding coordinate system management. But we should actually rename it: "Coordinate system" instead of "Coordsys" (which is ambiguous ; it could be about the COORDSYS function which is not the case here). If you speak about 4.2.7, I agree that this entire section should actually be removed (and maybe some content moved in the the COORDSYS and DISTANCE sections) because it duplicates what is already written in the COORDSYS and DISTANCE sections. I wait for your answer before doing anything in PR#95. -- GregoryMantelet 2023-07-26
(6) p. 38 "If the geometric arguments are expressed in different coordinate systems, the DISTANCE function is responsible for converting one, or both, of the arguments into a different coordinate system. If the DISTANCE function cannot perform the required conversion then it SHOULD throw an error. Details of the mechanism for reporting the error condition are implementation dependent." -- do we really want to do that? And does anyone implement that?
DaCHS certainly doesn't (for DISTANCE), although it will often think it has a good idea of the reference systems involved. But the whole thing is so troublesome and unpredictable (just think DISTANCE(ra+1, dec, l, b) -- is ra+1, dec still in ICRS?) that I won't implement anything like that. No, I'm sure that transformations are up to the scientists and should not happen magically.
But of course the scientists will need the
ivo_transform function I've lobbying for forever (and ivo_epoch_prop, by the way, but that's further along already) -- who's going to do the second implementation? But whatever: I'd suggest we drop the passage I'm citing above, as it's nothing but trouble and probably unimplemented.
(7) Analogously, we shouldn't say anything like that about INTERSECTS on p. 40 and on CONTAINS on p. 34. There,
DaCHS has traditionally attempted stuff like this, but it will no longer do so for geometries not constructed with COOSYS. Because it sucked.
(6) and (7)
GM Answer: Though I completely agree with you, I am quite reluctant to make a so major change between two minor
ADQL revisions (2.0 -> 2.1). We are going from one extrem to another: DISTANCE, CONTAINS and INTERSECTS have to perform coord. sys. conversion, and suddenly they must not. It is a quite hard transition while the coord. sys. arguments are just being deprecated (and not yet removed). I would rather prefer to state these functions must not do such conversion only when the coord. sys. argument will be definitely removed from
ADQL. Doing that right now seems too soon to me. What I would suggest, right now, is to add a comment such as:
"Since ADQL-2.1, the coordinate system argument of geometry constructors is deprecated as explained in section 4.2.5. While such argument is deprecated, DISTANCE, CONTAINS and INTERSECTS MAY still convert coordinates of its geometric operands if they are expressed in different coordinate systems. In a future version of ADQL, these functions will no longer be expected to perform any coordinate conversion." Maybe it should rephrase in another way, but that's the general idea of what I would like to write. Does it seem like a good compromise? Any other opinion on this topic? While waiting for an answer, I do not add any commit to
PR#95 regarding these two comments. --
GregoryMantelet 2023-07-26
(8) p. 44: "It is recommended that
ADQL implementers follow as much as possible the User Defined Function signatures listed in the Catalogue of
ADQL User Defined Functions". Umm... I think that is a bit unclear. If it means "When providing extended functionality, implementors should try to use UDFs from the UDF catalogue as much as possible", that's fine, but then it should be written like that. But perhaps even
ADQL itself should explicitly say something like: "UDFs prefixed with ivo_ always MUST follow the syntax and semantics given in the UDF catalogue." (Text like that is in the UDF cat, but some extra authority from
ADQL itself can't hurt).
- GM Answer: The intent of this sentence was indeed what you rephrased. Sorry for the clumsy wording. Mark Taylor, on behalf of the Operations IG, wrote a similar comment. I made a correction based on his comment in PR#92 : 1/ Both specifications and Endorsed Notes can define an ivo_ prefix, 2/ cite the UDF catalogue instead of RegTAP. This highlights the UDF catalogue. However, I don't think it is a good idea to limit the usage of the ivo_ prefix to just the UDF catalogue. I think it is still perfectly fair to also allow Recommandations to define the ivo_ UDF they need instead of waiting for the UDF catalogue to be updated. In this ADQL-2.1 section, we already say that the ivo_ prefix is reserved only for specifications and Endorsed Notes. I think it is already enough authority, though I agree that possibly spreading ivo_ UDF in multiple documents may lead to a bit of confusion for users. However, the UDF definition is never lost: it is defined in an IVOA spec. or EN, and it is declared as supported by a TAP service through TAPRegExt. So, what I suggest here, is to keep the correction suggestion from Mark Taylor and to remove the clumsy and confusing paragraph that you just mention. I just did this second correction in PR#95 which will be merged with the correction from Mark Taylor in PR#92. Is it ok like this? -- GregoryMantelet 2023-07-26
(9) LOWER, UPPER, ILIKE: The repetetive considerations on case folding are a bit tiring. Can't we just say "See LOWER for
ADQL's rules on case folding" in UPPER and ILIKE or something like that?
- GM Answer: I agree: repetition is annoying. Because UPPER does not reference the same algorithm as LOWER and ILIKE, I preferred to move the case folding explanations in a separate sub-section. I hope this proposition reads better. See PR#95. -- GregoryMantelet 2023-07-26
(10) Similarly the quip about 2m = 2km is perhaps useful for UNION, but it becomes tiresome in EXCEPT and INTERSECT. Perhaps again we can just cross-reference from the latter sections? Or move this discussion into 4.6.4? And frankly, I'd drop the displayed "2m = 2km". While instructionally impressive, I don't think it's something a specification couldn't live without.
- GM Answer: Agreed. Mark Taylor has also sent the same comment. I fixed it while joining and moving all duplicated explanations inside the PR#92. -- GregoryMantelet 2023-07-26
(11) Note that I did
not specifically review the BNF again. I
think I've looked at all of it over the years, as I followed it with my implementation, but I may have missed editorial mistakes. But then I'm hoping for a fast
ADQL 2.2 with a PEG grammar...
- GM Answer: You've certainly looked at this BNF more than me and you highly contributed to improve it. Me too, I really wait for the PEG grammar in ADQL-2.2. -- GregoryMantelet 2023-07-26
--
MarkusDemleitner - 2023-07-11
Comments from TCG member during the RFC/TCG Review Period: 25-Apr-2023 - 20-Jun-2023
WG chairs or vice chairs must read the Document, provide comments if any (including on topics not directly linked to the Group matters) or indicate that they have no comment.
IG chairs or vice chairs are also encouraged to do the same, althought their inputs are not compulsory.
TCG Chair & Vice Chair
With positive review by the TCG with a comments & feedback period successfully completed, the TCG chair/Vice Chair approve as well.
We took note of all the other changes requested already. The document is now well structured and clear. Well done.
Overall this is a substantial update to the
ADQL spec which is clear and easy to read.
Substantive
- §2.1.9 whitespace - In erratum 1, the section was titled "Whitespace and comments" which didn't refer to comments. Comments are defined in the BNF but are not allowed anywhere. Should this be made explicit in this section e.g. adding "Comments are not currently supported in ADQL."
- GM Answer: You're right. There is nothing about adding comments in ADQL though the BNF (and parsers) allows it. I just added a simple section description the syntax to use about comments and what parsers should do with them (in brief: nothing). See PR#100. -- GregoryMantelet 2023-10-11
- BNF - Should <numeric_primary> refer to <value_expression_primary> given that <value_expression_primary> can then refer to a numeric, string or geometry value expression?
- GM Answer: This is indeed a possibility given that, by instance, a column reference or a CAST function can return something else than a numeric. This problem may also happen with <character_primary> and <geometry_value_expression>. Unfortunately, I do not see any grammatical solution here. In such case, even a DBMS will throw an error only at execution time (instead of raising it at parsing time). This will be exactly what will happen with an ADQL query: it will pass the parsing but will fail on execution time. I guess it is the limit of a grammar. In my parser, I try to be as smart as possible in order to detect such problem, but it does not work in all cases. -- GregoryMantelet 2023-10-11
Trivial
- §4.2.3 - Coordinate limits - Galactic longitudes are often presented as [-180, 180] rather than [0, 360] - is it worth noting that this isn't supported in ADQL?
- GM Answer: Grid WG raised the same warning. Since we deprecated the coordinate system argument in geometrical expression, it is true that such coordinates limits make no more sense. We solved it in PR#99. -- GregoryMantelet 2023-10-11
- §4.2.4 - Datatype functions - The opening sentence "The following functions" confused me as I assumed the paras following were somehow defining functions. Instead, I gather this is referring to the functions defined in §4.2.8 onwards? If that is the case, then maybe reword it to something like "The functions in this section". If it is a subset it would be better to state it like is done in §4.2.6
- §4.2.5 - Coordinate system - "For interoperability reasons, queries against 2.0 services" - I assume this means "2.0 or later services"? If so it should state that.
- §4.3 User defined functions - The changelog states "Recommendation for data providers to look at the Catalogue of ADQL User Defined Functions before implementing a UDF" however while the catalogue is mentioned this recommendation isn't specifically stated
- GM Answer: You're right. It is more a suggestion than a recommendation. In his RFC review, Markus also wrote a comment about recommending this EN. Then, I fixed something and accidentely removed this explicit recommendation. However, as Markus also said, it should be stated clearly in this document. My previous wording was quite clumsy. I hope the paragraph I've just added in PR#100 is now better. -- GregoryMantelet 2023-10-18
- §4.3.1 Overview (UDF) - Should "Catalogue of ADQL User Defined Functions" have a citation reference here?
- GM Answer: Yes, it should, but it is not dependent from me. Ivoa-Tex has this rule of adding the citation only on the first occurence of the referenced resource. This Catalogue of UDF is indeed cited in section 4.2.5 ; there, we see the expected citation. -- GregoryMantelet 2023-10-18
- §4.6.2 Except - I assume that rows are excluded using the rules for duplicated rows in §4.6.4. Should this be stated?
- GM Answer: This was indeed done in a previous version of the ADQL 2.1 document, but it lead to a lot of repetition because ths rule about duplicated rows also apply to UNION and especially INTERSECT. But I assume we can have just a short sentence to tell readers to look at the section 4.6.4 to discover the exclusion rule but I fear it repeat again the same sentence in UNION, INTERSECT and EXCEPT sections while the very next section actually speak about duplicated rows. So, unless you really think it is absolutely important for readers, I think it is best to keep it like this. -- GregoryMantelet 2023-10-18
Raised in
PR#98
- §4.2.9 BOX
- "has been deprecated in this version of the standard" -> "has been deprecated in version 2.1 of the standard" so that it isn't confusing in v2.2
- Similarly "as of this version" should be replaced with "as of version 2.1" in §4.2.9, 4.2.11, 4.2.15, 4.2.18, and 4.2.19
- §4.4.2 LOWER - Should this mention that the conversion is according to the rules of the database's locale
- §4.4.3 UPPER - Should this mention that the conversion is according to the rules of the database's locale
- §4.5 Common Table Expressions - “Support for” is repeated in the sentence, making it a bit hard to read. It might be better if the second instance was dropped to read "MAY include support for the following optional common table expressions"
- §4.7.1 CAST - The type table appears above the "Input Types" paragraph making the last sentence ending in "support the following types:" rather confusing. I recommend the table be numbered and referred to in the text by number and, if possible moved to be below that paragraph.
- §4.9.1 IN_UNIT
- This section is the only one which refers to "numeric expression". I assume it intends "numeric value expression" as defined in the BNF? Should it be stated in that way (like in §4.2.16)
- "Consequently, regarding the unitless aspect of such numeric expression" -> Consequently, regarding the unitless aspect of such a numeric expression" (adding a)
- GM Answer: OK. Merged with main branch. -- GregoryMantelet 2023-10-18
--
JamesDempsey - 2023-10-10
The revised
ADQL spec v2.1 is well structured, clear and detailed. We observe that the authors integrated and answered to the comments received on this page. From the Data Model point of view, there is little implications and we have no further comments.
--
MathieuServillat - 2023-11-10
First, great standard and a very good advance. I have some comments, see below.
Minor vs Major version
My first comment is philosophical. If this is a minor version, should it be backwards compatible with version 2.0?
In the practical side, should a service
ADQL 2.1 compatible be able to provide unambiguous replies to
ADQL 2.0 queries? I partially (almost completely) implemented a parser for
ADQL 2.1 for the Gaia archive (not my current position), compatible with
ADQL 2.0 and sometimes the parsing was not able to get problems. I wonder if this is needed.
Removing the first geometrical parameter could cause ambiguous parsing, in particular when we use columns instead of values (where we can use the type to evaluate the syntax). For example, POLYGON(t.a, t.b, 3, 4, 5, 6, 7,...) where t.a could be a region or a first coordinate. "counting" parameters (even or odd) could help infer the data type expected but good parsing is not guaranteed) There are some clever restrictions in the spec (like requesting the same type of values for DISTANCE, either pair of coordinates or POINTS) that help a lot.
So, my general question is about backwards compatibility or not. In case this is the case, has this backwards compatibility been assessed in the spec for the operators that have changed signatures?
- GM Answer: I understand your point about backwards compatibility and I also sympathize with the parser update to 2.1. I experienced the same tricky ambiguous syntaxes with the geometrical functions parsing. As you did, I succeeded to clear up ambiguities thanks to the number and datatypes of parameters. I agree this is not obvious and quite tricky on this aspect. Hopefully, this geometries ambiguities are only temporary. They exist only because the coordinate system argument is deprecated and kept there optional exactly to ensure backward compatibility. So, unless I have missed something, ADQL 2.0 queries can be successfully interpreted and executed with an ADQL-2.1 processor. In this sense, backward compatibility is respected. Besides, I hope transitionning to a PEG grammar can highly improve parsing of ADQL queries, but we have to wait for the next version of ADQL to confirm that. -- GregoryMantelet 2023-10-11
REGION
The description of the syntax and level of implementation is still quite ambiguous to ensure a proper implementation. I wonder how many services are implementing it and if we really need it.
- GM Answer: Agreed. This minor revision of ADQL did not improve this part of the spec. I think this should be probably solved in a next version using region syntax described in DALI. I keep that in my notes for a next version of ADQL. Anyway, I think it is also good to keep some flexibility here ; I am quite in favor of recommending one or two syntaxes while still allowing any custom one, though I am not sure it would be really possible and if yes, how. Let's discuss about that while preparing next version. -- GregoryMantelet 2023-10-11
Coordinate limits
Now that we have gotten rid of the geometrical column for the operators, does it make sense to preserve this? Removing the coordinates system implies that the limits are not always [0, 360] and [-90, 90], isn't it? Are the services implementing this?
- GM Answer: You're right, and you are not the only one to make this comment. In my parser, I don't check whether the coordinates are in these ranges, and I don't know if any other implementation does it or not. In this regards, I think it would be reasonnable to change this part of the spec, either by removing these limits or by relaxing the constraint (e.g. by saying equatorial coordinates should be between these ranges). Let's try in PR#99 -- GregoryMantelet 2023-10-11
IN_UNIT
This is easy to implement in the parser but somehow tricky to implement in the database or similar. Are the reference implementations covering this (not at the parser level but at the DB level (or server level))? It does not sound easy to implement and I wonder if there are reference implementations of this operator.
- GM Answer: You're right, this is quite difficult to implement, and that's why it is an optional feature. My parser does not do that for the moment (but I'd like to implement it some time). However, the DACHS parser supports this function. I don't know about any other ADQL parser. -- GregoryMantelet 2023-10-11
WITH
It is not simple to parse the consistency of the query out of the WITH, having columns generated in the inner query. Should parsers ensure the correct use and the naming of the columns from the generated WITH query? (I really ignore the consistency of the outer query but it would be nice to know if there are better implementations)
- GM Answer: My parser checks for such column name consistency already without WITH queries. When some WITH queries are added, my parser merely sees them as tables. Considering that, it is not that difficult for my parser. I honestly don't know about other parsers, and anyway, I would never require them to do so, because as you said, it is not particularly trivial to do (I have spent some time myself to reach this level of check in my parser). -- GregoryMantelet 2023-10-11
For the rest, a clear spec and a good advance from previous language.
JesusSalgado - 2023-09-22
Section 4.9.1 describes IN_UNIT(), and the many cases where it is returning an error message. Maybe it would be good to give one example of possible success query ? The reason for this is that in the current form, it is very difficult for a reader not familiar with the TAP standard to figure out where the unit of the first argument is retrieved from (as it is not specified in the
ADQL query itself).
Maybe rephrase :
The first argument MUST be a numeric value expression. with : The first argument MUST be a numeric value expression. If the first argument is a column name, the VOUnits for this column ought to be found in the "unit" column of the TAP_SCHEMA.columns table of the TAP service.
maybe also add in the Examples : -- ra : column expressed in 'deg' (column_name="ra" and unit="deg" in TAP_SCHEMA.columns)
IN_UNIT(ra, 'rad') -- OK
SebastienDerriere 2023-11-10
- GM Answer: FIne with these changes. It is true a clarification was needed here for ADQL users. I applied all of them with a little more genericity regarding the way to get the unit for a column. See PullRequest #102. -- GregoryMantelet 2023-11-10
Interesting new features - thank Greg and all!
It looks good, more user-friendly, even if the migration needs important work for data-center (eg: WITH, CAST, geometrical functions, units)
Details :
- (4.2.7) "coord_value" is not defined.
- GM Answer: This part is an excerpt from the full BNF. So, obviously not all terms are not defined here, and should not be, otherwise it would too long inside the text. However, you're right, this paragraph is about a new function signature with new parameters, so it makes sense here to also includes the definition for <coord_value>. I just added it (see GitHub/ADQL#88). -- GregoryMantelet 2023-04-27
- (4.2.13) is it possible to use boolean instead of values {0,1} for CONTAINS ?
- GM Answer: Making boolean functions really returning a boolean value instead of an integer value (0 or 1) was a work initiated with this version of ADQL, but considering the important and complex grammar refactoring work it requires, it has been decided to postpone this for a next version (see GitHub/ADQL#32 for more details). I will speak about that at the May 2023 IVOA Interoperability Meeting. -- GregoryMantelet 2023-04-27
- (4.3) can you add link to UDF functions validated by IVOA (ivo_..) ?
- GM Answer: This link is already available in the document. It is referenced in sections 4.2.5 (COORSYS) and 4.3.1 (UDF Overview) (last words of the last paragraph ; but since it is not the first reference to this document the LaTeX template unfortunately hides the link); but I agree the PDF and HTML rendering does not make that kind of references really obvious in the text (except for the first reference occurrence) (or I am using the LaTeX template in the wrong way, which is really possible). It is also the 3rd reference in the References appendix. -- GregoryMantelet 2023-04-27
- (4.6.1) remove item "the columns in the operands SHOULD have the same metadata, e.g. units, UCD, etc." -
Tests should be limited to datatype - "same UCD" is clearly wrong (eg.: JOIN on primary-foregin key use meta.id;id.main and meta.id) , furthermore VOTable have not always UCD.
- GM Answer: For exactly the reason you mention, this item is a SHOULD instead of a MUST. It is just a recommendation and not a requirement to encourage data providers to provide set operation result with as rich and consistent metadata as possible. In this item, the UCD is just an example of metadata on which a data provider can choose to add a strong constraint while performing a set operation. So, it is up to the implementation to decide whether or not this constraint should be set, and on which metadata. Although I agree this can be tricky for UCD, this item could be perfectly valid for units and possibly for any other metadata a data providers can offer. So, since this item is just a recommendation (not a requirement) and is really up to implementation, I suggest to keep this item. -- GregoryMantelet 2023-04-27
Coord syst: Coordinate system parameter deprecation has impacts.
For instance, coordinate system used in
VizieR tables depends of each catalogues.
The coord syst is important in case of crossmatch (from an other table or from an UPLOAD)
If the parameter is removed, it requires;
- TO FIND A WAY TO REPORT THE NATIVE COORDINATE SYSTEM TO USER - in TAP_SCHEMA for instance
- GM Answer: You are perfectly right. This is however out of the scope of the ADQL standard. This information must be provided by the TAP standard. I know that a new version of TAP is in preparation and that a talk will happen on this topic at the May 2023 IVOA Interoperability Meeting. It seems to be a very good timing to suggest this addition to TAP. Personally, I propose coordinate system information on columns in the VOLLT/TAP library. This can be used as inspiration for next version of TAP. -- GregoryMantelet 2023-04-27
- to use UDF functions to make the conversion (is it possible to add with explicit example using UDF)
- GM Answer: It may be possible, though such functions should be defined in the Catalogue of UDF Endorsed Note. Currently, only one conversion function is defined in there: gavo_transform(from_sys, to_sys, geo) (see sec. A.1.5). Note the function prefix: gavo_. So, it is not yet an official IVOA User Defined Function ; it is just a function defined in a single service implementation. I know that other TAP implementations have implemented a such function. If at least 2 TAP services have the same function (except for the function prefix), I think we can make this function official and give it an ivoa_ prefix ; from what I know there are a GAVO and VizieR implementation for a such function. Thus, maybe it is the good moment to propose a new version of this Endorsed Note. Anyway, to come back on giving examples in the ADQL standard, I don't think it is wise to do so now ; it is too early. I prefer to wait until an ivoa_ function is officially endorsed in this note rather than suggesting to use an unstable and soon-deprecated function. Thank you Gilles for this suggestion, it will probably help updating the UDF catalogue, and I keep this idea also for the next version of ADQL. -- GregoryMantelet 2023-04-27
Notre : (4.2.16) COORDSYS function looks strange if system is removed from geometrical function - it asks a background
ADQL processing
GM Answer: This function is also deprecated. Once the coord. sys. argument definitely removed from all geometrical functions, the COORDSYS function will also be removed. --
GregoryMantelet 2023-04-27
This is generally well-written. Special commendation for the careful change list in Appendix C.1. I am happy to endorse it, but have a few suggestions that should be considered by the authors.
- I made a few uncontroversial fixes in PR#89 (merged on the 7th May 2023).
- Sec 2.1.3 SQL reserved keywords list: The entry END-EXEC doesn't really make sense here, since it is not syntactically capable of use as an Identifier. I suggest its removal.
- GM Answer: Although I completely agree with this, I prefer to keep it here as it is a reserved keyword in SQL-92 and that I can find it in some SQL dialects. It is probably being too much careful here, though. If another person wants to drop it, I won't fight more than that. -- GregoryMantelet 2023-07-25
- Sec 2.2.3 Search Condition could contain a reference to the IN construct alongside the others listed.
- Sec 4.2.8 (written by me, blame attributable accordingly): this section promotes use of the DISTANCE()<r idiom for crossmatching. It does not explicitly promote its use in cone-search-like WHERE clauses as well, but I now think it should do. I suggest appending the following paragraph to the end of this section: "Similar remarks apply to cone-search-like conditions in WHERE clauses: clients are encouraged to use, and services to provide efficient implementations of, conditions of the form WHERE DISTANCE(...) < r_max_deg."
- GM Answer: Are you sure it is needed? It is already explicitly said in the second paragraph that the WHERE syntax is allowed. It is also said that both syntaxes have to be supported and the server must guarantee the same level of performance. Would you mean that an example with the WHERE syntax is missing? -- GregoryMantelet 2023-07-25
- Only that the language throughout this section, including the title, is that of crossmatching which makes it look like that's the only context in which it applies. Where I mentioned WHERE in the second paragraph I was thinking of JOIN ... WHERE rather than SELECT ... WHERE. If you agree I could draft a PR to shift this emphasis and clarify a bit? I don't think a new example is required, though it might help. -- MarkTaylor - 2023-08-08
- Please, do so. I let you draft a PR to fix this. -- GregoryMantelet 2023-08-08
- Sec 4.2.13, 4.2.17, 4.2.18: the sections on CONTAINS, DISTANCE and INTERSECTS all contain a paragraph at the end along the lines: "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." This consideration only makes sense when the coordinates are specified using their optional, and now deprecated, coordsys arguments, so they are a bit confusing for a reader not familiar with the previous version of the standard. I suggest that these paragraphs are either removed, or that a comment is added to the effect that they only apply where the deprecated coordsys argument has been specified.
- GM Answer: You are right. This is ambiguous. Markus D. more or less stated the same thing. I am going to merge your remark and the comments of Markus together ; a common solution proposition will come in a PullRequest (in progress). -- GregoryMantelet 2023-07-25
- Sec 4.2.20 POLYGON. This section does not explicltly say what counts as inside or outside the specified polygon. It does say that it "corresponds semantically to the STC polygon," which probably answers the question, but the relevant STC definition discusses some non-sky geometries and is quite hard to understand (at least by me). I'm not sure what's best to do here, but possibly a reference to DALI which distills the STC advice into a more readable form might be helpful, if that is the intended semantics here (STC has some additional constraints about the maximum length of a polygon side). In practice it might be best to find out what the de facto standard PgSphere implementation does and make sure that advice is provided here which matches that. Or maybe I'm overthinking this and it's fine as it is.
- Commenting on my own comment, following AlbertoMicol 's polygon presentation at the Bologna Interop - my remarks here may be misguided or wrong. It could be worth considering whether the existing text is optimal in the light of Alberto's comments, but I no longer claim to have a good idea about what the right thing to do is. -- MarkTaylor - 2023-05-10
- GM Answer: Like you, I do not know exactly what should be done here for the moment. Alberto's presentation is a good start to clarify this polygon situation more generally in DAL. In the meantime, I am tempted to not change anything ; the risk is to add more ambiguous content in IVOA standards regarding the polygon definition. -- GregoryMantelet 2023-07-25
- Sec 4.3.1: "The ivo prefix is reserved for functions that have been defined in an IVOA specification" - this isn't quite true (assuming that "specification" refers to a Recommendation-track document) since the Catalogue of User Defined Functions referenced in the next paragraph is an Endorsed Note rather than an IVOA Specification. Could be reworded as "... have been defined in an IVOA specification or Endorsed Note". It might also be a good idea to use examples from the Catalogue of UDFs rather than RegTAP in the examples here, since that will be the more typical case.
- Sec 4.3.2: This section discusses declaration of UDFs as TAPRegExt Language Features. It quotes BNF for UDF signature syntax from TAPRegExt, including the terminal <type_name>. TAPRegExt 1.0 [or 1.1] sec 2.3 says "The type_name nonterminal is not defined by the ADQL grammar [in version 2.0]. For the purposes of TAPRegExt, it is sufficient to assume it expands to some sort of SQL type specifier." Since ADQL 2.1 now has introduced a type system, it would be appropriate to say something about type_name in this ADQL section beyond implicitly deferring to TAPRegExt (which refers to an earlier version of ADQL). I suggest something like "The <type_name> SHOULD be one of the terms defined in Section 3."
- Sec 4.6.1, 4.6.2, 4.6.3: These sections are quite repetitious.
- They each contain a list of MUST/SHOULD criteria which are almost, but not quite, the same as each other (I think by oversight rather than design). But in fact I don't think the lists quite make sense in any of these cases. The first two items (number/type equivalence of columns) are indeed MUST criteria for the operations, but the other two are not. I suggest retaining the first two as MUST criteria as at present, and replace the metadata advice with "In addition the operands SHOULD have the same metadata e.g. units, UCD etc. The metadata for the results SHOULD (or MUST?) in any case be generated from the left-hand operand."
- They each end with a warning that no clever unit manipulation will be done, ending with the displayed formula "2m = 2km." In my opinion the whole paragraph is kind of obvious and could be omitted; but at least I suggest to remove the (obviously untrue!) sentence "2m = 2km", which doesn't really add anything.
- It might be worth pulling out both these items and saying them once near the start of sec 4.6 rather than repeating them in each of the subsections 4.6.1, 4.6.2 and 4.6.3.
- GM Answer: I agree too. Comment similar to the one from Markus D. I will work on a common solution in a new PullRequest. -- GregoryMantelet 2023-07-25
- GM Answer: Actually, I removed all the duplications and the "2m = 2km" inside the PR#92. -- GregoryMantelet 2023-07-26
- I find the presentation style for ADQL examples in which each parameter is written on a different line, leading in most cases to examples extending over many lines when they could be fit on one, a bit annoying. But that's just my personal preference, if the authors prefer it as it stands that's fine.
- GM Answer: I personally completely agree with you. I did not touch this presentation style to preserve the style of the former editors. I am 100% ok to change that. Consider it done in PR#92. -- GregoryMantelet 2023-07-25
--
MarkTaylor - 2023-05-07
Nothing to add.
--
Anne Raugh - 2023-11-10
TCG Vote : 6-Jun-2023 - 20-Jun-2023
If you have minor comments (typos) on the last version of the document please indicate it in the Comments column of the table and post them in the TCG comments section above with the date.
Group |
Yes |
No |
Abstain |
Comments |
TCG |
* |
|
|
|
Apps |
* |
|
|
|
DAL |
* |
|
|
|
DM |
* |
|
|
|
GWS |
* |
|
|
|
Registry |
* |
|
|
|
Semantics |
* |
|
|
|
DCP |
* |
|
|
|
Edu |
|
|
|
|
KDIG |
* |
|
|
|
Ops |
* |
|
|
|
Radio |
|
|
|
|
SSIG |
* |
|
|
|
Theory |
|
|
|
|
TD |
* |
|
|
|
<nop>StdProc |
|
|
|
|
<!--
Set ALLOWTOPICRENAME =<span class="WYSIWYG_PROTECTED">
TWikiAdminGroup </span>
-->