-
Notifications
You must be signed in to change notification settings - Fork 883
Allow for mixing parameter styles #816
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
e41fe16
to
d501a90
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the patch! I agree that it doesn't make much sense to require a single parameter style. A few request:
- please pick a more descriptive name for the tests, such as
sqlc_arg_mixed
orparams_mixed
. - Move the current tests into a
postgresql
subdirectory and add the same test for MySQL
Thanks for the feedback. (One thing I did notice is if you have a query as follows, the numbers always precede the sqlc.args convetion)
|
This would be great fix and would be awesome if merged. There is an outstanding issue w.r.t using named_parameter for LIMIT for MySQL. We can use this fix to sort of work around it in the short term, but for the below: I applied the patch and tried it for the query:
The generated code looks like this:
Aren't the orders of parameters arg.Limit and arg.MinBookID switched? |
Yeah I kind of realized that was an issue but, with postgresql you can use dollar numbers format so it is not an issue and I did not think of how it would work with mysql. I can look into fixing it but I really do not have the time. |
Understandable. I also don't have the time to look into fixing this. @dant4301 If you'd like this feature, you're going to have to get your hands dirty and take a shot at fixing this PR. I'm going to close this for now, but feel free to fork this branch and open up another PR if you can get it to work. |
@ovadbar, I do not use sqlc now since I did a limited amount of codegen just enough for my needs. Thanks again though. |
This should allow for mixed styles of parameters.