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

remove n^2 part of person triggers, improve community aggregate trigger #3739

Merged
merged 3 commits into from Jul 27, 2023

Conversation

phiresky
Copy link
Collaborator

@phiresky phiresky commented Jul 27, 2023

This PR has two changes:

  1. it removes the O(n^2) part of the person_aggregates_comment_count and person_aggregates_post_count triggers. This part has never actually worked due to the use of coalesce(0, ...). See SQL Trigger/function 'person_aggregates_comment_count' is broken #2910 . In order to fix the function and not make it be too expensive, the triggers would have to get an enforced order or be merged into one trigger (some effort in post UPDATE trigger focused on all the combined actions of a byebye a… #3727, but incomplete so far)
  2. it removes an unnecessary select in the community trigger that loops through all comments on a post on post delete (see https://gist.github.com/phiresky/402999658552f70af2ade151bc8ded34 for a comparison in query plans)

This is part of the solution to #3528. After this and #3732, the only expensive trigger remaining should be community_aggregates_post_count.

@phiresky
Copy link
Collaborator Author

I've commented out the person_aggregates comment_score test part, because that only ever accidentally succeeded - the comment delete in that test deletes the only comment, so a resulting score of 0 is correct. But in reality, a delete always sets the score to 0 even if it shouldn't be.

@Nutomic Nutomic merged commit e315092 into LemmyNet:main Jul 27, 2023
1 check passed
dessalines pushed a commit that referenced this pull request Jul 27, 2023
…er (#3739)

* remove n^2 part of person triggers, improve community aggregate trigger

* comment out comment_score tests since previously they only accidentally succeeded

* empty
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.

None yet

3 participants