gmantele / vollt

Java libraries implementing the IVOA protocol: ADQL, UWS and TAP
http://cdsportal.u-strasbg.fr/taptuto/
30 stars 29 forks source link

Return type of user defined function #36

Open vforchi opened 7 years ago

vforchi commented 7 years ago

Hi,

I'm trying to define a UDF with the following signature: INTERSECTION(region1 region, region2 region) -> region I provided a class that implements it and it works, but if I use it in a select I don't get the desired format because the return value is interpreted as UNKNOWN instead of REGION.

Do I have to define it in my UDF class? IF so, how?

gmantele commented 7 years ago

How did you define and then declare your UDF?

vforchi commented 7 years ago

The class extends UserDefinedFunction, then I added the definition in the original post to the field udfs in the configuration file.

Just to clarify: everything works if I change the return type to double. I also tried to change isGeometry and isNumber in my UDF, with no luck.

vforchi commented 7 years ago

in ADQLQuery there's this

if (operand instanceof DefaultUDF && ((DefaultUDF)operand).getDefinition() != null) {
    DBType type = ((DefaultUDF)operand).getDefinition().returnType;
    ((DefaultDBColumn)col).setDatatype(type);
}

But my class is not an instance of DefaultUDF (it's a final class), but it extends UserDefinedFunction: am I doing something wrong?

gmantele commented 7 years ago

No, your are not doing anything wrong. In the tree generated by the ADQL parser, your function has been first set as a DefaultUDF and since you declared your UDF with a class extending UserDefinedFunction, the DefaultUDF has then been replaced by your class (thanks to DBChecker).

This piece of code is ADQLQuery is just in case no class has been specified for the UDF ; which is not your case.

The issue you have now is actually the same with all ADQL functions in my library: there is no precise datatype for them. All the time (except for DefaultUDF), the type is guessed from the ResultSet returned by your database. Since here you returned a geometry, the type stays unknown in the output VOTable.

There are two ways to solve this issue:

  1. If in your database the function INTERSECTION (or whatever is its real DB name) returns a REGION, you must indicate in your SQLServerTranslator how to identify geometry types ; in convertTypeFromDB and convertTypeToDB but also how to interpret such types by implementing the functions translateGeometryFromDB and translateGeometryToDB. For an example, you can see adql.translator.PgSphereTranslator.
  2. I should deal with a precise datatype for all functions in the ADQL library. That is however not a trivial modification of the library since it may change the behavior of translations in ADQLLib and result formatting in TAPLib. So I need some time to do that properly and to be sure to not break anything.

So, I will probably take care of the 2nd point, but you should actually deal with the first one on your side anyway....this is a normal way to support geometries in the TAP library.

vforchi commented 7 years ago

I'm afraid there's also something in the parser: I implemented {{convertTypeFromDB}}, but with this query:

select TOP 10  AREA(INTERSECTION(s_region, s_region)) from ivoa.ObsCore

I get this error:

<VOTABLE xmlns="http://www.ivoa.net/xml/VOTable/v1.3" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" version="1.3" xsi:schemaLocation="http://www.ivoa.net/xml/VOTable/v1.3 http://www.ivoa.net/xml/VOTable/v1.3">
<RESOURCE type="results">
<INFO name="QUERY_STATUS" value="ERROR">
Incorrect ADQL query: Encountered "(". Was expecting one of: ")" "." "." ")"
</INFO>
<INFO name="PROVIDER" value="ESO">"TAP service for data products (development)"</INFO>
<INFO name="REQ_ID" value="S1491303455753"/>
<INFO name="ERROR_TYPE" value="fatal"/>
<INFO name="ACTION" value="sync"/>
</RESOURCE>
</VOTABLE>
gmantele commented 6 years ago

Sorry for having not answer to this issue for a so long time.

This issue was actually due to the definition of AREA in the BNF for ADQL-2.0:

<geometry_value_function> ::=
 <box>
 | <centroid>
 | <circle>
 | <point>
 | <polygon>
 | <region>

It seems that it has been fixed in ADQL-2.1:

<geometry_value_function> ::=
        <box>
      | <centroid>
      | <circle>
      | <point>
      | <polygon>
      | <user_defined_function>

So, when I will start (soon I hope) implementing ADQL-2.1, this issue should be automatically resolved.

Anyway, I did not forget this issue. I will keep this issue thread updated when something new about it happens.

almicol commented 7 months ago

Hi Gregory, may I please ask you to comment on the status of this ticket, now that ADQL 2.1 is an official IVOA recommendation? Thank you in advance, Alberto