[CALCITE-6624] SqlParser should parse MySQL DATETIME type #4002
Conversation
dssysolyatin
force-pushed
the
feature/support-mysql-datetime-type-parsing
branch
from
b24d053 to
86e204b
Compare
| @@ -6182,6 +6182,12 @@ SqlTypeNameSpec DateTimeTypeName() : | |||
| { | |||
| return new SqlBasicTypeNameSpec(typeName, precision, s.end(this)); | |||
| } | |||
| | | |||
There was a problem hiding this comment.
is this supposed to be in Babel?
There was a problem hiding this comment.
Regarding Babel, my initial plan was to implement DATETIME datatype parsing at the Babel parser level, but I noticed that we have DATETIME literal parsing for BigQuery inside Parser.jj, which is also valid for MySQL. Therefore, I decided to put DATETIME type parsing in Parser.jj, as it would be better to have both DATETIME datatype parsing and DATETIME literal parsing in the same place.
dssysolyatin
force-pushed
the
feature/support-mysql-datetime-type-parsing
branch
2 times, most recently
from
492a2ae to
2833f46
Compare
dssysolyatin
force-pushed
the
feature/support-mysql-datetime-type-parsing
branch
from
2833f46 to
42206a9
Compare
dssysolyatin
changed the title
|
Please retry analysis of this Pull-Request directly on SonarCloud |
|
|
||
| !ok | ||
|
|
||
| SELECT cast(cast(TIMESTAMP'9999-12-31 23:59:59' as DATETIME) as DATETIME); |
There was a problem hiding this comment.
Can you also write DATETIME '1000-01-01 00:00:00'?
Are you supposed to?
There was a problem hiding this comment.
MySQL syntax is kind of messy
-
Time literals can only be defined using DATE 'str', TIME 'str', or TIMESTAMP 'str'. There is no syntax like DATETIME 'str'.
-
The TIMESTAMP syntax produces a DATETIME value in MySQL because DATETIME has a range that more closely corresponds to the standard SQL TIMESTAMP type, which has a year range from 0001 to 9999. (The MySQL TIMESTAMP year range is 1970 to 2038.)
Currently, SELECT DATETIME '1000-01-01 00:00:00' works in Calcite when using the MySQL dialect. Ideally, it should throw an exception. However, I’m not sure if anyone actually needs this behavior or if it’s worth spending time on.
mihaibudiu
changed the title
|
If we decide that this is a useful feature, the PR is fine. |
|
Anyone else wants to comment on this issue? |
No description provided.