SubmitUpdates Operation Order
devinlyons opened this issue · 11 comments
Is your feature request related to a problem? Please describe.
When inserting a record with child records, there is no guarantee that the child records will be created after the parent. This causes a database error.
Describe the solution you'd like
It would be good if there was some consistency in the order of the operations. Something like FIFO. Perhaps the data SubmitUpdates method could accept a ProcessingStrategy input.
Describe alternatives you've considered
N/A
Additional context
N/A
What if you call multiple times submitupdates? The disadvantage may be completing transaction too early?
Exactly. My hope was that I could queue up the operations and commit them all in a single transaction. Otherwise, I have to manually create the transaction. That's easy enough but there's no documentation on doing that with SQLProvider. The page on CRUD operations mentions it briefly but there is no code example and I can't find anything in the API documentation.
I went for manual transaction handling. In .NET the default TransactionScopeOption
is Required
meaning SQLProvider should just commit to a transaction if you have one.
This thread might help you a little bit: #238 (comment)
That's what I ended up doing. My code is working but I still think having the SubmitUpdates
method perform updates in a predictable manner is a worthwhile feature.
Anyway, thanks for the help!
Hello.
I highly agree with @devinlyons opinion, I think that having deterministic update order is really a must-have feature when working with foreign keys constraints.
Looking into the code of
Seq.sortBy (fun e -> e.Value)
in the pipeline before Seq.iter should be really straightforward and it allows to use an ordering of Create()
calls to ensure foreign-key friendly order of inserts.
What do you think @Thorium ?
I'm not familiar enough of the issue, so my point of view is pretty general:
Can this be a breaking change?
Can someone already trust the current order?
And what about the upsert things people are doing, (conflict strategy parameter). I have no idea of those?
https://fsprojects.github.io/SQLProvider/core/crud.html#OnConflict
Maybe this sorting should be also a new additional parameter, do it or not, and by default keep the existing behaviour?
But if we add that, there should be docs update, "when do you need it".
I don't think that ConcurrentDictionary enumerator has any ordering guarantees. Well, I guess it offers some kind of ordering unlike a fully random shuffle, but trusting to that order by anybody is meaningless. And I don't think that adding sortBy is a breaking change, since we're not touching any public APIs in any way.
Considering OnConflict strategy, at least in SQLite case, it supports only unique constraints or primary keys (doc)
I'm not sure if the code change suggested will fix the issue:
The pending changes dictionary is just keeping account in memory what updates are not yet processed to database. As far as I know TryRemove is not actually caring the relationships.
The reason I'm scared about sorting is that the sorted list is not concurrent anymore, so potential thread-safety issue?
What is the sorted "Value" anyway? Is it database entity values like Key="Age", Value="22' ? If so, what is the sort by value adding?
I'm not sure if the code change suggested will fix the issue:
The pending changes dictionary is just keeping account in memory what updates are not yet processed to database. As far as I know TryRemove is not actually caring the relationships.
The reason I'm scared about sorting is that the sorted list is not concurrent anymore, so potential thread-safety issue?
What is the sorted "Value" anyway? Is it database entity values like Key="Age", Value="22' ? If so, what is the sort by value adding?
Sorry, I misread the code and linked a wrong line 🙂 . Even though basic idea stands - we use already present DateTime.UTCNow (of moments when an entity was created) values of pendingChanges dictionary. I've made a draft PR to show one way of implementing this feature - #797. It will require similar changes to all provider implementations. Another way would be to sort key-value pairs before passing them to ProcessUpdates[Async], so we can keep sorting logic in one place. However, it would require changes in method signatures of ISqlProvider.
What do you think?
Generally looks good to me...
Generally looks good to me...
Thanks 🙂 . I will submit a full PR in the next few days then.