gmantele / vollt

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

Geometry UDF not treated as such by ADQL parser? #155

Open mbtaylor opened 6 months ago

mbtaylor commented 6 months ago

Hi @gmantele. I noticed some ADQL queries using UDFs that are declared to return POINT values are not being parsed as I'd expect; if they are used as the argument of e.g. COORD1, the parsing reports a syntax error, though standard ADQL geometry functions can be used in such positions. This is the case even though the relevant FunctionDef.isGeometry() call returns true. I present a short program that demonstrates this:

import adql.db.FunctionDef;
import adql.parser.ADQLParser;
import adql.parser.feature.FeatureSet;
import adql.parser.feature.LanguageFeature;
import adql.parser.grammar.ParseException;

public class GeomUdfTest {
    public static void main(String[] args) throws Exception {

        // FeatureSet with some basic geometry functions.
        FeatureSet fset = new FeatureSet(false);
        fset.support(new LanguageFeature(LanguageFeature.TYPE_ADQL_GEO, "POINT", true));
        fset.support(new LanguageFeature(LanguageFeature.TYPE_ADQL_GEO, "COORD1", true));

        // Add a POINT-yielding and a DOUBLE-yielding UDF.
        // The POINT-yielding one is correctly identified as geometrical.
        String[] udfForms = {
            "POINT_UDF(x DOUBLE, y DOUBLE) -> POINT",
            "FLOAT_UDF(x DOUBLE, y DOUBLE) -> DOUBLE",
        };
        for (String form : udfForms) {
            FunctionDef fdef = FunctionDef.parse(form);
            System.out.println(fdef + "; isGeom: " + fdef.isGeometry());
            fset.support(fdef.toLanguageFeature());
        }

        // Set up a parser.
        ADQLParser parser = new ADQLParser(ADQLParser.ADQLVersion.V2_1);
        parser.setSupportedFeatures( fset );

        // Parse some queries.  The built-in geometry function is OK
        // as the argument of COORD1, but the geometry UDF is not.
        String[] queries = {
            "SELECT * FROM dual",
            "SELECT COORD1(POINT(ra, dec)) FROM dual",      // succeeds
            "SELECT COORD1(POINT_UDF(ra, dec)) FROM dual",  // fails
        };
        for (String q : queries) {
            System.out.println(q);
            try {
                parser.parseQuery(q);
                System.out.println(" --> OK");
            }
            catch (ParseException e) {
                System.out.println(" --> Fail: " + e);
            }
        }
    }
}

which produces the following output:

POINT_UDF(x DOUBLE, y DOUBLE) -> POINT; isGeom: true
FLOAT_UDF(x DOUBLE, y DOUBLE) -> DOUBLE; isGeom: false
SELECT * FROM dual
 --> OK
SELECT COORD1(POINT(ra, dec)) FROM dual
 --> OK
SELECT COORD1(POINT_UDF(ra, dec)) FROM dual
 --> Fail: adql.parser.grammar.ParseException:  Encountered "(". Was expecting one of: ")" "." "." ")" 

I am using adqlLib-2.0-beta.

Is this known/expected behaviour? Thanks.

gmantele commented 6 months ago

Hi @mbtaylor .

This is a known behavior....but, of course, it is undesired. It has already been fixed in the next coming release of ADQL (branch adql2.1): https://github.com/gmantele/vollt/commit/7ada398b9bb13f94e950051e3856b95632a66985#diff-7f5b3b1d90449d14a965fa8e0a86538a79005d50b0f84d11027cd177b689d537

The problem actually came from the parser which was too strict. It will be fixed for ADQL-2.1 queries.

mbtaylor commented 6 months ago

Hi @gmantele - the above output is using the jar file adql-2.0-beta.jar, which IIRC corresponds to the tag adqlLib-2.0-beta which is a point on branch adql2.1 later in the history than 7ada398. So I think this remains unfixed.

gmantele commented 6 months ago

Ok. Then, I'll try to look at this in more details sometime next week.