Return all columns not in both insert and update columns when doing upsert #1306
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I would argue that a model should be completely up to date with the database after an upsert, however as demonstrated in #1296 this is currently not always the case. This PR is a proposed fix for this. By returning all columns not present in both the insert and update column sets we make sure that the model is always up to date with the database after an upsert, no matter if an insert or update was performed.
As
updateColumns
andinsertColumns
are bothColumns
structs that might have different types (infer, whitelist etc) I did not find it suitable to add a method on theColumns
struct to calculate this and do it different depending on type. Instead opting to just always calculate the return columns the same way.Although the biggest issue in my example in #1296 was that the ID (primary key) was not updated when generated in go code and not in the database, I still think all properties on the model should be updated to match the database when an upsert is performed. Hence not just adding a check to make sure primary keys are in the return set.
Happy to hear feedback on this!
Note: This PR also relies on the
SetIntersect()
method being added to the strmangle package. See volatiletech/strmangle#11