ADQL parsing error on: distance(centroid(s_region),point('',187.48, 2.05))

[almicol]

Hi Gregory,
A query like:

SELECT top 10 centroid(s_region), distance(centroid(s_region),point('', 187.48, 2.05))
FROM ivoa.ObsCore

returns the error:
Incorrect ADQL query: Encountered "centroid". Was expecting one of: "POINT" """ <REGULAR_IDENTIFIER> "POINT" <REGULAR_IDENTIFIER> """

Though, CENTROID does return a POINT.

Using distance( cast(centroid(s_region) as POINT), point('', 187.48, 2.05) ) does not help.
Hope you could have a look.
Many thanks in advance,
Alberto

[vforchi]

For the record, this happens on our implementation, that uses a SQLServer backend, but the error comes from the parser, that expects either a Point or a Column as argument of the DISTANCE operator.

[gmantele]

Yes, exactly. The parser is accepting only a POINT or a column reference as parameter of DISTANCE. This is actually how it is described by the ADQL's standard (v2.0). I agree that allowing CENTROID in parameter of DISTANCE would make perfectly sense otherwise.

I could easily change this behaviour, but my parser would not follow anymore the standard. In theory it is not a real big deal, but TOPCAT is using it in its ADQL editor.

Maybe it could be possible to make this part of the next version of ADQL (v2.1).

[gmantele]

A way to get round this grammar issue would be with the following ADQL query:

SELECT c, distance(c, point('', 187.48, 2.05))
FROM (SELECT TOP 10 centroid(s_region) AS c FROM ivoa.ObsCore) AS trick

Ugly, but it should do what is expected.

[vforchi]

ADQL 2.1 already specifies that DISTANCE works between two geometries:

The specification defines two versions of the DISTANCE function, one that accepts two geometries, and one that accepts four separate numeric values.

[gmantele]

Not according to the following:

The DISTANCE function computes the arc length along a great circle
between two points and returns a numeric value expression in degrees.

And also to the grammar definition:

<distance_function> ::=
DISTANCE <left_paren>
<coord_value> <comma>
<coord_value>
<right_paren>
| DISTANCE <left_paren>
<numeric_value_expression> <comma>
<numeric_value_expression> <comma>
<numeric_value_expression> <comma>
<numeric_value_expression>
<right_paren>

<coord_value> ::= <point> | <column_reference>

[vforchi]

I would say that, at the very least, the standard is not clear on this point (pun intended):

This function computes the centroid of a given geometry and returns a POINT (See 2.4.11).

By the way, the reference does not exist.

[almicol]

Your proposed get-around (ugly, but interesting!) query returns an error:
Incorrect ADQL query: 1 unresolved identifiers!

• Type mismatch! A geometry was expected instead of "c".

You might want to have a look at that too.

[gmantele]

A first comment to both of you: the ADQL library is not yet adapted for ADQL 2.1. So the function CAST, for instance, can not yet work.

Then, @almicol, I admit to be a bit puzzled. This query passes the ADQL query validation on the validator of the website (http://cdsportal.u-strasbg.fr/adqltuto/validator.html) which is using the last stable version, and passes also the validation on the new on-development version on my computer.

• Are you using a special version? If yes, which one?
• Have you changed a bit the parser or any related class?

[vforchi]

Agreed, it's not in 2.0, I was quoting 2.1 only because you said it could be included in 2.1, but the current draft is not very clear.

We are using my fork, where I added the support for the geometry in SQLServer.
I didn't change the parser, but I modified a little bit the translator. I'll check if I introduced the error.

For the record, I forked from the trunk, not from the stable version, so it may be something there.

[vforchi]

It looks like you changed this method in ADQLColumn

@Override
public boolean isGeometry(){
   return (dbLink == null || dbLink.getDatatype() == null || dbLink.getDatatype().isGeometry() || (dbLink.getDatatype().isUnknown() && !dbLink.getDatatype().isNumeric()));
}

from:

@Override
public boolean isGeometry(){
   return (dbLink == null || dbLink.getDatatype() == null || dbLink.getDatatype().isGeometry());
}

[gmantele]

Indeed, but it is actually not the problem. It turns out that I declared the wrong return type for CENTROID in CentroidFunction ; it was declared as a numeric instead of a geometry. After having fixed that, the error disappeared (but another occurred: bad translation of CENTROID into PgSphere ; but that should not bother you). So, that should fix the problem on your side too. Otherwise, tell me.

To get this correction, either you update your fork with the last version of my repository, or you apply the correction by yourself after having read my last commit. I recommend you the first solution because it will avoid you merge issues later.

---

By the way, I could also import your SQLServer translation corrections (atan2 and TOP/DISTINCT) in my repository ; you can send a PullRequest if you want. Otherwise, I will probably apply the same corrections later on my side.

[gmantele]

The grammar issue about the function DISTANCE is reported on ivoa/lyonetia#6 and may be fixed in the next version of the ADQL standard (v2.1).