fix(sql): fix comparing Date columns with date literals #3862
Conversation
Consider this query: `select * from tab where date = '1970-01-01'`
Without these new functions QuestDB resorts to picking the overloaded
equal function which accepts 2 longs. Why?
1. There is no exact match for eq(Date, Date) - such function does not exist at all!
2. The date column is `DateColumn` and the Date type can be casted to Long.
See the overloading priority: /* 7 DATE */, {DATE, TIMESTAMP, LONG}
3. The second parameter is `StrConstant` and String can be also casted to Long.
Again, the overloading priority: /* 11 STRING */, {STRING, CHAR, DOUBLE, LONG, INT, FLOAT, SHORT, BYTE}
4. Hence, the function parser picks eq(Long, Long).
But then the `StrFunction.getLong()` fails to parse '1970-01-01' as a long.
It expects a simple number. It does not know it's supposed to treat the argument as a Date.
The same query works if you change the column type from Date to Timestamp.
Why? Timestamp does not rely on casting - it implements Timestamp-specific
functions where necessary. So I copied this for Date.
treat such timestamps as UTC
…tterns also: more tests for Date casting
|
@jerrinot this is a solid PR! I do think tho that title should be a "fix" or "feat". It is user facing problem. Also description needs a succint TLDR |
|
@bluestreak01 thanks. I think "fix" is the more appropriate categorization. As most users will perceive the current behaviour as buggy. |
jerrinot
changed the title
|
@jerrinot is this draft draft or ready to go draft? |
|
@bluestreak01: I would like to add some more tests, I believe @marregui also wanted to add some tests.
|
|
hi @bluestreak01 package io.questdb.griffin.engine.functions.cast is a led zepelin. In the long term we would benefit from generating such classes on compile time, the full extent, or found an alternative approach. I will add a few tests, and will double check the approach, but if it works well, would you agree that it should be enough? |
for =() only. for now.
This reverts commit 08a6963.
|
@bluestreak01: I tried the experiment we discussed the other day: I introduced Consider this table: |
[PR Coverage check]😍 pass : 153 / 155 (98.71%) file detail
|
Consider this query:
select * from tab where date = '1970-01-01'Without these new functions QuestDB resorts to picking the overloaded equal function which accepts 2 longs. Why?
DateColumnand the Date type can be casted to Long.See the overloading priority: /* 7 DATE */, {DATE, TIMESTAMP, LONG}
StrConstantand String can be also casted to Long.Again, the overloading priority: /* 11 STRING */, {STRING, CHAR, DOUBLE, LONG, INT, FLOAT, SHORT, BYTE}
But then the
StrFunction.getLong()fails to parse '1970-01-01' as a long. It expects a simple number. It does not know it's supposed to treat the argument as a Date.The same query works if you change the column type from Date to Timestamp. Why? Timestamp does not rely on casting - it implements Timestamp-specific functions where necessary. So I copied this for Date.