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

Optimize .mark_as_read! and extend to work on relations #95

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

fatkodima
Copy link
Contributor

@fatkodima fatkodima commented Jan 24, 2018

This PR optimizes .mark_as_read! to be roughly one query for collections and for relations.

Implements

Partly done by #93 - an optimized "one query" solution for scopes is still pending.

from #73 and fixes #80.

This is a an improvement over my previous PR #94, so probably that can be closed.

Implementation is based on INSERT ... ON CONFLICT DO ... like feature on modern databases (aka UPSERT) using gem upsert which uses native support of "upsert" or emulates if db does not provide it and allows to insert many records at once without db constraint violation.

PostgresSQL requires to have a unique constraint (just index is not sufficient) as so upsert gem, as stated on gem's readme so migration to add such was created.

TODO:

  • documentation

@coveralls
Copy link

coveralls commented Jan 24, 2018

Coverage Status

Coverage decreased (-0.8%) to 99.16% when pulling a75caf8 on fatkodima:optimize-mark_as_read into e3e9626 on ledermann:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.8%) to 99.16% when pulling a75caf8 on fatkodima:optimize-mark_as_read into e3e9626 on ledermann:master.

@fatkodima
Copy link
Contributor Author

@ledermann Rails now supports upsert natively, so no need of external gems.
Are you interested in this PR?

@ledermann
Copy link
Owner

@fatkodima Jepp, using native upsert from Rails 6.1 would be nice. But the current behavior (using external gem upsert) is still required for Rails < 6.1.

@fatkodima
Copy link
Contributor Author

We can optimize and use a native upsert for only rails >= 6.1, but ignore for < 6.1. Wdyt?

@fatkodima
Copy link
Contributor Author

@ledermann As you removed support for rails 6, wdyt on moving this forward?

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.

Slow N+1 query
3 participants