Change stance on SQL dialect support #130
Conversation
This greatly softens the language around SQL support. A short list of functions are specified as SHOULD implement, rather than a long list of MUST implement. The expectation is that we will revise this list over time as usage dictates. This addresses issue ga4gh-discovery#128.
|
I significantly reduced the set of functions here based on two criteria:
Please, push back on a removal if you think it's critical that it be done in query as opposed to after query. |
|
Thanks @kemp-verily. I think this is a good base, but there are probably a few more that seem at the same level of importance to me, and we've used them in examples before. How do you feel about The broader question is - do you feel strongly about removing the remaining functions completely vs. leaving them in as optional, maybe with a |
SPEC.md
Outdated
|
|
||
| * ga4gh_type (described above) | ||
| * **Logical Operators** | ||
| * `AND`, `OR`, `NOT` | ||
| * **Comparison Operators** | ||
| * `<`, `>`, `<=`, `>=`, `=`, `<>`, `!=` |
There was a problem hiding this comment.
I've just noticed < is not escaped correctly.
There was a problem hiding this comment.
I changed this just to < since it shouldn't be escaped at all AFAIK.
There was a problem hiding this comment.
I've highlighted some of the functions and operators that I've found indispensable so far while working with phenotypic data in the context of Data Connect. I have certainly used other functions too.
What would you think about a MUST/SHOULD/MAY system where the very small set you propose (perhaps with the addition of NOW() or CURRENT_TIMESTAMP()), but then a SHOULD with more functions and operators and finally a MAY with the most.
My concern is that in accommodating non-SQL-backed implementations, we shouldn't actively encourage diversity in operator and function names. Without guidance and with the ANSI SQL spec being somewhat hard to get, people will naturally make diverse choices when implementing the non-mandatory functions.
| * `uuid()*` | ||
| * **Session Information Functions** | ||
| * `current_user`* | ||
| * `IF` |
There was a problem hiding this comment.
If I could only have one conditional expression, I'd want CASE. I can't think of a time I've seen IF in a SQL query, but CASE pops up quite a bit. I'm not aware of any incompatibilities between SQL implementations here.
COALESCE isn't 100% necessary, but it's part of ANSI SQL and because of the way SQL handles nulls, it's often important to have.
There was a problem hiding this comment.
Replaced IF with CASE.
There was a problem hiding this comment.
Added COALESCE and IF both actually
| * **String manipulation** | ||
| * **Operators:** | ||
| * `Concatenation (||)`* |
There was a problem hiding this comment.
I think we will need a way to splice strings together
There was a problem hiding this comment.
Added, was an oversight!
| * `current_date` | ||
| * `current_time` | ||
| * `current_timestamp` | ||
| * `current_timestamp(p)`* |
There was a problem hiding this comment.
We'll need these (or now()) along with date arithmetic to convert a date of birth to an age at time of observation.
There was a problem hiding this comment.
Sorry, of course I should have said date of birth and observation date.
|
filed #134 to track the regex work |
This greatly softens the language around SQL support. A short list of
functions are specified as SHOULD implement, rather than a long list of MUST
implement.
The expectation is that we will revise this list over time as usage dictates.
This addresses issue #128.