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

support microseconds resolution for created_at/updated_at/deleted_at #2512

Merged
merged 19 commits into from
Mar 20, 2023
Merged

support microseconds resolution for created_at/updated_at/deleted_at #2512

merged 19 commits into from
Mar 20, 2023

Conversation

wesleywu
Copy link
Contributor

@wesleywu wesleywu commented Mar 9, 2023

We often need microseconds resolution for timestamp/datetime columns, especially when we sort records by timestamp column and do some batch processing.

The current implementation in GF uses String() representation of gtime/time for insert/update operations, thus truncating the time precision to second.

@codecov-commenter
Copy link

codecov-commenter commented Mar 9, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: -0.02 ⚠️

Comparison is base (1cd1449) 78.62% compared to head (19d0c19) 78.61%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2512      +/-   ##
==========================================
- Coverage   78.62%   78.61%   -0.02%     
==========================================
  Files         630      629       -1     
  Lines       51318    51158     -160     
==========================================
- Hits        40349    40217     -132     
+ Misses       8984     8963      -21     
+ Partials     1985     1978       -7     
Flag Coverage Δ
go-1.15-386 ?
go-1.15-amd64 78.63% <100.00%> (-0.02%) ⬇️
go-1.16-386 ?
go-1.16-amd64 ?
go-1.17-386 ?
go-1.17-amd64 ?
go-1.18-386 78.58% <100.00%> (-0.01%) ⬇️
go-1.18-amd64 78.59% <100.00%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
contrib/drivers/dm/dm.go 73.25% <100.00%> (+0.12%) ⬆️
database/gdb/gdb_model_delete.go 100.00% <100.00%> (ø)
database/gdb/gdb_model_insert.go 64.35% <100.00%> (ø)
database/gdb/gdb_model_update.go 68.47% <100.00%> (+0.34%) ⬆️

... and 15 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@wesleywu
Copy link
Contributor Author

wesleywu commented Mar 10, 2023

MySQL 5.7 does not recoginze +07:00 in timestamp string, while MySQL 8.0/PostgreSQL/MSSQL/Sqlite/Dameng do.

update demo_table
set updated_at = '2023-03-10 13:19:03.142798-09:00'  -- supported since MySQL 8.0
where id =  '01186883-7690'

So we have to use gtime.Now().Local().Time.Format("2006-01-02 15:04:05.999999") instead of gtime.Now().Time.Format("2006-01-02 15:04:05.999999-09:00") when setting updated_at/deleted_at as string.

This implemantion also requires the DB server(or docker container/host os) other than MySQL running at the same timezone as the client, otherwise the timestamp might be updated with wrong timezone (mostly UTC).

@Issues-translate-bot
Copy link

Bot detected the issue body's language is not English, translate it automatically. 👯👭🏻🧑‍🤝‍🧑👫🧑🏿‍🤝‍🧑🏻👩🏾‍🤝‍👨🏿👬🏿


MySQL 5.7 does not recognize +07:00 in timestamp string, while MySQL 8.0 can.

@wesleywu
Copy link
Contributor Author

wesleywu commented Mar 10, 2023

Confirmed all DBMS drivers implemented in contrib/drivers can correctly parse string with format '2023-03-10 13:19:03.999999' into datetime/timestamp types.
I think this feature can be safely merged.
@gqcn

contrib/drivers/dm/dm.go Outdated Show resolved Hide resolved
@houseme houseme requested a review from gqcn March 11, 2023 02:38
contrib/drivers/dm/dm.go Outdated Show resolved Hide resolved
Copy link
Contributor Author

@wesleywu wesleywu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

restore the default organization of imports

@wesleywu wesleywu requested a review from gqcn March 12, 2023 11:53
@gqcn gqcn added the ready to merge Used in PR, which means this PR is reviewed. label Mar 13, 2023
@houseme
Copy link
Member

houseme commented Mar 13, 2023

restore the default organization of imports

The unit test needs to add a test that is not six decimal places to verify that it can be written properly.

database/gdb/gdb_model_delete.go Outdated Show resolved Hide resolved
database/gdb/gdb_model_update.go Outdated Show resolved Hide resolved
wumengye added 2 commits March 14, 2023 13:37
# Conflicts:
#	contrib/drivers/README.MD
@wesleywu wesleywu requested a review from gqcn March 14, 2023 05:40
@wesleywu
Copy link
Contributor Author

现在应该比较完美了

@Issues-translate-bot
Copy link

Bot detected the issue body's language is not English, translate it automatically. 👯👭🏻🧑‍🤝‍🧑👫🧑🏿‍🤝‍🧑🏻👩🏾‍🤝‍👨🏿👬🏿


It should be perfect now

@wesleywu
Copy link
Contributor Author

The unit test needs to add a test that is not six decimal places to verify that it can be written properly.

test added.

@gqcn
Copy link
Member

gqcn commented Mar 20, 2023

@wesleywu Awesome!

@gqcn gqcn merged commit e721124 into gogf:master Mar 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement ready to merge Used in PR, which means this PR is reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants