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

`repsertMany` does not match `mapM_ (uncurry repsert)` #832

Closed
naushadh opened this Issue Sep 30, 2018 · 0 comments

Comments

Projects
None yet
1 participant
@naushadh
Contributor

naushadh commented Sep 30, 2018

Issue

repsert works like so:

  • Find record by ID
  • If found, replace record for ID
  • else, insert record with ID

repsertMany works like so:

  • Find records by Ids
  • new ids = given ids diff found ids
  • insert records matching new ids

The following step is missing in repsertMany to make it symmetric with mapM_ (uncurry repsert):

  • replace records matching found ids

Issue was introduced at the very conception of the feature, and there are no tests to verify this API.

Suggested fix

Since there is no batch means to add the missing step mentioned above, we should perhaps let each driver implement its own repsertMany natively akin to putMany; i.e., create a new field in SqlBackend called connRepsertManySql before letting each backend define it.

Bonus, this will actually address race condition errors/deadlocks arising from non-atomic client side operations (SELECT + INSERT) on the same table. I've run into parallel insert errors with repsertMany but never with the natively/atomically implemented putMany using Postgres 10.

@psibi psibi closed this in #833 Oct 5, 2018

psibi added a commit that referenced this issue Oct 5, 2018

Merge pull request #833 from naushadh/repsertMany-atomic
Fix #832: `repsertMany == mapM_ (uncurry repsert)` and is atomic.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment