IDENTITY opt-in config Issue: #233 #715
Conversation
|
I like this! The one thing I would like to change is that instead of storing at the top level of the snapshot, we should store within each attribute by overriding the type. We can set the type to |
|
I believe I removed everything storing it as a snapshot and so it should now be storing as the attribute like you requested, let me know if anything needs tweaking. |
| length(types) == 1 -> | ||
| hd(types) | ||
|
|
||
| :identity in types -> |
There was a problem hiding this comment.
This doesn't seem right to me. The resources need to agree on this otherwise it should be an error.
There was a problem hiding this comment.
This looks like the only issue, after that we can get it merged! 🙇
i ran mix format by accident and then when I was going back to revert everything to how it was this slipped through the cracks.
|
Things should be good to go now, let me know if there’s anything else! |
| source = attribute.source || attribute.name | ||
|
|
||
| type = | ||
| if source in identity_column_defaults and attribute.generated? and type in [:bigint, :integer] do |
There was a problem hiding this comment.
Okay, sorry one more thing 😢. Now that we are just setting the column type to :identity, there is no real reason we need a new DSL option here. You should just be able to do migration_types: [id: :identity] and then you're good to go. No need for identity_column_defaults 😄
Also, be sure to run mix spark.cheat_sheets, mix format etc. locally.
|
I removed the DSL/identity_column_defaults logic, and altered the tests to not test for that logic either. |
2 participants
Contributor checklist
Leave anything that you believe does not apply unchecked.
Refactored some of the functions to opt-in to using identity columns using the identity_columns_default DSL rather than simply using serial/bigserial columns. Also created migration tests to test opting for identity columns in various scenarios as well as falling back to serial when identity is not opted into.