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

SQL: rename DataType.DATE to something else #36440

Closed
costin opened this issue Dec 10, 2018 · 3 comments
Closed

SQL: rename DataType.DATE to something else #36440

costin opened this issue Dec 10, 2018 · 3 comments

Comments

@costin
Copy link
Member

costin commented Dec 10, 2018

While respecting the Elasticsearch type naming, Es-SQL has a DATE type which in practice is a DateTime.
However this clashes with the SQL type DATE causing a lot of confusion especially when migrating.

Several options were discussed going forward:

  1. rename date

Rename date to datetime. This adds more meaning to the content however breaks the convention that it maps the elasticsearch date type.
On the upside there's no collision with the SQL types and allows the SQL date type if needed.

  1. rename date to timestamp

This is a variant of 1 however it follows the SQL terminology which might be both confusing and clear.

  1. don't expose ES types in the queries

Prevent the problem from exposing by not allowing the user to create the ES data types (typically through CAST - CAST ('1' AS KEYWORD). This will impact testing and be asymmetric since such types are returned as results. Further more, the confusion around date remains since it shows up in the table description.

@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search

@costin
Copy link
Member Author

costin commented Dec 10, 2018

/cc @eskibars

@astefan astefan self-assigned this Jan 10, 2019
@astefan astefan added the v6.7.0 label Jan 10, 2019
@astefan astefan assigned matriv and unassigned astefan Jan 10, 2019
@costin
Copy link
Member Author

costin commented Jan 11, 2019

FTR, the plan is to rename DATE to DATETIME to better indicate its structure while keeping it short. This also doesn't clash with any other data type and while it won't match the naming in Elasticsearch, it won't clash with SQL DATE either.

matriv added a commit to matriv/elasticsearch that referenced this issue Jan 14, 2019
SQL data type DATE has only the date part (e.g.: 2019-01-14)
without any time information. Previously the SQL type DATE was
referring to the ES DATE which contains also the time part along
with TZ information. To conform with SQL data types the data type
`DATE` is renamed to `DATETIME`, since it includes also the time,
as a new runtime SQL `DATE` data type will be introduced down the road,
which only contains the date part and meets the SQL standard.

Closes: elastic#36440
matriv added a commit that referenced this issue Jan 17, 2019
* SQL: Rename SQL data type DATE to DATETIME

SQL data type DATE has only the date part (e.g.: 2019-01-14)
without any time information. Previously the SQL type DATE was
referring to the ES DATE which contains also the time part along
with TZ information. To conform with SQL data types the data type
`DATE` is renamed to `DATETIME`, since it includes also the time,
as a new runtime SQL `DATE` data type will be introduced down the road,
which only contains the date part and meets the SQL standard.

Closes: #36440

* Address comments
matriv added a commit that referenced this issue Jan 17, 2019
* SQL: Rename SQL data type DATE to DATETIME

SQL data type DATE has only the date part (e.g.: 2019-01-14)
without any time information. Previously the SQL type DATE was
referring to the ES DATE which contains also the time part along
with TZ information. To conform with SQL data types the data type
`DATE` is renamed to `DATETIME`, since it includes also the time,
as a new runtime SQL `DATE` data type will be introduced down the road,
which only contains the date part and meets the SQL standard.

Closes: #36440

* Address comments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants