diff --git a/ts/sql/Server.ts b/ts/sql/Server.ts index 3f23970696..3033bcda3b 100644 --- a/ts/sql/Server.ts +++ b/ts/sql/Server.ts @@ -2933,44 +2933,50 @@ function getAllStories( sourceServiceId?: ServiceIdString; } ): GetAllStoriesResultType { + const [storiesQuery, storiesParams] = sql` + SELECT json, id + FROM messages + WHERE + 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; - hasReplies: number; - hasRepliesFromSelf: number; - }> = db - .prepare( - ` - 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 - FROM messages - WHERE - type IS 'story' AND - ($conversationId IS NULL OR conversationId IS $conversationId) AND - ($sourceServiceId IS NULL OR sourceServiceId IS $sourceServiceId) - ORDER BY received_at ASC, sent_at ASC; - ` + }> = 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)), })); } diff --git a/ts/sql/migrations/1130-isStory-index.ts b/ts/sql/migrations/1130-isStory-index.ts new file mode 100644 index 0000000000..103dd6531a --- /dev/null +++ b/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!'); +} diff --git a/ts/sql/migrations/index.ts b/ts/sql/migrations/index.ts index f1370d9e61..eb22748a9e 100644 --- a/ts/sql/migrations/index.ts +++ b/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 { diff --git a/ts/test-electron/sql/stories_test.ts b/ts/test-electron/sql/stories_test.ts index 88a6cf7f52..ea33ee471f 100644 --- a/ts/test-electron/sql/stories_test.ts +++ b/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); + }); }); }); diff --git a/ts/test-node/sql/migration_1130_test.ts b/ts/test-node/sql/migration_1130_test.ts new file mode 100644 index 0000000000..5516c9a867 --- /dev/null +++ b/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>?)' + ); + }); +});