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

Fix MySQL visibility indexes with close_time column #3927

Merged

Conversation

rodrigozhou
Copy link
Collaborator

@rodrigozhou rodrigozhou commented Feb 9, 2023

What changed?
Cast max datetime string to datetime type in MySQL COALESCE calls.
Refactor getCoalesceCloseTimeExpr to be part of pluginConverter interface.

Why?
MySQL coalesce with datetime and string results in a string, which can cause incorrect comparisons with datetime (eg: times 12:12:12 is different than 12:12:12.000).
So, adding a CAST to max datetime so it's handled correctly as datetime type column.
This also fixes flaky tests in TestMySQL8VisibilityPersistenceSuite

How did you test it?
Hard coded a time with leading zeros, and tests are now passing.

Potential risks
No risks.

Is hotfix candidate?
No.

@rodrigozhou rodrigozhou requested a review from a team as a code owner February 9, 2023 18:46
@rodrigozhou rodrigozhou marked this pull request as draft February 9, 2023 23:00
@rodrigozhou rodrigozhou changed the title Change MySQL datetime format to always have leading zeros Fix MySQL visibility indexes with close_time column Feb 9, 2023
@rodrigozhou rodrigozhou marked this pull request as ready for review February 9, 2023 23:45
@rodrigozhou rodrigozhou merged commit 77a4487 into temporalio:master Feb 10, 2023
@rodrigozhou rodrigozhou deleted the fix-mysql-datetime-format branch February 10, 2023 00:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants