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

resolve TBD on how to use interval type in ADQL #44

Open
pdowler opened this issue Nov 17, 2020 · 8 comments
Open

resolve TBD on how to use interval type in ADQL #44

pdowler opened this issue Nov 17, 2020 · 8 comments
Labels
enhancement New feature or request

Comments

@pdowler
Copy link

pdowler commented Nov 17, 2020

At some point in the past. we discussed defining an "overlaps" function. The semantics are simple but finding a good name... not so much.

One possibility we discussed was overloading INTERSECTS with two arguments, but I don't know what impact that would have on grammar complexity.

I do not have any current use case to justify a "contains" function, but it is equally easy to define.

@pdowler
Copy link
Author

pdowler commented Nov 17, 2020

We have implemented and make use of INTERSECTS(<interval>, <interval>) = 1 in several CADC TAP services. We support columns of type interval so generally this is used with one column ref and one constant defined with the INTERVAL function.

@gmantele
Copy link
Collaborator

I do not remember this discussion, but being now aware of it, this TBD makes sense for me.
So, what was the result of this past discussion about interval overlaps?

@gmantele

This comment has been minimized.

@gmantele
Copy link
Collaborator

Sorry for my last comment/proposal. I naively thought that interval notion was about time instead of just numeric values. Proof that I was not yet really aware about this new datatype in ADQL/DALI and that I answered a bit too quickly 😖

@gmantele gmantele added the enhancement New feature or request label Nov 18, 2020
@gmantele gmantele added this to the 2.1 milestone Nov 18, 2020
@gmantele gmantele pinned this issue Nov 25, 2020
@gmantele
Copy link
Collaborator

With this fresh new year, I sense there is more thinking needed here. Here are my current personal thoughts on this topic:

  • Interval can easily be confused with time interval (and maybe other kinds of interval).

    In PostgreSQL, MySQL and MS-SQLServer, the datatype INTERVAL is related to time ; it is generally the type used to express the difference between two dates/times.

    Then, using the keyword INTERVAL for numeric values might be ambiguous later when we will need to use time interval (especially now that the time domain topic is raising in the VO).

  • Postgres has a datatype named "RANGE" which can be declined in function of the values to represent. There are for instance: int4range, numrange, tsrange (for timestamp), daterange, ...

    I rather like this term (range) instead of interval....

    ...but I can not find anything similar for MySQL and MS-SQLServer, though I assume it can easily be reproduced by something like an array or a string (with a coma to separate interval/range bounds). If we go for this solution, we would first have to ensure that it works fine with MySQL and MS-SQLServer which do not support arrays.

  • INTERSECTS (and CONTAINS) are for the moment defined for a geometrical purpose.

    We will have to make clear that they can apply to something else (which should not be a problem, I think).

Considering these, my current feeling is that we should wait for next version of ADQL (2.2 or 3.0).

But these are, of course, preliminary thoughts. Any more comments, opinions are welcome.

@msdemlei
Copy link
Contributor

msdemlei commented Jan 13, 2021 via email

@gmantele
Copy link
Collaborator

I agree about the fact that it is now too late to change the terminology ; we will deal with interval as we can. Sorry, I thought about that but I forgot to write it...

The thing is that at some point we will have to deal with time interval and at that moment we will have to figure out another name/terminology/trick (e.g. use a prefix like time_)....anyway, it will probably be the topic of another discussion.

However, as far as I can see, ADQL won't need any keyword for the
type (unless we want to let people cast to intervals).

But I fear it is going to happen. At some point, we may have to create intervals inside an ADQL query. CAST can be used in this intent, but probably a constructor as well (but that's another topic... #11 )...in anyway, the term interval will have to be explicitly used.

Agreed -- it's not ultra-nice that we introduce more items that need
type introspection in the translator, but it's not the end of the
world either.

Agreed.

@Zarquan
Copy link
Member

Zarquan commented Jan 13, 2021

Considering these, my current feeling is that we should wait for next version of ADQL (2.2 or 3.0).

Yes, definitely. We should have frozen ADQL 2.1 five years ago.

Absolutely, freeze it now, no new functionality, get ADQL 2.1 done and then we can move on to the PEG grammar.
New functionality will be much easier to consider after we have moved to the PEG grammar.

@gmantele gmantele modified the milestones: 2.1, future-version Jan 13, 2021
@gmantele gmantele unpinned this issue Jan 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants