search

jmoiron / modl

golang database modelling library
MIT License
479 stars 48 forks source link

Use existing nonzero values in autoincr fields instead of overwriting with new autoincr value #29

Open sqs opened 9 years ago

sqs commented 9 years ago

Previously, if a field mapped to an auto-incremented column, its value was thrown away during inserts and was replaced by the auto-incremented value reported by the database. This commit changes that behavior. Now, if the field value is nonzero, the existing value is inserted into the database and the auto-incrementing sequence is not used. If the field value is 0, the previous behavior still applies.

This breaks code that expects modl to overwrite nonzero autoincr fields with the next value from the auto-incrementing sequence.

Motivation: When writing tests for code that uses modl, it's nice to be able to insert rows with a known ID, as opposed to having to insert it and then read it back to see the ID. Also, if you need to insert something with a known ID in your application, you need to use raw SQL (or create a new DbMap without the key's autoincr prop set) because there is currently no way to do so in modl.

All tests pass for me (PostgreSQL, MySQL, SQLite3) :white_check_mark:

Issues/questions for @jmoiron:

jmoiron commented 9 years ago

I'm going to look into this tomorrow, specifically answering question 1 and figuring out the ways it might break existing code so I can document it. Superficially, it seems like the behavioral change of just using the supplied ID is the only thing, but I'll look into it a bit deeper. Great change though, I totally agree with this behavior.

sqs commented 9 years ago

Just a friendly follow-up to see if you've had a chance to look into those things.

jmoiron commented 9 years ago

Sorry for the delay @sqs! I'm going on holiday in a few days which actually means I'll have plenty of downtime to really go through this and make sure it's all right.