From e315092ee38e7c16005cafa9045042c8307224b0 Mon Sep 17 00:00:00 2001 From: phiresky Date: Thu, 27 Jul 2023 23:36:51 +0200 Subject: [PATCH] remove n^2 part of person triggers, improve community aggregate trigger (#3739) * remove n^2 part of person triggers, improve community aggregate trigger * comment out comment_score tests since previously they only accidentally succeeded * empty --- .../src/aggregates/person_aggregates.rs | 12 ++- .../down.sql | 80 +++++++++++++++++++ .../up.sql | 47 +++++++++++ 3 files changed, 135 insertions(+), 4 deletions(-) create mode 100644 migrations/2023-07-27-134652_remove-expensive-broken-trigger/down.sql create mode 100644 migrations/2023-07-27-134652_remove-expensive-broken-trigger/up.sql diff --git a/crates/db_schema/src/aggregates/person_aggregates.rs b/crates/db_schema/src/aggregates/person_aggregates.rs index 43feadd45..2e71844fa 100644 --- a/crates/db_schema/src/aggregates/person_aggregates.rs +++ b/crates/db_schema/src/aggregates/person_aggregates.rs @@ -161,7 +161,8 @@ mod tests { .await .unwrap(); assert_eq!(0, after_parent_comment_removed.comment_count); - assert_eq!(0, after_parent_comment_removed.comment_score); + // TODO: fix person aggregate comment score calculation + // assert_eq!(0, after_parent_comment_removed.comment_score); // Remove a parent comment (the scores should also be removed) Comment::delete(pool, inserted_comment.id).await.unwrap(); @@ -172,7 +173,8 @@ mod tests { .await .unwrap(); assert_eq!(0, after_parent_comment_delete.comment_count); - assert_eq!(0, after_parent_comment_delete.comment_score); + // TODO: fix person aggregate comment score calculation + // assert_eq!(0, after_parent_comment_delete.comment_score); // Add in the two comments again, then delete the post. let new_parent_comment = Comment::create(pool, &comment_form, None).await.unwrap(); @@ -186,13 +188,15 @@ mod tests { .await .unwrap(); assert_eq!(2, after_comment_add.comment_count); - assert_eq!(1, after_comment_add.comment_score); + // TODO: fix person aggregate comment score calculation + // assert_eq!(1, after_comment_add.comment_score); Post::delete(pool, inserted_post.id).await.unwrap(); let after_post_delete = PersonAggregates::read(pool, inserted_person.id) .await .unwrap(); - assert_eq!(0, after_post_delete.comment_score); + // TODO: fix person aggregate comment score calculation + // assert_eq!(0, after_post_delete.comment_score); assert_eq!(0, after_post_delete.comment_count); assert_eq!(0, after_post_delete.post_score); assert_eq!(0, after_post_delete.post_count); diff --git a/migrations/2023-07-27-134652_remove-expensive-broken-trigger/down.sql b/migrations/2023-07-27-134652_remove-expensive-broken-trigger/down.sql new file mode 100644 index 000000000..1a5c4ec4f --- /dev/null +++ b/migrations/2023-07-27-134652_remove-expensive-broken-trigger/down.sql @@ -0,0 +1,80 @@ +create or replace function person_aggregates_comment_count() + returns trigger language plpgsql +as $$ +begin + IF (was_restored_or_created(TG_OP, OLD, NEW)) THEN + update person_aggregates + set comment_count = comment_count + 1 where person_id = NEW.creator_id; + ELSIF (was_removed_or_deleted(TG_OP, OLD, NEW)) THEN + update person_aggregates + set comment_count = comment_count - 1 where person_id = OLD.creator_id; + + -- If the comment gets deleted, the score calculation trigger won't fire, + -- so you need to re-calculate + update person_aggregates ua + set comment_score = cd.score + from ( + select u.id, + coalesce(0, sum(cl.score)) as score + -- User join because comments could be empty + from person u + left join comment c on u.id = c.creator_id and c.deleted = 'f' and c.removed = 'f' + left join comment_like cl on c.id = cl.comment_id + group by u.id + ) cd + where ua.person_id = OLD.creator_id; + END IF; + return null; +end $$; + +create or replace function person_aggregates_post_count() + returns trigger language plpgsql +as $$ +begin + IF (was_restored_or_created(TG_OP, OLD, NEW)) THEN + update person_aggregates + set post_count = post_count + 1 where person_id = NEW.creator_id; + + ELSIF (was_removed_or_deleted(TG_OP, OLD, NEW)) THEN + update person_aggregates + set post_count = post_count - 1 where person_id = OLD.creator_id; + + -- If the post gets deleted, the score calculation trigger won't fire, + -- so you need to re-calculate + update person_aggregates ua + set post_score = pd.score + from ( + select u.id, + coalesce(0, sum(pl.score)) as score + -- User join because posts could be empty + from person u + left join post p on u.id = p.creator_id and p.deleted = 'f' and p.removed = 'f' + left join post_like pl on p.id = pl.post_id + group by u.id + ) pd + where ua.person_id = OLD.creator_id; + + END IF; + return null; +end $$; + +create or replace function community_aggregates_comment_count() + returns trigger language plpgsql +as $$ +begin + IF (was_restored_or_created(TG_OP, OLD, NEW)) THEN +update community_aggregates ca +set comments = comments + 1 from comment c, post p +where p.id = c.post_id + and p.id = NEW.post_id + and ca.community_id = p.community_id; +ELSIF (was_removed_or_deleted(TG_OP, OLD, NEW)) THEN +update community_aggregates ca +set comments = comments - 1 from comment c, post p +where p.id = c.post_id + and p.id = OLD.post_id + and ca.community_id = p.community_id; + +END IF; +return null; +end $$; \ No newline at end of file diff --git a/migrations/2023-07-27-134652_remove-expensive-broken-trigger/up.sql b/migrations/2023-07-27-134652_remove-expensive-broken-trigger/up.sql new file mode 100644 index 000000000..66a78371b --- /dev/null +++ b/migrations/2023-07-27-134652_remove-expensive-broken-trigger/up.sql @@ -0,0 +1,47 @@ +create or replace function person_aggregates_comment_count() + returns trigger language plpgsql +as $$ +begin + IF (was_restored_or_created(TG_OP, OLD, NEW)) THEN + update person_aggregates + set comment_count = comment_count + 1 where person_id = NEW.creator_id; + ELSIF (was_removed_or_deleted(TG_OP, OLD, NEW)) THEN + update person_aggregates + set comment_count = comment_count - 1 where person_id = OLD.creator_id; + END IF; + return null; +end $$; + +create or replace function person_aggregates_post_count() + returns trigger language plpgsql +as $$ +begin + IF (was_restored_or_created(TG_OP, OLD, NEW)) THEN + update person_aggregates + set post_count = post_count + 1 where person_id = NEW.creator_id; + + ELSIF (was_removed_or_deleted(TG_OP, OLD, NEW)) THEN + update person_aggregates + set post_count = post_count - 1 where person_id = OLD.creator_id; + END IF; + return null; +end $$; + +create or replace function community_aggregates_comment_count() + returns trigger language plpgsql +as $$ +begin + IF (was_restored_or_created(TG_OP, OLD, NEW)) THEN +update community_aggregates ca +set comments = comments + 1 from post p +where p.id = NEW.post_id + and ca.community_id = p.community_id; +ELSIF (was_removed_or_deleted(TG_OP, OLD, NEW)) THEN +update community_aggregates ca +set comments = comments - 1 from post p +where p.id = OLD.post_id + and ca.community_id = p.community_id; + +END IF; +return null; +end $$; \ No newline at end of file