Skip to content

MySQL: support lowercase date types and min and max on date types #4243

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

Merged
merged 3 commits into from
Jun 8, 2023

Conversation

shellderp
Copy link
Contributor

@shellderp shellderp commented Jun 7, 2023

MySQL supports lowercase type names, and all other types are allowed in lowercase, so lets support TIMESTAMP and friends in lowercase.

Also brought back support for min/max functions on timestamps, which was working in 1.* version

@shellderp shellderp marked this pull request as ready for review June 7, 2023 16:55
@shellderp shellderp force-pushed the mgersh.timestamp-minmax-lower branch 2 times, most recently from 7abaa8b to 2955310 Compare June 7, 2023 18:22
@shellderp
Copy link
Contributor Author

hey @hfhbd can you please take a look, I'm unable to add reviewers.

@hfhbd
Copy link
Collaborator

hfhbd commented Jun 7, 2023

I am not against lower case support, I am just surprised it works in some cases which is somehow confusing because we don't have general lowercase support.

Copy link
Collaborator

@hfhbd hfhbd left a comment

Choose a reason for hiding this comment

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

Other changes are okay, nice.

@shellderp
Copy link
Contributor Author

Shrug, we just use lowercase in our codebase, and it seems to work in all the cases. I can't kick off the build or merge btw.

@shellderp shellderp force-pushed the mgersh.timestamp-minmax-lower branch from 182ef95 to 8dd27d3 Compare June 7, 2023 22:25
@hfhbd hfhbd merged commit b577579 into sqldelight:master Jun 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants