Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
mbtaylor opened this issue Feb 20, 2024 · 3 comments
Open

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

mbtaylor opened this issue Feb 20, 2024 · 3 comments
Assignees
Labels

Comments

@mbtaylor
Copy link
Contributor

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 gmantele self-assigned this Feb 27, 2024
@gmantele gmantele added the bug label Feb 27, 2024
@gmantele
Copy link
Owner

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): 7ada398#diff-7f5b3b1d90449d14a965fa8e0a86538a79005d50b0f84d11027cd177b689d537

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

@mbtaylor
Copy link
Contributor Author

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
Copy link
Owner

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants