-
Notifications
You must be signed in to change notification settings - Fork 962
feat(mysql): Implement cast function parser #2473
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
Conversation
|
Hi and thanks! I haven't looked at this closely yet but regarding the testing question I think the best thing would be to create a new endtoend test in either To generate the expected test output (from your patch) you'll naturally need a dev build of sqlc with your patch in it. You can build using You can run all the endtoend tests with |
|
I've added the test, however I'm not really sure if I did it right:
|
|
The tests look great, thanks! You don't need to add the Regarding the actual code, I think there is a subtle change in behavior introduced by removing the switch statement in I think we can get this merged for the next release though, so I'm adding it to the 1.20 milestone. |
|
Done, I've reverted the |
|
I think this looks good. @kyleconroy if you agree feel free to merge or comment if not. |
… postgres tests for same Related to discussion in #2473 (comment)
… postgres tests for same (#2551) Related to discussion in #2473 (comment)
What is this As the title said, this PR wants to add support for CAST function in MySQL. This PR is based from PR by @ryanpbrewster here (which unfortunately he didn't send here, and only exist in his repository). Why is this PR created Currently sqlc unable to infer the correct type from SQL function like MAX, MIN, SUM, etc. For those function, sqlc will return its value as interface{}. This behavior can be seen in this playground. As workaround, it advised to use CAST function to explicitly tell what is the type for that column, as mentioned in sqlc-dev#1574. Unfortunately, currently sqlc only support CAST function in PostgreSQL and not in MySQL. Thanks to this, right now MySQL users have to parse the interface{} manually, which is not really desirable. What does this PR do? Implement convertFuncCast function for MySQL. Add better nil pointer check in some functions that related to convertFuncCast. I haven't write any test because I'm not sure how and where to put it. However, as far as I know the code that handle ast.TypeCast for PostgreSQL also don't have any test, so I guess it's fine 🤷♂️ Related issues Support CAST ... AS sqlc-dev#687, which currently is the oldest MySQL issue that still opened. Using MYSQL functions ( CONVERT and CAST) result in removing column from struct sqlc-dev#1622 Unable to Type Alias sqlc-dev#1866 sum in select result in model field type interface{} sqlc-dev#1901 MIN() returns an interface{} sqlc-dev#1965
… postgres tests for same (sqlc-dev#2551) Related to discussion in sqlc-dev#2473 (comment)
What is this
As the title said, this PR wants to add support for
CASTfunction in MySQL.This PR is based from PR by @ryanpbrewster here (which unfortunately he didn't send here, and only exist in his repository).
Why is this PR created
Currently
sqlcunable to infer the correct type from SQL function likeMAX,MIN,SUM, etc. For those function, sqlc will return its value asinterface{}. This behavior can be seen in this playground.As workaround, it advised to use
CASTfunction to explicitly tell what is the type for that column, as mentioned in #1574.Unfortunately, currently sqlc only support
CASTfunction in PostgreSQL and not in MySQL. Thanks to this, right now MySQL users have to parse theinterface{}manually, which is not really desirable.What does this PR do?
convertFuncCastfunction for MySQL.convertFuncCast.I haven't write any test because I'm not sure how and where to put it. However, as far as I know the code that handle
ast.TypeCastfor PostgreSQL also don't have any test, so I guess it's fine 🤷♂️Related issues