Improve performance of site_aggregates and person_aggregates triggers

api_tests/src/user.spec.ts

@@ -17,6 +17,7 @@ import {
   resolveComment,
   saveUserSettingsFederated,
   setupLogins,
+  followCommunity,
 } from "./shared";
 import { LemmyHttp } from "lemmy-js-client";
 import { GetPosts } from "lemmy-js-client/dist/types/GetPosts";
@@ -90,6 +91,19 @@ test("Delete user", async () => {
     .comment;
   expect(remoteComment).toBeDefined();
 
+  // subscribe to alpha community from user
+  await followCommunity(user, true, alphaCommunity.community.id);
+
+  // for aggregate checking, refresh community before account delete
+  let beforeAlphaCommunity = (
+    await resolveCommunity(alpha, "!main@lemmy-alpha:8541")
+  ).community;
+  if (!beforeAlphaCommunity) {
+    throw "Missing alpha community";
+  }
+  // also getSite
+  let beforeSite = await getSite(alpha);
+
   await deleteUser(user);
 
   await expect(resolvePost(alpha, localPost)).rejects.toBe(
@@ -104,6 +118,46 @@ test("Delete user", async () => {
   await expect(resolveComment(alpha, remoteComment)).rejects.toBe(
     "couldnt_find_object",
   );
+  // try to use the account that is now deleted
+  await expect(resolveBetaCommunity(user)).rejects.toBe("deleted");
+
+  // let alpha view the aftermath
+  let afterAlphaCommunity = (
+    await resolveCommunity(alpha, "!main@lemmy-alpha:8541")
+  ).community;
+  if (!afterAlphaCommunity) {
+    throw "Missing alpha community";
+  }
+  let afterSite = await getSite(alpha);
+
+  // possible bug in 0.18.2
+  // Does lemmy_server do a SQL DELETE of a person or only mark the deleted=true column? This trigger does not seem to actually fire:
+  //    CREATE TRIGGER site_aggregates_person_delete AFTER DELETE ON public.person FOR EACH ROW WHEN ((old.local = true)) EXECUTE FUNCTION public.site_aggregates_person_delete();
+  //  expect(beforeSite.site_view.counts.users).toBe(afterSite.site_view.counts.users + 1);
+
+  console.log(
+    "%d vs %d and %d vs %d",
+    beforeSite.site_view.counts.posts,
+    afterSite.site_view.counts.posts,
+    beforeAlphaCommunity?.counts.subscribers,
+    afterAlphaCommunity?.counts.subscribers,
+  );
+  expect(beforeSite.site_view.counts.posts).toBe(
+    afterSite.site_view.counts.posts + 2,
+  );
+  expect(beforeSite.site_view.counts.comments).toBe(
+    afterSite.site_view.counts.comments + 2,
+  );
+  expect(beforeAlphaCommunity?.counts.comments).toBe(
+    afterAlphaCommunity?.counts.comments + 1,
+  );
+  expect(beforeAlphaCommunity?.counts.posts).toBe(
+    afterAlphaCommunity?.counts.posts + 1,
+  );
+
+  // possible bug in 0.18.2
+  // does deleting a user account unsubscribe from communities?
+  //  expect(beforeAlphaCommunity?.counts.subscribers).toBe(afterAlphaCommunity?.counts.subscribers + 1);
 });
 
 test("Requests with invalid auth should be treated as unauthenticated", async () => {

crates/db_schema/src/aggregates/site_aggregates.rs

@@ -3,13 +3,18 @@ use crate::{
   schema::site_aggregates,
   utils::{get_conn, DbPool},
 };
-use diesel::result::Error;
+use diesel::{result::Error, ExpressionMethods, QueryDsl};
 use diesel_async::RunQueryDsl;
 
 impl SiteAggregates {
+  /// reads the site aggregate for site_id = 1 (the local site)
   pub async fn read(pool: &mut DbPool<'_>) -> Result<Self, Error> {
+    use site_aggregates::dsl::site_id;
     let conn = &mut get_conn(pool).await?;
-    site_aggregates::table.first::<Self>(conn).await
+    site_aggregates::table
+      .filter(site_id.eq(1))
+      .first::<Self>(conn)
+      .await
   }
 }
 
@@ -29,8 +34,10 @@ mod tests {
       site::{Site, SiteInsertForm},
     },
     traits::Crud,
-    utils::{build_db_pool_for_tests, DbPool},
+    utils::{build_db_pool_for_tests, get_conn, DbPool},
   };
+  use diesel::sql_query;
+  use diesel_async::RunQueryDsl;
   use serial_test::serial;
 
   async fn prepare_site_with_community(
@@ -53,6 +60,15 @@ mod tests {
       .instance_id(inserted_instance.id)
       .build();
 
+    {
+      let conn = &mut get_conn(pool).await.unwrap();
+
+      // reset the site sequence since parts of the code assume that the local site (the only one for which site_aggregates are updated) has id 1
+      sql_query("ALTER SEQUENCE site_id_seq RESTART WITH 1;")
+        .execute(conn)
+        .await
+        .unwrap();
+    }
     let inserted_site = Site::create(pool, &site_form).await.unwrap();
 
     let new_community = CommunityInsertForm::builder()

crates/db_schema/src/aggregates/structs.rs

@@ -134,6 +134,8 @@ pub struct PersonPostAggregatesForm {
 #[cfg_attr(feature = "full", diesel(belongs_to(crate::source::site::Site)))]
 #[cfg_attr(feature = "full", ts(export))]
 /// Aggregate data for a site.
+/// Warning: site aggregate data is currently only set / updated for the local site
+/// The local site is assumed to always have site_id = 1
 pub struct SiteAggregates {
   pub id: i32,
   pub site_id: SiteId,

migrations/2023-07-23-182318_site_aggregates_local_only/down.sql

@@ -0,0 +1,147 @@
+-- This file undoes what is in `up.sql`
+-- to ensure no confusion with all the migrations
+--    this was mostly created by dumping from PostgreSQL 15.3 schema
+
+DROP INDEX idx_site_aggregates_site_id;
+
+CREATE OR REPLACE FUNCTION  site_aggregates_comment_delete() RETURNS trigger
+    LANGUAGE plpgsql
+    AS $$
+begin
+    IF (was_removed_or_deleted(TG_OP, OLD, NEW)) THEN
+        update site_aggregates sa
+        set comments = comments - 1
+        from site s
+        where sa.site_id = s.id;
+    END IF;
+    return null;
+end $$;
+
+
+
+CREATE OR REPLACE FUNCTION  site_aggregates_comment_insert() RETURNS trigger
+    LANGUAGE plpgsql
+    AS $$
+begin
+    IF (was_restored_or_created(TG_OP, OLD, NEW)) THEN
+        update site_aggregates sa
+        set comments = comments + 1
+        from site s
+        where sa.site_id = s.id;
+    END IF;
+    return null;
+end $$;
+
+
+
+CREATE OR REPLACE FUNCTION  site_aggregates_community_delete() RETURNS trigger
+    LANGUAGE plpgsql
+    AS $$
+begin
+        IF (was_removed_or_deleted(TG_OP, OLD, NEW)) THEN
+        update site_aggregates sa
+        set communities = communities - 1
+        from site s
+        where sa.site_id = s.id;
+    END IF;
+    return null;
+end $$;
+
+
+
+CREATE OR REPLACE FUNCTION  site_aggregates_community_insert() RETURNS trigger
+    LANGUAGE plpgsql
+    AS $$
+begin
+    IF (was_restored_or_created(TG_OP, OLD, NEW)) THEN
+        update site_aggregates sa
+        set communities = communities + 1
+        from site s
+        where sa.site_id = s.id;
+    END IF;
+    return null;
+end $$;
+
+
+
+CREATE OR REPLACE FUNCTION  site_aggregates_person_delete() RETURNS trigger
+    LANGUAGE plpgsql
+    AS $$
+begin
+  -- Join to site since the creator might not be there anymore
+  update site_aggregates sa
+  set users = users - 1
+  from site s
+  where sa.site_id = s.id;
+  return null;
+end $$;
+
+
+
+CREATE OR REPLACE FUNCTION  site_aggregates_person_insert() RETURNS trigger
+    LANGUAGE plpgsql
+    AS $$
+begin
+  update site_aggregates 
+  set users = users + 1;
+  return null;
+end $$;
+
+
+
+CREATE OR REPLACE FUNCTION  site_aggregates_post_delete() RETURNS trigger
+    LANGUAGE plpgsql
+    AS $$
+begin
+    IF (was_removed_or_deleted(TG_OP, OLD, NEW)) THEN
+        update site_aggregates sa
+        set posts = posts - 1
+        from site s
+        where sa.site_id = s.id;
+    END IF;
+    return null;
+end $$;
+
+
+
+CREATE OR REPLACE FUNCTION  site_aggregates_post_insert() RETURNS trigger
+    LANGUAGE plpgsql
+    AS $$
+begin
+    IF (was_restored_or_created(TG_OP, OLD, NEW)) THEN
+        update site_aggregates sa
+        set posts = posts + 1
+        from site s
+        where sa.site_id = s.id;
+    END IF;
+    return null;
+end $$;
+
+
+
+CREATE OR REPLACE FUNCTION  site_aggregates_activity(i text) RETURNS integer
+    LANGUAGE plpgsql
+    AS $$
+declare
+   count_ integer;
+begin
+  select count(*)
+  into count_
+  from (
+    select c.creator_id from comment c
+    inner join person u on c.creator_id = u.id
+    inner join person pe on c.creator_id = pe.id
+    where c.published > ('now'::timestamp - i::interval) 
+    and u.local = true
+    and pe.bot_account = false
+    union
+    select p.creator_id from post p
+    inner join person u on p.creator_id = u.id
+    inner join person pe on p.creator_id = pe.id
+    where p.published > ('now'::timestamp - i::interval)
+    and u.local = true
+    and pe.bot_account = false
+  ) a;
+  return count_;
+end;
+$$;