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

return non-fatal NoFieldInTypeError when selecting too many columns (with tests) #188

Merged
merged 1 commit into from
Aug 25, 2014

Conversation

vadimg
Copy link
Contributor

@vadimg vadimg commented Jul 27, 2014

We are using gorp in production with a medium-sized team (10 developers + 3 contractors). Our deploys go like this:

  1. Migrate the db
  2. Push the new code

The issue is that, in between the first and second step, we potentially have more database columns than fields in a struct. If someone wrote a query with select *, that query will now error, and the API will go down.

This isn't a problem if no one ever does select *, but we are a startup and work long hours, sometimes pushing code when we're tired. Not all lines of code will be reviewed, not everything will be thought out all the time, and slip-ups are all but guaranteed. And as we rapidly add more people to the team, this behavior becomes very dangerous for the stability of our production system.

I understand that some people might want the current behavior, and the error should definitely not be silently swallowed. In the case of too many columns, the code in this pull request will return a specific type of error, NoFieldInTypeError, but otherwise proceed normally. The caller can check if the error is a NoFieldInTypeError, or just use gorp.NonFatalError(err) to see if the program can continue.

@coopernurse coopernurse added this to the v1.7 milestone Aug 25, 2014
@coopernurse coopernurse merged commit 6078ff2 into go-gorp:develop Aug 25, 2014
@coopernurse
Copy link
Contributor

Please test your application against the develop branch. I've merged several PRs in. Please open a new ticket if you run into any problems. If no problems are reported by Aug 30 I'll merge to master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants