gmantele / vollt

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

register user defined functions #151

Open vforchi opened 8 months ago

vforchi commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

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

gmantele commented 8 months ago

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 commented 8 months ago

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

vforchi commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

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 commented 7 months ago

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