Update message attachment migration

ts/background.ts

@@ -1019,7 +1019,7 @@ export async function startApp(): Promise<void> {
       `Starting background data migration. Target version: ${Message.CURRENT_SCHEMA_VERSION}`
     );
     idleDetector.on('idle', async () => {
-      const NUM_MESSAGES_PER_BATCH = 1000;
+      const NUM_MESSAGES_PER_BATCH = 250;
       const BATCH_DELAY = durations.SECOND / 4;
 
       if (isIdleTaskProcessing) {

ts/sql/Interface.ts

@@ -129,10 +129,9 @@ export type MessageType = MessageAttributesType;
 // - Make sure the name matches the one in `MessageAttributeTypes`
 // - Update `hydrateMessage`
 //
-export const MESSAGE_COLUMNS = [
+const MESSAGE_PRIMARY_KEY_COLUMNS = ['id'] as const;
+export const MESSAGE_NON_PRIMARY_KEY_COLUMNS = [
   'json',
-
-  'id',
   'body',
   'conversationId',
   'expirationStartTimestamp',
@@ -161,6 +160,11 @@ export const MESSAGE_COLUMNS = [
   'unidentifiedDeliveryReceived',
 ] as const;
 
+export const MESSAGE_COLUMNS = [
+  ...MESSAGE_PRIMARY_KEY_COLUMNS,
+  ...MESSAGE_NON_PRIMARY_KEY_COLUMNS,
+] as const;
+
 export type MessageTypeUnhydrated = {
   json: string;
 

ts/sql/Server.ts

@@ -191,6 +191,7 @@ import {
   AttachmentDownloadSource,
   MESSAGE_COLUMNS,
   MESSAGE_ATTACHMENT_COLUMNS,
+  MESSAGE_NON_PRIMARY_KEY_COLUMNS,
 } from './Interface';
 import {
   _removeAllCallLinks,
@@ -235,6 +236,7 @@ import {
 import { generateMessageId } from '../util/generateMessageId';
 import type { ConversationColorType, CustomColorType } from '../types/Colors';
 import { sqlLogger } from './sqlLogger';
+import { APPLICATION_OCTET_STREAM } from '../types/MIME';
 
 type ConversationRow = Readonly<{
   json: string;
@@ -2559,8 +2561,8 @@ function saveMessageAttachment({
     conversationId,
     sentAt,
     clientUuid: attachment.clientUuid,
-    size: attachment.size,
+    size: attachment.size ?? 0,
-    contentType: attachment.contentType,
+    contentType: attachment.contentType ?? APPLICATION_OCTET_STREAM,
     path: attachment.path,
     localKey: attachment.localKey,
     plaintextHash: attachment.plaintextHash,
@@ -2642,8 +2644,7 @@ function saveMessageAttachment({
         INSERT OR REPLACE INTO message_attachments 
           (${MESSAGE_ATTACHMENT_COLUMNS.join(', ')}) 
         VALUES 
-          (${MESSAGE_ATTACHMENT_COLUMNS.map(name => `$${name}`).join(', ')})
+          (${MESSAGE_ATTACHMENT_COLUMNS.map(name => `$${name}`).join(', ')});
-        RETURNING rowId;
       `
   ).run(values);
 }
@@ -2830,20 +2831,24 @@ function saveMessage(
 
   if (id && !forceSave) {
     db.prepare(
+      // UPDATE queries that set the value of a primary key column can be very slow when
+      // that key is referenced via a foreign key constraint, so we are careful to exclude
+      // it here.
       `
         UPDATE messages SET
-        ${MESSAGE_COLUMNS.map(name => `${name} = $${name}`).join(', ')}
+          ${MESSAGE_NON_PRIMARY_KEY_COLUMNS.map(name => `${name} = $${name}`).join(', ')}
         WHERE id = $id;
       `
     ).run({ ...payloadWithoutJson, json: objectToJSON(dataToSaveAsJSON) });
 
+    if (normalizeAttachmentData) {
+      saveMessageAttachments(db, message);
+    }
+
     if (jobToInsert) {
       insertJob(db, jobToInsert);
     }
 
-    if (normalizeAttachmentData) {
-      saveMessageAttachments(db, message);
-    }
     return id;
   }
 

ts/sql/migrations/1360-attachments.ts

@@ -85,24 +85,32 @@ export function updateToSchemaVersion1360(
       ) STRICT;
     `);
 
-    db.exec(
+    // The following indexes were removed in migration 1370
-      'CREATE INDEX message_attachments_messageId ON message_attachments (messageId);'
+
-    );
+    // db.exec(
-    db.exec(
+    //   'CREATE INDEX message_attachments_messageId
-      'CREATE INDEX message_attachments_plaintextHash ON message_attachments (plaintextHash);'
+    //    ON message_attachments (messageId);'
-    );
+    // );
-    db.exec(
+    // db.exec(
-      'CREATE INDEX message_attachments_path ON message_attachments (path);'
+    //   'CREATE INDEX message_attachments_plaintextHash
-    );
+    //    ON message_attachments (plaintextHash);'
-    db.exec(
+    // );
-      'CREATE INDEX message_attachments_all_thumbnailPath ON message_attachments (thumbnailPath);'
+    // db.exec(
-    );
+    //   'CREATE INDEX message_attachments_path
-    db.exec(
+    //    ON message_attachments (path);'
-      'CREATE INDEX message_attachments_all_screenshotPath ON message_attachments (screenshotPath);'
+    // );
-    );
+    // db.exec(
-    db.exec(
+    //   'CREATE INDEX message_attachments_all_thumbnailPath
-      'CREATE INDEX message_attachments_all_backupThumbnailPath ON message_attachments (backupThumbnailPath);'
+    //    ON message_attachments (thumbnailPath);'
-    );
+    // );
+    // db.exec(
+    //   'CREATE INDEX message_attachments_all_screenshotPath
+    //    ON message_attachments (screenshotPath);'
+    // );
+    // db.exec(
+    //   'CREATE INDEX message_attachments_all_backupThumbnailPath
+    //    ON message_attachments (backupThumbnailPath);'
+    // );
 
     db.pragma('user_version = 1360');
   })();

ts/sql/migrations/1370-message-attachment-indexes.ts

@@ -0,0 +1,31 @@
+// Copyright 2025 Signal Messenger, LLC
+// SPDX-License-Identifier: AGPL-3.0-only
+
+import type { LoggerType } from '../../types/Logging';
+import type { WritableDB } from '../Interface';
+
+export const version = 1370;
+
+export function updateToSchemaVersion1370(
+  currentVersion: number,
+  db: WritableDB,
+  logger: LoggerType
+): void {
+  if (currentVersion >= 1370) {
+    return;
+  }
+
+  db.transaction(() => {
+    db.exec(`
+      DROP INDEX IF EXISTS message_attachments_messageId;
+      DROP INDEX IF EXISTS message_attachments_plaintextHash;
+      DROP INDEX IF EXISTS message_attachments_path;
+      DROP INDEX IF EXISTS message_attachments_all_thumbnailPath;
+      DROP INDEX IF EXISTS message_attachments_all_screenshotPath;
+      DROP INDEX IF EXISTS message_attachments_all_backupThumbnailPath;
+    `);
+    db.pragma('user_version = 1370');
+  })();
+
+  logger.info('updateToSchemaVersion1370: success!');
+}

ts/sql/migrations/index.ts

@@ -111,10 +111,11 @@ import { updateToSchemaVersion1320 } from './1320-unprocessed-received-at-date';
 import { updateToSchemaVersion1330 } from './1330-sync-tasks-type-index';
 import { updateToSchemaVersion1340 } from './1340-recent-gifs';
 import { updateToSchemaVersion1350 } from './1350-notification-profiles';
+import { updateToSchemaVersion1360 } from './1360-attachments';
 import {
-  updateToSchemaVersion1360,
+  updateToSchemaVersion1370,
   version as MAX_VERSION,
-} from './1360-attachments';
+} from './1370-message-attachment-indexes';
 
 import { DataWriter } from '../Server';
 
@@ -2104,6 +2105,7 @@ export const SCHEMA_VERSIONS = [
   updateToSchemaVersion1340,
   updateToSchemaVersion1350,
   updateToSchemaVersion1360,
+  updateToSchemaVersion1370,
 ];
 
 export class DBVersionFromFutureError extends Error {

ts/test-electron/normalizedAttachments_test.ts

@@ -12,7 +12,13 @@ import type {
   ThumbnailType,
   BackupThumbnailType,
 } from '../types/Attachment';
-import { IMAGE_JPEG, IMAGE_PNG, LONG_MESSAGE } from '../types/MIME';
+import {
+  APPLICATION_OCTET_STREAM,
+  IMAGE_JPEG,
+  IMAGE_PNG,
+  LONG_MESSAGE,
+  type MIMEType,
+} from '../types/MIME';
 import type { MessageAttributesType } from '../model-types';
 import { generateAci } from '../types/ServiceId';
 import { ReadStatus } from '../messages/MessageReadStatus';
@@ -608,6 +614,8 @@ describe('normalizes attachment references', () => {
   it('handles bad data', async () => {
     const attachment: AttachmentType = {
       ...composeAttachment(),
+      size: undefined as unknown as number,
+      contentType: undefined as unknown as MIMEType,
       uploadTimestamp: {
         low: 6174,
         high: 0,
@@ -629,6 +637,8 @@ describe('normalizes attachment references', () => {
     assert(messageFromDB, 'message was saved');
     assert.deepEqual(messageFromDB.attachments?.[0], {
       ...attachment,
+      size: 0,
+      contentType: APPLICATION_OCTET_STREAM,
       uploadTimestamp: undefined,
       incrementalMac: undefined,
     });

ts/test-node/sql/migration_1360_test.ts

@@ -3,7 +3,7 @@
 
 import { assert } from 'chai';
 
-import { sql, sqlJoin } from '../../sql/util';
+import { sql } from '../../sql/util';
 import { createDB, explain, updateToVersion } from './helpers';
 import type { WritableDB } from '../../sql/Interface';
 import { DataWriter } from '../../sql/Server';
@@ -29,61 +29,8 @@ describe('SQL/updateToSchemaVersion1360', () => {
       );
       assert.strictEqual(
         details,
-        'SEARCH message_attachments USING COVERING INDEX message_attachments_messageId (messageId=?)'
+        'SEARCH message_attachments USING COVERING INDEX sqlite_autoindex_message_attachments_1 (messageId=?)'
       );
     });
-
-    it('uses index to select based on messageId', async () => {
-      const details = explain(
-        db,
-        sql`SELECT * from message_attachments WHERE messageId IN (${sqlJoin(['id1', 'id2'])});`
-      );
-      assert.strictEqual(
-        details,
-        'SEARCH message_attachments USING INDEX message_attachments_messageId (messageId=?)'
-      );
-    });
-
-    it('uses index find path with existing plaintextHash', async () => {
-      const details = explain(
-        db,
-        sql`
-          SELECT path, localKey 
-          FROM message_attachments 
-          WHERE plaintextHash = ${'plaintextHash'}
-          LIMIT 1;
-        `
-      );
-      assert.strictEqual(
-        details,
-        'SEARCH message_attachments USING INDEX message_attachments_plaintextHash (plaintextHash=?)'
-      );
-    });
-
-    it('uses all path indices to find if path is being referenced', async () => {
-      const path = 'path';
-      const details = explain(
-        db,
-        sql`
-           SELECT 1 FROM message_attachments 
-            WHERE 
-              path = ${path} OR 
-              thumbnailPath = ${path} OR
-              screenshotPath = ${path} OR
-              backupThumbnailPath = ${path};
-        `
-      );
-      assert.deepStrictEqual(details.split('\n'), [
-        'MULTI-INDEX OR',
-        'INDEX 1',
-        'SEARCH message_attachments USING INDEX message_attachments_path (path=?)',
-        'INDEX 2',
-        'SEARCH message_attachments USING INDEX message_attachments_all_thumbnailPath (thumbnailPath=?)',
-        'INDEX 3',
-        'SEARCH message_attachments USING INDEX message_attachments_all_screenshotPath (screenshotPath=?)',
-        'INDEX 4',
-        'SEARCH message_attachments USING INDEX message_attachments_all_backupThumbnailPath (backupThumbnailPath=?)',
-      ]);
-    });
   });
 });