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
cmd/vet: warn when sql queries don't have the right amount of arguments #19471
Comments
CC @dominikh. At a glance I didn't see a staticcheck check for this, but maybe it'd be appropriate there, if not in vet itself. |
Or maybe https://github.com/stripe/safesql that's already focused on checking SQL? |
Vet shouldn't be checking statements from another language. It's a Go tool. It seems there is plenty of opportunity - and existing tools - for checking SQL queries, but for my money vet is not the place for them. |
@dgryski the stripe package is great but it's designed for security. Here I'm suggesting more of a basic check on the number of arguments provided to the @robpike I agree but I'm not asking to validate statement from other languages, just the number of arguments sent. Looking at the last 4 years of using Go in production it has been our number one cause of random (and stupid) bugs. I'm pretty confident others are in the same camp. I could certainly create a sql vet tool or make sure safesql supports that check. But most new gophers wouldn't know about the issue until it's too late (and wouldn't know about the existence of safesql or my new tool). |
How would be the correct number of |
@mattetti You realize that means vet would need an SQL parser. Even leaving aside the dialect issue, that seems a bridge too far. |
@robpike I didn't realize that, I thought placeholders ( |
|
@mattetti I don't think you'll have much luck. SQL is too complex just to move the error from I did a code generation for postgres SQL queries yak shave recently. Because of the complexity, embedded languages, pl/pgsql, pl/perl, execute statements (sql within sql) the only practical and somewhat reliable way I found to parse postgres SQL for bind values was to throw it at the databases own parser using C bindings and traverse the AST. |
@mattetti Just to confirm this, I ran your example through |
Sounds like this isn't going to happen. Closing. |
I'd like to see go vet warn me if I'm crafting a sql query using placeholders and the number of arguments isn't matching. This is a very common issue that obviously isn't caught by the compiler. Ideally, it would work similarly to the way printf vetting works.
Example:
The above line should get a go vet warning letting me know that 2 arguments are passed when only 1 is expected. The chance of bug increases as the number of placeholder increases such as in an update or insert.
The text was updated successfully, but these errors were encountered: