From 7ef2a9155c002c62ce70850fcbe502403e93136d Mon Sep 17 00:00:00 2001 From: trevor-signal <131492920+trevor-signal@users.noreply.github.com> Date: Fri, 28 Jun 2024 20:47:05 -0400 Subject: [PATCH] Add indices to improve messages_on_delete trigger --- .../migrations/1090-message-delete-indexes.ts | 32 +++ ts/sql/migrations/index.ts | 6 +- ts/test-node/sql/migration_1080_test.ts | 214 +++++++++--------- ts/test-node/sql/migration_1090_test.ts | 56 +++++ 4 files changed, 198 insertions(+), 110 deletions(-) create mode 100644 ts/sql/migrations/1090-message-delete-indexes.ts create mode 100644 ts/test-node/sql/migration_1090_test.ts diff --git a/ts/sql/migrations/1090-message-delete-indexes.ts b/ts/sql/migrations/1090-message-delete-indexes.ts new file mode 100644 index 0000000000..cb1b1aee71 --- /dev/null +++ b/ts/sql/migrations/1090-message-delete-indexes.ts @@ -0,0 +1,32 @@ +// 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 = 1090; + +export function updateToSchemaVersion1090( + currentVersion: number, + db: Database, + logger: LoggerType +): void { + if (currentVersion >= 1090) { + return; + } + + db.transaction(() => { + db.exec(` + CREATE INDEX reactions_messageId + ON reactions (messageId); + + CREATE INDEX storyReads_storyId + ON storyReads (storyId); + `); + })(); + + db.pragma('user_version = 1090'); + + logger.info('updateToSchemaVersion1090: success!'); +} diff --git a/ts/sql/migrations/index.ts b/ts/sql/migrations/index.ts index 4c4da9c470..f716fd4dc6 100644 --- a/ts/sql/migrations/index.ts +++ b/ts/sql/migrations/index.ts @@ -83,10 +83,11 @@ import { updateToSchemaVersion1040 } from './1040-undownloaded-backed-up-media'; import { updateToSchemaVersion1050 } from './1050-group-send-endorsements'; import { updateToSchemaVersion1060 } from './1060-addressable-messages-and-sync-tasks'; import { updateToSchemaVersion1070 } from './1070-attachment-backup'; +import { updateToSchemaVersion1080 } from './1080-nondisappearing-addressable'; import { - updateToSchemaVersion1080, + updateToSchemaVersion1090, version as MAX_VERSION, -} from './1080-nondisappearing-addressable'; +} from './1090-message-delete-indexes'; function updateToSchemaVersion1( currentVersion: number, @@ -2038,6 +2039,7 @@ export const SCHEMA_VERSIONS = [ updateToSchemaVersion1060, updateToSchemaVersion1070, updateToSchemaVersion1080, + updateToSchemaVersion1090, ]; export class DBVersionFromFutureError extends Error { diff --git a/ts/test-node/sql/migration_1080_test.ts b/ts/test-node/sql/migration_1080_test.ts index cca34c428e..0212bc5de6 100644 --- a/ts/test-node/sql/migration_1080_test.ts +++ b/ts/test-node/sql/migration_1080_test.ts @@ -39,109 +39,108 @@ describe('SQL/updateToSchemaVersion1080', () => { }); describe('Addressable Messages', () => { - describe('Storing of new attachment jobs', () => { - it('returns only incoming/outgoing messages', () => { - const conversationId = generateGuid(); - const otherConversationId = generateGuid(); + it('returns only incoming/outgoing messages', () => { + const conversationId = generateGuid(); + const otherConversationId = generateGuid(); - insertData(db, 'messages', [ - generateMessage({ - id: '1', - conversationId, - type: 'incoming', - received_at: 1, - sent_at: 1, - timestamp: 1, - }), - generateMessage({ - id: '2', - conversationId, - type: 'story', - received_at: 2, - sent_at: 2, - timestamp: 2, - }), - generateMessage({ - id: '3', - conversationId, - type: 'outgoing', - received_at: 3, - sent_at: 3, - timestamp: 3, - }), - generateMessage({ - id: '4', - conversationId, - type: 'group-v1-migration', - received_at: 4, - sent_at: 4, - timestamp: 4, - }), - generateMessage({ - id: '5', - conversationId, - type: 'group-v2-change', - received_at: 5, - sent_at: 5, - timestamp: 5, - }), - generateMessage({ - id: '6', - conversationId, - type: 'incoming', - received_at: 6, - sent_at: 6, - timestamp: 6, - expireTimer: DurationInSeconds.fromMinutes(10), - }), - generateMessage({ - id: '7', - conversationId, - type: 'profile-change', - received_at: 7, - sent_at: 7, - timestamp: 7, - }), - generateMessage({ - id: '8', - conversationId: otherConversationId, - type: 'incoming', - received_at: 8, - sent_at: 8, - timestamp: 8, - }), - ]); + insertData(db, 'messages', [ + generateMessage({ + id: '1', + conversationId, + type: 'incoming', + received_at: 1, + sent_at: 1, + timestamp: 1, + }), + generateMessage({ + id: '2', + conversationId, + type: 'story', + received_at: 2, + sent_at: 2, + timestamp: 2, + }), + generateMessage({ + id: '3', + conversationId, + type: 'outgoing', + received_at: 3, + sent_at: 3, + timestamp: 3, + }), + generateMessage({ + id: '4', + conversationId, + type: 'group-v1-migration', + received_at: 4, + sent_at: 4, + timestamp: 4, + }), + generateMessage({ + id: '5', + conversationId, + type: 'group-v2-change', + received_at: 5, + sent_at: 5, + timestamp: 5, + }), + generateMessage({ + id: '6', + conversationId, + type: 'incoming', + received_at: 6, + sent_at: 6, + timestamp: 6, + expireTimer: DurationInSeconds.fromMinutes(10), + }), + generateMessage({ + id: '7', + conversationId, + type: 'profile-change', + received_at: 7, + sent_at: 7, + timestamp: 7, + }), + generateMessage({ + id: '8', + conversationId: otherConversationId, + type: 'incoming', + received_at: 8, + sent_at: 8, + timestamp: 8, + }), + ]); - const messages = getMostRecentAddressableNondisappearingMessagesSync( - db, - conversationId - ); + const messages = getMostRecentAddressableNondisappearingMessagesSync( + db, + conversationId + ); - assert.lengthOf(messages, 2); - assert.deepEqual(messages, [ - { - id: '3', - conversationId, - type: 'outgoing', - received_at: 3, - sent_at: 3, - timestamp: 3, - }, - { - id: '1', - conversationId, - type: 'incoming', - received_at: 1, - sent_at: 1, - timestamp: 1, - }, - ]); - }); + assert.lengthOf(messages, 2); + assert.deepEqual(messages, [ + { + id: '3', + conversationId, + type: 'outgoing', + received_at: 3, + sent_at: 3, + timestamp: 3, + }, + { + id: '1', + conversationId, + type: 'incoming', + received_at: 1, + sent_at: 1, + timestamp: 1, + }, + ]); + }); - it('ensures that index is used for getMostRecentAddressableNondisappearingMessagesSync, with storyId', () => { - const { detail } = db - .prepare( - ` + it('ensures that index is used for getMostRecentAddressableNondisappearingMessagesSync, with storyId', () => { + const { detail } = db + .prepare( + ` EXPLAIN QUERY PLAN SELECT json FROM messages INDEXED BY messages_by_date_addressable_nondisappearing @@ -152,16 +151,15 @@ describe('SQL/updateToSchemaVersion1080', () => { ORDER BY received_at DESC, sent_at DESC LIMIT 5; ` - ) - .get(); + ) + .get(); - assert.notInclude(detail, 'B-TREE'); - assert.notInclude(detail, 'SCAN'); - assert.include( - detail, - 'SEARCH messages USING INDEX messages_by_date_addressable_nondisappearing (conversationId=? AND isAddressableMessage=?)' - ); - }); + assert.notInclude(detail, 'B-TREE'); + assert.notInclude(detail, 'SCAN'); + assert.include( + detail, + 'SEARCH messages USING INDEX messages_by_date_addressable_nondisappearing (conversationId=? AND isAddressableMessage=?)' + ); }); }); }); diff --git a/ts/test-node/sql/migration_1090_test.ts b/ts/test-node/sql/migration_1090_test.ts new file mode 100644 index 0000000000..39cd25a8d7 --- /dev/null +++ b/ts/test-node/sql/migration_1090_test.ts @@ -0,0 +1,56 @@ +// Copyright 2024 Signal Messenger, LLC +// SPDX-License-Identifier: AGPL-3.0-only + +import { assert } from 'chai'; +import type { Database } from '@signalapp/better-sqlite3'; +import SQL from '@signalapp/better-sqlite3'; +import { updateToVersion } from './helpers'; + +describe('SQL/updateToSchemaVersion1090', () => { + let db: Database; + beforeEach(() => { + db = new SQL(':memory:'); + updateToVersion(db, 1090); + }); + + afterEach(() => { + db.close(); + }); + + describe('Additional messages_on_delete indexes', () => { + it('uses index for selecting reactions by messageId', () => { + const details = db + .prepare( + `EXPLAIN QUERY PLAN + SELECT rowid FROM reactions + WHERE messageId = '123'; + ` + ) + .all() + .map(step => step.detail) + .join(', '); + + assert.strictEqual( + details, + 'SEARCH reactions USING COVERING INDEX reactions_messageId (messageId=?)' + ); + }); + + it('uses index for selecting storyReads by storyId', () => { + const details = db + .prepare( + `EXPLAIN QUERY PLAN + DELETE FROM storyReads WHERE storyId = '123'; + ` + ) + .all() + .map(step => step.detail) + .join(', '); + + assert.strictEqual( + details, + 'SEARCH storyReads USING INDEX storyReads_storyId (storyId=?)' + ); + }); + }); +});