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 str_to_date function #2667

Merged
merged 5 commits into from
Sep 23, 2024
Merged

Fix str_to_date function #2667

merged 5 commits into from
Sep 23, 2024

Conversation

ktat
Copy link
Contributor

@ktat ktat commented Sep 18, 2024

STR_TO_DATE function cannot parse "%Y%m%d".
I mentioned it in the issue #2666

- Bug fix: cannot parse "%Y%m%d" format dolthub#2666
Copy link
Contributor

@jycor jycor left a comment

Choose a reason for hiding this comment

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

Thanks a lot for your contribution!
The fix looks great, just some small changes to the tests, and will be happy to merge this

sql/expression/function/str_to_date_test.go Outdated Show resolved Hide resolved
sql/expression/function/str_to_date_test.go Outdated Show resolved Hide resolved
sql/expression/function/str_to_date_test.go Outdated Show resolved Hide resolved
@ktat
Copy link
Contributor Author

ktat commented Sep 19, 2024

@jycor Thanks. I fiexed the code you mentioned.

Copy link
Contributor

@jycor jycor left a comment

Choose a reason for hiding this comment

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

Thanks for the changes
LGTM!

@ktat
Copy link
Contributor Author

ktat commented Sep 20, 2024

@jycor sorry I added additional commit. It fixes the bug of parsing "%Y%m%d%T" format.

Copy link
Contributor

@jycor jycor left a comment

Choose a reason for hiding this comment

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

Additional changes LGTM!
Thanks again!

sql/expression/function/str_to_date_test.go Show resolved Hide resolved
@jycor jycor merged commit aad5072 into dolthub:main Sep 23, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants