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

register user defined functions #151

Open
vforchi opened this issue Dec 21, 2023 · 11 comments
Open

register user defined functions #151

vforchi opened this issue Dec 21, 2023 · 11 comments
Assignees
Labels

Comments

@vforchi
Copy link
Contributor

vforchi commented Dec 21, 2023

I am trying to port my TAP server to the adqLib 2.0, and I noticed that it doesn't find the user defined functions.

With adqlLib-1.5 I just added them to the DBChecker.

@gmantele gmantele self-assigned this Jan 5, 2024
@gmantele
Copy link
Owner

gmantele commented Jan 5, 2024

Yes, and that's "normal". It is the reason why the branch adql2.1 is still a branch. I have to finalize the integration of the new ADQL-Lib into TAP-Lib. For the moment, TAP-Lib is not able to work with this new version of ADQL-Lib....that's what I am working on currently. Sorry for the delay in this development. I also try to make TAP-Lib fully compatible with TAP-1.1 (which is still not the case for the moment).

@vforchi
Copy link
Contributor Author

vforchi commented Jan 8, 2024

But I am not using TAP-Lib: how should I register UDF into my program?

@gmantele
Copy link
Owner

gmantele commented Jan 9, 2024

Ok. I did not know.

You can find an example on how to declare a UDF with this new version in the directory ADQLLib/examples/, and more precisely: ADQLLib/examples/adql/example/parse/E_DeclareUDF.java

@gmantele
Copy link
Owner

gmantele commented Jan 9, 2024

Let me know if you need more help while waiting for an updated documentation.

@vforchi
Copy link
Contributor Author

vforchi commented Jan 9, 2024

Thanks for the pointer, I played around and I still can't make it work.
I have some classes that extend UserDefinedFunction, I added them all to the checker, and now I tried to add them to the parser, like in the example, but I get Unsupported ADQL feature:

if I do

System.out.println(udf.getFeatureDescription());
System.out.println(udf.getFunctionDefinition().toLanguageFeature()); 

I get

ivo://ivoa.net/std/TAPRegExt#features-udf!ESO_DATEADD_SEC() -> ?type?
ivo://ivoa.net/std/TAPRegExt#features-udf!ESO_DATEADD_SEC(seconds INTEGER, date TIMESTAMP) -> TIMESTAMP

whichever I add to the parser, I get the same error. I guess it doesn't like the prefix?

For reference, the UDF is:

package org.eso.dfs.tap.adql.udf;

import adql.query.ADQLObject;
import adql.query.operand.ADQLOperand;
import adql.translator.ADQLTranslator;
import adql.translator.TranslationException;

public class DateAddSecondsFunction extends TapUserDefinedFunction {

    public DateAddSecondsFunction(ADQLOperand[] params) throws NullPointerException {
        super(params);
    }

    public DateAddSecondsFunction(TapUserDefinedFunction toCopy) throws Exception {
        super(toCopy);
    }

    @Override
    public boolean isNumeric() {
        return false;
    }

    @Override
    public boolean isString() {
        return true;
    }

    @Override
    public boolean isGeometry() {
        return false;
    }

    @Override
    public String getName() {
        return "ESO_DATEADD_SEC";
    }

    @Override
    public ADQLObject getCopy() throws Exception {
        return new DateAddSecondsFunction(this);
    }

    @Override
    public String translate(ADQLTranslator caller) throws TranslationException {
        StringBuffer buf = new StringBuffer("dateadd(ss, ");
        buf.append(caller.translate(parameters[0]));
        buf.append(", ");
        buf.append(caller.translate(parameters[1]));
        buf.append(")");
        return buf.toString();
    }

    @Override
    public String getSignature() {
        return "ESO_DATEADD_SEC(seconds INTEGER, date TIMESTAMP) -> TIMESTAMP";
    }

    @Override
    public String getDescription() {
        return "Some description";
    }

}

Am I missing something?

@gmantele
Copy link
Owner

The prefix you get is perfectly normal. This is the default toString() implementation. This serialization is not aimed for anything else than debugging.

It seems your UDF is just about a simple translation from ADQL to SQL (no magic thing done in Java before writing the SQL translation). If so, you no longer need to use a UserDefinedFunction. The following lines should work (sorry, I am not able to test right now):

// DECLARE A UDF:
// ...get the list of supported features to update:
final FeatureSet features = parser.getSupportedFeatures();
// ...define this function:
FunctionDef myUdf = FunctionDef.parse("ESO_DATEADD_SEC(seconds INTEGER, date TIMESTAMP) -> TIMESTAMP", parser.getADQLVersion());
// ...set a human description (visible in the capabilities of the TAP service):
myUdf.setDescription("Some description");
// ...set (SQL) translation pattern:
myUdf.setTranslationPattern("dateadd(ss, $1, $2)");
// ...now add it to the supported features:
if (!features.support(myUdf.toLanguageFeature()))
	throw new Error("Impossible to support the UDF `" + myUdf + "`! This is the important point of this example file.");

@gmantele
Copy link
Owner

I have some classes that extend UserDefinedFunction, I added them all to the checker, and now I tried to add them to the parser, like in the example, but I get Unsupported ADQL feature:

if I do

System.out.println(udf.getFeatureDescription());
System.out.println(udf.getFunctionDefinition().toLanguageFeature()); 

I get

ivo://ivoa.net/std/TAPRegExt#features-udf!ESO_DATEADD_SEC() -> ?type?
ivo://ivoa.net/std/TAPRegExt#features-udf!ESO_DATEADD_SEC(seconds INTEGER, date TIMESTAMP) -> TIMESTAMP

whichever I add to the parser, I get the same error. I guess it doesn't like the prefix?

I guess something is indeed missing in your TapUserDefinedFunction. In this new version of ADQLLib, I have updated the class UserDefinedFunction. Maybe something I added is not present in your class extension.

@vforchi
Copy link
Contributor Author

vforchi commented Jan 10, 2024

I think I don't need TapUserDefinedFunction anymore.
I changed the class to:

package org.eso.dfs.tap.adql.udf;

import adql.query.ADQLObject;
import adql.query.operand.ADQLOperand;
import adql.query.operand.function.UserDefinedFunction;
import adql.translator.ADQLTranslator;
import adql.translator.TranslationException;

public class DateAddSecondsFunction extends UserDefinedFunction {

    public DateAddSecondsFunction(ADQLOperand[] params) throws NullPointerException {
        super("DateAddSecondsFunction", params);
    }

    public DateAddSecondsFunction(DateAddSecondsFunction toCopy) throws Exception {
        super(toCopy);
    }

    @Override
    public boolean isNumeric() {
        return false;
    }

    @Override
    public boolean isString() {
        return true;
    }

    @Override
    public boolean isGeometry() {
        return false;
    }

    @Override
    public String getName() {
        return "ESO_DATEADD_SEC";
    }

    @Override
    public ADQLObject getCopy() throws Exception {
        return new DateAddSecondsFunction(this);
    }

    @Override
    public String translate(ADQLTranslator caller) throws TranslationException {
        StringBuffer buf = new StringBuffer("dateadd(ss, ");
        buf.append(caller.translate(parameters.get(0)));
        buf.append(", ");
        buf.append(caller.translate(parameters.get(1)));
        buf.append(")");
        return buf.toString();
    }

}

But it still fails.
I don't need to keep this, and I can use the approach you suggested with FunctionDef.parse: I still have to add them to the checker, right?

@gmantele
Copy link
Owner

No more need to add this to the checker, only to the parser.

It actually did not make to set this kind of information in the DBChecker as it is not really table or column information, but just additional functions into the ADQL language. That's why I moved this responsibility directly into the parser itself (i.e. parser.getSupportedFeatures().support(myUdf.toLanguageFeature())).

I hope it does not break too much your code. The honest intent was really to simplify code.

@vforchi
Copy link
Contributor Author

vforchi commented Jan 10, 2024

I managed to make it work the same as before, but I still add to add them to the checker (I'm not actually 100% sure, I changed too many things too many times): if they are not needed there are you going to remove the constructor?

@gmantele
Copy link
Owner

Maybe. I will see that when I'll adapt TAPLib.

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