getAllStories: Remove nested queries

ts/sql/Server.ts

@@ -2933,44 +2933,50 @@ function getAllStories(
     sourceServiceId?: ServiceIdString;
   }
 ): GetAllStoriesResultType {
-  const rows: ReadonlyArray<{
-    json: string;
-    hasReplies: number;
-    hasRepliesFromSelf: number;
-  }> = db
-    .prepare<Query>(
-      `
-      SELECT
-        json,
-        (SELECT EXISTS(
-          SELECT 1
-          FROM messages as replies
-          WHERE replies.storyId IS messages.id
-        )) as hasReplies,
-        (SELECT EXISTS(
-          SELECT 1
-          FROM messages AS selfReplies
-          WHERE
-            selfReplies.storyId IS messages.id AND
-            selfReplies.type IS 'outgoing'
-        )) as hasRepliesFromSelf
+  const [storiesQuery, storiesParams] = sql`
+    SELECT json, id
     FROM messages
     WHERE
-        type IS 'story' AND
-        ($conversationId IS NULL OR conversationId IS $conversationId) AND
-        ($sourceServiceId IS NULL OR sourceServiceId IS $sourceServiceId)
+      isStory = 1 AND
+      (${conversationId} IS NULL OR conversationId IS ${conversationId}) AND
+      (${sourceServiceId} IS NULL OR sourceServiceId IS ${sourceServiceId})
     ORDER BY received_at ASC, sent_at ASC;
-      `
+  `;
+  const rows: ReadonlyArray<{
+    id: string;
+    json: string;
+  }> = db.prepare(storiesQuery).all(storiesParams);
+
+  const [repliesQuery, repliesParams] = sql`
+    SELECT DISTINCT storyId
+    FROM messages
+    WHERE storyId IS NOT NULL
+  `;
+  const replies: ReadonlyArray<{
+    storyId: string;
+  }> = db.prepare(repliesQuery).all(repliesParams);
+
+  const [repliesFromSelfQuery, repliesFromSelfParams] = sql`
+    SELECT DISTINCT storyId
+    FROM messages
+    WHERE (
+      storyId IS NOT NULL AND
+      type IS 'outgoing'
     )
-    .all({
-      conversationId: conversationId || null,
-      sourceServiceId: sourceServiceId || null,
-    });
+  `;
+  const repliesFromSelf: ReadonlyArray<{
+    storyId: string;
+  }> = db.prepare(repliesFromSelfQuery).all(repliesFromSelfParams);
+
+  const repliesLookup = new Set(replies.map(row => row.storyId));
+  const repliesFromSelfLookup = new Set(
+    repliesFromSelf.map(row => row.storyId)
+  );
 
   return rows.map(row => ({
     ...jsonToObject(row.json),
-    hasReplies: row.hasReplies !== 0,
-    hasRepliesFromSelf: row.hasRepliesFromSelf !== 0,
+    hasReplies: Boolean(repliesLookup.has(row.id)),
+    hasRepliesFromSelf: Boolean(repliesFromSelfLookup.has(row.id)),
   }));
 }
 

ts/sql/migrations/1130-isStory-index.ts

@@ -0,0 +1,31 @@
+// Copyright 2024 Signal Messenger, LLC
+// SPDX-License-Identifier: AGPL-3.0-only
+
+import type { Database } from '@signalapp/better-sqlite3';
+
+import type { LoggerType } from '../../types/Logging';
+
+export const version = 1130;
+
+export function updateToSchemaVersion1130(
+  currentVersion: number,
+  db: Database,
+  logger: LoggerType
+): void {
+  if (currentVersion >= 1130) {
+    return;
+  }
+
+  db.transaction(() => {
+    // This is to improve the performance of getAllStories
+    db.exec(`
+      CREATE INDEX messages_isStory
+        ON messages(received_at, sent_at)
+        WHERE isStory = 1;
+    `);
+  })();
+
+  db.pragma('user_version = 1130');
+
+  logger.info('updateToSchemaVersion1130: success!');
+}

ts/sql/migrations/index.ts

@@ -88,10 +88,11 @@ import { updateToSchemaVersion1080 } from './1080-nondisappearing-addressable';
 import { updateToSchemaVersion1090 } from './1090-message-delete-indexes';
 import { updateToSchemaVersion1100 } from './1100-optimize-mark-call-history-read-in-conversation';
 import { updateToSchemaVersion1110 } from './1110-sticker-local-key';
+import { updateToSchemaVersion1120 } from './1120-messages-foreign-keys-indexes';
 import {
-  updateToSchemaVersion1120,
+  updateToSchemaVersion1130,
   version as MAX_VERSION,
-} from './1120-messages-foreign-keys-indexes';
+} from './1130-isStory-index';
 
 function updateToSchemaVersion1(
   currentVersion: number,
@@ -2044,9 +2045,11 @@ export const SCHEMA_VERSIONS = [
   updateToSchemaVersion1070,
   updateToSchemaVersion1080,
   updateToSchemaVersion1090,
+
   updateToSchemaVersion1100,
   updateToSchemaVersion1110,
   updateToSchemaVersion1120,
+  updateToSchemaVersion1130,
 ];
 
 export class DBVersionFromFutureError extends Error {

ts/test-electron/sql/stories_test.ts

@@ -137,5 +137,114 @@ describe('sql/stories', () => {
         'storiesByAuthor last should be story2'
       );
     });
+
+    it('populates hasReplies and hasRepliesFromSelf', async () => {
+      assert.lengthOf(await _getAllMessages(), 0);
+
+      const now = Date.now();
+      const conversationId = generateUuid();
+      const sourceServiceId = generateAci();
+      const ourAci = generateAci();
+      const storyId1 = generateUuid();
+      const storyId2 = generateUuid();
+
+      const story1: MessageAttributesType = {
+        id: storyId1,
+        body: 'story 1',
+        type: 'story',
+        conversationId,
+        sent_at: now - 20,
+        received_at: now - 20,
+        timestamp: now - 20,
+        sourceServiceId: generateAci(),
+      };
+      const story2: MessageAttributesType = {
+        id: storyId2,
+        body: 'story 2',
+        type: 'story',
+        conversationId: generateUuid(),
+        sent_at: now - 10,
+        received_at: now - 10,
+        timestamp: now - 10,
+        sourceServiceId,
+      };
+      const story3: MessageAttributesType = {
+        id: generateUuid(),
+        body: 'story 3',
+        type: 'story',
+        conversationId: generateUuid(),
+        sent_at: now,
+        received_at: now,
+        timestamp: now,
+        sourceServiceId,
+      };
+      const replyTo1: MessageAttributesType = {
+        id: generateUuid(),
+        body: 'message 3',
+        type: 'incoming',
+        storyId: storyId1,
+        conversationId: generateUuid(),
+        sent_at: now,
+        received_at: now,
+        timestamp: now,
+        sourceServiceId,
+      };
+      const replyFromSelfTo1: MessageAttributesType = {
+        id: generateUuid(),
+        body: 'story 4',
+        type: 'outgoing',
+        storyId: storyId1,
+        conversationId,
+        sent_at: now,
+        received_at: now,
+        timestamp: now,
+        sourceServiceId: generateAci(),
+      };
+      const replyTo2: MessageAttributesType = {
+        id: generateUuid(),
+        body: 'story 5',
+        type: 'incoming',
+        storyId: storyId2,
+        conversationId: generateUuid(),
+        sent_at: now,
+        received_at: now,
+        timestamp: now,
+        sourceServiceId,
+      };
+
+      await saveMessages(
+        [story1, story2, story3, replyTo1, replyFromSelfTo1, replyTo2],
+        {
+          forceSave: true,
+          ourAci,
+        }
+      );
+
+      assert.lengthOf(await _getAllMessages(), 6);
+
+      const stories = await getAllStories({});
+      assert.lengthOf(stories, 3, 'expect three total stories');
+
+      // They are in ASC order
+      assert.strictEqual(
+        stories[0].id,
+        story1.id,
+        'stories first should be story1'
+      );
+      assert.strictEqual(
+        stories[2].id,
+        story3.id,
+        'stories last should be story3'
+      );
+
+      assert.strictEqual(stories[0].hasReplies, true);
+      assert.strictEqual(stories[0].hasRepliesFromSelf, true);
+
+      assert.strictEqual(stories[1].hasReplies, true);
+      assert.strictEqual(stories[1].hasRepliesFromSelf, false);
+
+      assert.strictEqual(stories[2].hasReplies, false);
+      assert.strictEqual(stories[2].hasRepliesFromSelf, false);
+    });
   });
 });

ts/test-node/sql/migration_1130_test.ts

@@ -0,0 +1,145 @@
+// Copyright 2024 Signal Messenger, LLC
+// SPDX-License-Identifier: AGPL-3.0-only
+
+import { assert } from 'chai';
+import type { WritableDB } from '../../sql/Interface';
+import { createDB, updateToVersion } from './helpers';
+
+describe('SQL/updateToSchemaVersion1130', () => {
+  let db: WritableDB;
+  beforeEach(() => {
+    db = createDB();
+    updateToVersion(db, 1130);
+  });
+
+  afterEach(() => {
+    db.close();
+  });
+
+  it('uses new index for getAllStories query and no params', () => {
+    const details = db
+      .prepare(
+        `
+        EXPLAIN QUERY PLAN 
+        SELECT json, id
+        FROM messages
+        WHERE
+          isStory = 1 AND
+          (NULL IS NULL OR conversationId IS NULL) AND
+          (NULL IS NULL OR sourceServiceId IS NULL)
+        ORDER BY received_at ASC, sent_at ASC;
+        `
+      )
+      .all()
+      .map(step => step.detail)
+      .join(', ');
+
+    assert.strictEqual(details, 'SCAN messages USING INDEX messages_isStory');
+  });
+
+  it('uses new index for getAllStories query and with conversationId', () => {
+    const details = db
+      .prepare(
+        `
+        EXPLAIN QUERY PLAN 
+        SELECT json, id
+        FROM messages
+        WHERE
+          isStory = 1 AND
+          ('something' IS NULL OR conversationId IS 'something') AND
+          (NULL IS NULL OR sourceServiceId IS NULL)
+        ORDER BY received_at ASC, sent_at ASC;
+        `
+      )
+      .all()
+      .map(step => step.detail)
+      .join(', ');
+
+    assert.strictEqual(details, 'SCAN messages USING INDEX messages_isStory');
+  });
+
+  it('uses new index for getAllStories query and with sourceServiceId', () => {
+    const details = db
+      .prepare(
+        `
+        EXPLAIN QUERY PLAN 
+        SELECT json, id
+        FROM messages
+        WHERE
+          isStory = 1 AND
+          (NULL IS NULL OR conversationId IS NULL) AND
+          ('something' IS NULL OR sourceServiceId IS 'something')
+        ORDER BY received_at ASC, sent_at ASC;
+        `
+      )
+      .all()
+      .map(step => step.detail)
+      .join(', ');
+
+    assert.strictEqual(details, 'SCAN messages USING INDEX messages_isStory');
+  });
+
+  it('uses new index for getAllStories query and both params', () => {
+    const details = db
+      .prepare(
+        `
+        EXPLAIN QUERY PLAN 
+        SELECT json, id
+        FROM messages
+        WHERE
+          isStory = 1 AND
+          ('something' IS NULL OR conversationId IS 'something') AND
+          ('something' IS NULL OR sourceServiceId IS 'something')
+        ORDER BY received_at ASC, sent_at ASC;
+        `
+      )
+      .all()
+      .map(step => step.detail)
+      .join(', ');
+
+    assert.strictEqual(details, 'SCAN messages USING INDEX messages_isStory');
+  });
+
+  it('uses previous index for getAllStories get replies query', () => {
+    const details = db
+      .prepare(
+        `
+        EXPLAIN QUERY PLAN
+        SELECT DISTINCT storyId
+        FROM messages
+        WHERE storyId IS NOT NULL
+        `
+      )
+      .all()
+      .map(step => step.detail)
+      .join(', ');
+
+    assert.strictEqual(
+      details,
+      'SEARCH messages USING COVERING INDEX messages_by_storyId (storyId>?)'
+    );
+  });
+
+  it('uses previous index for getAllStories get replies from self query', () => {
+    const details = db
+      .prepare(
+        `
+        EXPLAIN QUERY PLAN
+        SELECT DISTINCT storyId
+        FROM messages
+        WHERE (
+          storyId IS NOT NULL AND
+          type IS 'outgoing'
+        )
+        `
+      )
+      .all()
+      .map(step => step.detail)
+      .join(', ');
+
+    assert.strictEqual(
+      details,
+      'SEARCH messages USING INDEX messages_by_storyId (storyId>?)'
+    );
+  });
+});