Skip to content
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

Unify all bulk inserts to a single code path #610

Merged
merged 1 commit into from
Sep 23, 2024
Merged

Conversation

bgentry
Copy link
Contributor

@bgentry bgentry commented Sep 23, 2024

I set out to unify all code paths including single inserts, but ultimately realized it's a bit too messy to do that until we get rid of advisory lock unique inserts. The reason is that the current multi-insert code structure drops the dbunique.UniqueOpts on the floor where it generates insert args, which isn't super close to the point where the actual insert operation happens (and thus where we could branch out to the old unique inserter path). Ultimately I felt it wasn't worth trying to do that because in one more point release I think we can safely remove the advisory lock unique inserts.

For now, I've unified all multi-insert paths to use the same code for generating insert options, emitting notifications, etc, with each separate top level group (regular insert many vs fast/copy) passing a function for the actual insert using a common API structure.

I think if you want to integrate #584 into this it makes sense to just focus on this unified multi insert code path, and we can ship middleware in the same release where we remove advisory lock uniqueness.

@bgentry
Copy link
Contributor Author

bgentry commented Sep 23, 2024

There are probably other ways of achieving this deduplication and I'm open to trying other approaches. However I think I'm convinced that this is a good idea overall—we should have a lot less risk of bugs if most of the core functionality lives in a single code path to the extent it can. Going to merge this and move forward with shipping a proper v0.12.0 (non-rc).

@bgentry bgentry merged commit 9be1d11 into master Sep 23, 2024
14 checks passed
@bgentry bgentry deleted the bg-unified-insert-path branch September 23, 2024 19:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant