-
Notifications
You must be signed in to change notification settings - Fork 648
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
Create versioninfo table with primary key. #1743
Conversation
I would rather we not mix the unique constraint with the primary key constraint. Otherwise, I like the overall approach. Do you think you can refactor this so that the two are distinct? I do realize the drawback of my request is extra storage space for a second index, but as it is, without a primary key it is going to be stored as a heap, so it should actually be more efficient. |
@vaital3000 Hi, we have not heard back from you. Can you please update the PR and we can then merge it? |
@jzabroski Hi, sory, i did'n see your message without mention. I want to test you suggestion and will come back with fixes. |
Great. Again, no issue with the core concept to address mysql feature. Just think it should not be a breaking change. |
@jzabroski To ensure that the change is non-breaking, I added a property CreateWithPrimaryKey to IVersionTableMetaData with a default value of false. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@schambers @fubar-coder Do you see any downside to the default setting for CreateWithPrimaryKey being true? This would go into 6.0.0 ideally. I'm trying to think if any databases disallow combining unique constraints and primary keys and I don't know of any.
I think if anything, certain databases may throw an error if attempted to combine unique constraint and primary key on same column but perhaps we determine if thats the case during integration testing. This shouldn't cause any issues |
I should finish reviewing and most likely emerging this this week. It will go into 6.0.0 |
Should merge this tonight. 6.0.0 is getting closer. I am mainly trying to remove build warnings before calling 6.0.0 done. |
We use MySQL with option "
sql_require_primary_key
", so we can't use fluentmigrator now, because we can't create table without PK.I replaced VersionMigration.cs realization in my project and it's works fine, but it can be needed for somebody yet.