Skip to content

fix(plugin-search): delete does not also delete the search doc #12148

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

Merged
merged 2 commits into from
Apr 18, 2025

Conversation

DanRibbens
Copy link
Contributor

@DanRibbens DanRibbens commented Apr 17, 2025

The plugin-search collection uses an afterDelete hook to remove search records from the database. Since a deleted document in postgres causes cascade updates for the foreign key, the query for the document by relationship was not returning the record to be deleted.

The solution was to change the delete hook to beforeDelete for the search enabled collections. This way we purge records before the main document so the search document query can find and delete the record as expected.

An alternative solution in #9623 would remove the req so the delete query could still find the document, however, this just works outside of transactions which isn't desirable.

fixes #9443

@DanRibbens DanRibbens marked this pull request as ready for review April 18, 2025 04:16
@DanRibbens DanRibbens changed the title test(plugin-search): test incorrectly passing in postgres fix(plugin-search): delete does not also delete the search doc Apr 18, 2025
@DanRibbens DanRibbens requested a review from r1tsuu April 18, 2025 04:35
Copy link
Member

@r1tsuu r1tsuu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, I was thinking that there could be de synchronization if we use beforeDelete, but if the transaction fails this should be aborted as well, since we pass the req.

@akhrarovsaid
Copy link
Contributor

Looks great! Yeah, this was the solution I ended up on as well - albeit took me much longer to realize what was actually happening with the transaction/doc not being surfaced haha. There is an added little improvement here in omitting an extra find and just using a where in the delete which is nice!

I was directing users to this solution in the meantime. Super happy to see it finally getting resolved here! Thanks for your work.

@DanRibbens DanRibbens merged commit df7a369 into main Apr 18, 2025
82 checks passed
@DanRibbens DanRibbens deleted the fix/postgres-db-search branch April 18, 2025 13:47
Copy link
Contributor

🚀 This is included in version v3.36.0

kendelljoseph pushed a commit that referenced this pull request May 15, 2025
The plugin-search collection uses an `afterDelete` hook to remove search
records from the database. Since a deleted document in postgres causes
cascade updates for the foreign key, the query for the document by
relationship was not returning the record to be deleted.

The solution was to change the delete hook to `beforeDelete` for the
search enabled collections. This way we purge records before the main
document so the search document query can find and delete the record as
expected.

An alternative solution in #9623 would remove the `req` so the delete
query could still find the document, however, this just works outside of
transactions which isn't desirable.

fixes #9443
kendelljoseph pushed a commit that referenced this pull request May 19, 2025
The plugin-search collection uses an `afterDelete` hook to remove search
records from the database. Since a deleted document in postgres causes
cascade updates for the foreign key, the query for the document by
relationship was not returning the record to be deleted.

The solution was to change the delete hook to `beforeDelete` for the
search enabled collections. This way we purge records before the main
document so the search document query can find and delete the record as
expected.

An alternative solution in #9623 would remove the `req` so the delete
query could still find the document, however, this just works outside of
transactions which isn't desirable.

fixes #9443
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Search records don't get deleted when related document is deleted
3 participants