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

return type of UserDefinedFunction is not defined #130

Open
vforchi opened this issue Apr 28, 2021 · 6 comments
Open

return type of UserDefinedFunction is not defined #130

vforchi opened this issue Apr 28, 2021 · 6 comments
Assignees
Labels

Comments

@vforchi
Copy link
Contributor

vforchi commented Apr 28, 2021

I developed some custom functions by implementing UserDefinedFunction. One of these functions returns a Timestamp (a DATE in the signature), but when it is in one of the return columns the DBType is null.
I think this is because the ADQL parser only considers instances of DefaultUDF and not of UserDefinedFunction. I can't extend DefaultUDF, because it's final.

I can't easily get rid of the custom classes for each function, because I rely this in other parts of the code, is there any other way around it?

@gmantele
Copy link
Owner

gmantele commented May 6, 2021

Hi, sorry for the late answer.

I have some trouble to see what it the exact problem.

If I understand well, you've got a problem in queries like:

SELECT my_UDF_returning_a_timestamp(...)
FROM ....

Is it correct?

Do you have any error message that can help me better understand the issue?

When does this error occur? During the query parsing, while translating or when formatting the query result into VOTable (or any other format)?

@gmantele gmantele added the bug label May 6, 2021
@gmantele gmantele self-assigned this May 6, 2021
@vforchi
Copy link
Contributor Author

vforchi commented May 6, 2021

There is no error, it's just that the return type is not what I would expect, and it defaults to String

@gmantele
Copy link
Owner

gmantele commented May 6, 2021

So, the problem is in the result file returned by TAP ?
The value returned by the database is returned/considered by VOLLT/TAP as a String instead of a Date. Is that your problem?

@vforchi
Copy link
Contributor Author

vforchi commented May 6, 2021

Yes, correct

@gmantele
Copy link
Owner

gmantele commented May 6, 2021

Ok, then 2 things:

  1. You're right, the DBType of the returned column is NULL because I extract the info only from DefaultUDF. This latter is final and can not be extended. That is now clearly a design flaw. Right now, I am improving some things in the management of UDF in VOLLT/ADQL. What changes for you is that DefaultUDF will disappear and UserDefinedFunction will no longer be abstract. Both will be merged into UserDefinedFunction ; none function defined in there will be final. So for you, it should not change your extensions of UserDefinedFunction (but you will probably be able to make some clean up as most functions will already be implemented). Doing that will also fix this GitHub Issue, as there will no longer be any processing difference in the library between DefaultUDF and UserDefinedFunction. And so, a DBType will be associated with your column.

  2. However, this may not change the fact that, I assume, you get a String from the database. Your JDBC driver probably returns DB's date/timestamp as String. Currently, TAPLib format any java.sql.Date, java.sql.Time and java.util.Date returned by the JDBC driver into a String following ISO8601. In the current state, this processing does not apply for String, even if they are marked as date/timestamp in the TAP/ADQL metadata. I should probably do that anyway, though I will not know the format of date/timestamp returned/used by the DB/JDBC driver....but something by default could be done anyway.

In conclusion, with 1. fixed, there will be a DBType generated by the library and so there will datatype="char" arraysize="*" xtype="adql:TIMESTAMP" in the VOTable output for this column. But because of 2., I can not guarantee the format of the date/timestamp string in there.

Currently, I don't think there is anything simple you can do to fix (even temporarily) this issue, unless you want to change a core class of the library in a fork.

I will commit tomorrow (or beginning of next week) the merge of DefaultUDF and UserDefinedFunction as I am already working on it. So, if you want, I can notify you through this GitHub issue when it is available (and eventually generate a tap.jar for you). However, it will be commited in the branch adql-2.1 which is not guaranteed to be yet stable and so, may imply some changes on your side....but I am really not sure...it depends on what you have/use.

Anyway, did I answer to all your questions?

@vforchi
Copy link
Contributor Author

vforchi commented May 6, 2021

Yes, thanks. It was very clear. I will wait for adql 2.1 to be released.

I get a Timestamp from the DB, and I format it properly using STIL. The only thing that is wrong is the metadata.

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