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

Add upsert_multi #119

Merged
merged 3 commits into from
Jul 12, 2024
Merged

Add upsert_multi #119

merged 3 commits into from
Jul 12, 2024

Conversation

feluelle
Copy link
Contributor

@feluelle feluelle commented Jul 4, 2024

Description

This is about enabling users to upsert multiple models/rows.

Changes

  • add upsert_multi to FastCRUD using dialect-optimized sql
  • add psycopg dependency for executing postgres statements via sqlalchemy
  • add testcontainers to run tests in a ephemeral postgres container

closes: #110

Tests

  • add postgres-specific upsert_multi test
  • add sqlite-specific upsert_multi test
  • add mysql-specific upsert_multi test

Checklist

  • I have read the CONTRIBUTING document.
  • My code follows the code style of this project.
  • I have added necessary documentation (if appropriate).
  • I have added tests that cover my changes (if applicable).
  • All new and existing tests passed.

@feluelle
Copy link
Contributor Author

feluelle commented Jul 4, 2024

cc @JakNowy @igorbenav

@JakNowy
Copy link
Contributor

JakNowy commented Jul 4, 2024

This is great, I'm 100% for this postgresql approach in other cases.

@feluelle feluelle force-pushed the feature/upsert-multi branch 2 times, most recently from bdaa922 to cda717e Compare July 5, 2024 11:40
@feluelle feluelle marked this pull request as ready for review July 5, 2024 12:13
@feluelle
Copy link
Contributor Author

feluelle commented Jul 5, 2024

@JakNowy @igorbenav PTAL

@feluelle feluelle marked this pull request as draft July 5, 2024 12:44
@feluelle feluelle marked this pull request as ready for review July 5, 2024 13:01
@igorbenav
Copy link
Owner

Code looks good. Please add a db agnostic implementation as well and change the error for a warning, something like f"Upsert multi is not implemented for {db.bind.dialect.name}. Defaulting to slow db agnostic implementation."

@feluelle
Copy link
Contributor Author

feluelle commented Jul 9, 2024

Sure, can do that.

@feluelle
Copy link
Contributor Author

feluelle commented Jul 9, 2024

Hmm, I thought about it but instead added more support for sqlite and mysql/mariadb.

I think adding slow upsert using the current upsert method is not an option. It doesn't scale.

@igorbenav
Copy link
Owner

Nice one, thanks!

@igorbenav igorbenav merged commit d1cd6db into igorbenav:main Jul 12, 2024
7 checks passed
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.

upsert multi
3 participants