-
Notifications
You must be signed in to change notification settings - Fork 50
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
Transactions + foreign key constraints #230
Comments
Thanks for bringing this up. Can you describe the issue in more detail where the current logic is insufficient? And perhaps give an example? |
Thanks for getting back to me. So here is the artificial simplest example:
When we go into SyncChanges/SyncChanges/Synchronizer.cs Line 513 in 334c791
And since https://github.com/mganss/SyncChanges/blob/334c79173372483e039a87b5c89b9bd89661527c/SyncChanges/Synchronizer.cs#L504C17-L504C100 sorts by Table.Name - then only table names that determines will the inserts work or fail with
It will get more complex when there are UPDATES or DELETES involved. |
Thanks a lot for providing the example. I think the fix you propose wouldn't work, unfortunately, because for some cases there does not exist an order of changes that would satisfy the foreign key constraints at every step. This is because some information about the original insert value is lost, see the explanation here: https://github.com/mganss/SyncChanges?tab=readme-ov-file#foreign-key-constraints My current idea is to disable all foreign key constraints that are involved in changes that occur during the same transaction, i.e. where creation version and version are equal. Do you think there are cases where this would not suffice? Also, my understanding is that the issue is only with inserts and subsequent updates to the same row. The updates do not show up as updates per se but are merged into the initial insert but with the value of the last update. Do you have an example where deletes or updates would also create issues? It's a shame that SQL Server doesn't support deferred constraints, esp. considering SQL Server's own change tracking feature creates this issue that would require them: https://stackoverflow.com/a/5974847/1970064 |
Thank you, I don't know this deep enough to see all the consequences.
Sounds like it could work. I am dealing with very active databases with sometimes a lot of foreign keys - so I went for simpler solution - calculate involved keys for all the changes (but don't check actual values). Then disable all of them and re-enable at the end. I don't have any other examples I am afraid, since I needed to solve the issue asap, so I took shortcuts. https://github.com/ValtsS/SyncChanges/blob/64c2934fd227adb943d78a17109e522e5c169bb0/SyncChanges/Synchronizer.cs#L643 P.S. Also, my fork requires snapshot support, because I think without it there are chances of data inconsistency otherwise. |
OK thanks for your input. I'm gonna go with the idea of disabling foreign key constraints for all changes in a transaction for now. |
Recently I ran into an issue where the current logic of the foreign key constraint check enable/disable is not sufficient.
Why this happens - ComputeForeignKeyConstraintsToDisable checks only for changes somewhere in the future and does the workaround by disabling constraints and re-enabling them afterwards.
To solve this issue in my fork I turn off all the keys involved in the changes - this is not a great approach, because in my case there are a lot of them.
What would be the correct approach - group the changes together by createVersion & version (thus they are in a single transaction). Then do a topological sort (graph of FKs) to reorder changes correctly, taking into account is it DELETE or INSERT or UPDATE. I struggle to come up with such logic at this time.
The text was updated successfully, but these errors were encountered: