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

[Bug report] MysqlTypeConverter cannot handle the datetime data type. #1761

Closed
hzxiongyinke opened this issue Jan 29, 2024 · 7 comments · Fixed by #3477
Closed

[Bug report] MysqlTypeConverter cannot handle the datetime data type. #1761

hzxiongyinke opened this issue Jan 29, 2024 · 7 comments · Fixed by #3477
Assignees
Labels
bug Something isn't working

Comments

@hzxiongyinke
Copy link

hzxiongyinke commented Jan 29, 2024

Version

main branch

Describe what's wrong

Gravitino with mysql catalog, MysqlTypeConverter cannot handle the datetime data type.

Error message and/or stacktrace

image

How to reproduce

mysql> CREATE TABLE datetime_table ( id INT PRIMARY KEY, date_time DATETIME );
image

Additional context

I want to participate in fixing this bug. Can anyone give me some advice?

@hzxiongyinke hzxiongyinke added the bug Something isn't working label Jan 29, 2024
@FANNG1
Copy link
Contributor

FANNG1 commented Jan 29, 2024

@Clearvive @mchades could you provide some advise?

@Clearvive
Copy link
Contributor

image At present, the date time type is not supported and requires issue support. Can you do this @hzxiongyinke ?

@Clearvive
Copy link
Contributor

@hzxiongyinke Or you can check if we can use our timestamp type instead

@mchades
Copy link
Contributor

mchades commented Feb 1, 2024

Can you describe the differences between DATETIME type and TIMESTAMP type of MySQL? @hzxiongyinke

@hzxiongyinke
Copy link
Author

Can you describe the differences between DATETIME type and TIMESTAMP type of MySQL? @hzxiongyinke

For differences between MySQL DATETIME and TIMESTAMP types, refer to https://dev.mysql.com/doc/refman/8.0/en/datetime.html.
By the way, what rules does the data type design of Gravitino follow?

@mchades
Copy link
Contributor

mchades commented Feb 6, 2024

Can you describe the differences between DATETIME type and TIMESTAMP type of MySQL? @hzxiongyinke

For differences between MySQL DATETIME and TIMESTAMP types, refer to https://dev.mysql.com/doc/refman/8.0/en/datetime.html. By the way, what rules does the data type design of Gravitino follow?


Before designing the Gravitino type system, we referenced the type systems of other open-source engines such as Substrait, Spark, Hive, and Trino.

The design goal of the Gravitino type system is to provide a unified semantic and support user-defined types. As it stands, we are striving to cover the most commonly used types in data definition.

Regarding the timestamp type of Gravitino, as described in the documentation:

Timestamp type indicates a timestamp without timezone
Timestamp with timezone type indicates a timestamp with timezone

We should be mindful of the differences between datetime and timestamp in MySQL:

MySQL converts TIMESTAMP values from the current time zone to UTC for storage, and back from UTC to the current time zone for retrieval. (This does not occur for other types such as DATETIME.)

Therefore, based on the principle of unified semantics, I believe that we should:

  • map datetime of MySQL to timestamp of Gravitino
  • map timestamp of MySQL to timestamp_tz type of Gravitino.

Obviously, our current mapping of MySQL timestamp type conversion is not appropriate.

However, my main concern is: Gravitino's timestamp type and MySQL's timestamp type have the same name but different semantics. Will this potentially confuse users?

What do you think? @jerryshao @FANNG1 @Clearvive

@FANNG1
Copy link
Contributor

FANNG1 commented Feb 6, 2024

I think Gravitino should provide a clear and general type definition, the underlying catalog should follow this if there is conflict. and we should add the document to describe this.

  • clear means the user could easily know the type.
  • general means most systems follow this.

github-actions bot pushed a commit that referenced this issue May 22, 2024
)

### What changes were proposed in this pull request?

 - map `datetime` of MySQL to `timestamp` of Gravitino
 - map `timestamp` of MySQL to `timestamp_tz` type of Gravitino.

### Why are the changes needed?

unify the type semantics

Fix: #1761 

### Does this PR introduce _any_ user-facing change?

no

### How was this patch tested?

tests added
mchades added a commit that referenced this issue May 22, 2024
)

### What changes were proposed in this pull request?

 - map `datetime` of MySQL to `timestamp` of Gravitino
 - map `timestamp` of MySQL to `timestamp_tz` type of Gravitino.

### Why are the changes needed?

unify the type semantics

Fix: #1761 

### Does this PR introduce _any_ user-facing change?

no

### How was this patch tested?

tests added

Co-authored-by: mchades <liminghuang@datastrato.com>
diqiu50 pushed a commit to diqiu50/gravitino that referenced this issue Jun 13, 2024
…rt (apache#3477)

### What changes were proposed in this pull request?

 - map `datetime` of MySQL to `timestamp` of Gravitino
 - map `timestamp` of MySQL to `timestamp_tz` type of Gravitino.

### Why are the changes needed?

unify the type semantics

Fix: apache#1761 

### Does this PR introduce _any_ user-facing change?

no

### How was this patch tested?

tests added
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants