Skip to content

Commit

Permalink
Switches "UNIQUE" constraint check to a "SELECT"
Browse files Browse the repository at this point in the history
  • Loading branch information
Spoffy committed Jan 14, 2025
1 parent e1c9a8d commit 227350c
Showing 1 changed file with 29 additions and 26 deletions.
55 changes: 29 additions & 26 deletions app/server/lib/DocStorage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -786,11 +786,7 @@ export class DocStorage implements ISQLiteDB, OnDemandStorage {
storageId?: string,
): Promise<boolean> {
return this.execTransaction(async (db) => {
const isNewFile = await this._addBasicFileRecord(db, fileIdent);
if (isNewFile) {
await this._updateFileRecord(db, fileIdent, fileData, storageId);
}
return isNewFile;
return await this._addFileRecord(db, fileIdent, fileData, storageId);
});
}

Expand All @@ -813,17 +809,20 @@ export class DocStorage implements ISQLiteDB, OnDemandStorage {
storageId?: string,
): Promise<boolean> {
return this.execTransaction(async (db) => {
const isNewFile = await this._addBasicFileRecord(db, fileIdent);
await this._updateFileRecord(db, fileIdent, fileData, storageId);
return isNewFile;
const wasAdded = await this._addFileRecord(db, fileIdent, fileData, storageId);
if (!wasAdded) {
await this._updateFileRecord(db, fileIdent, fileData, storageId);
}
return wasAdded;
});
}

/**
* Reads and returns the data for the given attachment.
* @param {string} fileIdent - The unique identifier of a file, as used by attachFileIfNew.
* @param {boolean} includeData - Load file contents from the database, in addition to metadata
* @returns {Promise[FileInfo | null]} - File information, or null if no record exists for that file identifier.
* @returns {Promise[FileInfo | null]} - File information, or null if no record exists for that
* file identifier.
*/
public getFileInfo(fileIdent: string, includeData: boolean = true): Promise<FileInfo | null> {
const columns = includeData ? 'ident, storageId, data' : 'ident, storageId';
Expand Down Expand Up @@ -938,9 +937,10 @@ export class DocStorage implements ISQLiteDB, OnDemandStorage {
}

/**
* Unmarshals and decodes data received from db.allMarshal() method (which we added to node-sqlite3).
* The data is a dictionary mapping column ids (including 'id') to arrays of values. This should
* be used for Grist data, which is encoded. For non-Grist data, use `marshal.loads()`.
* Unmarshals and decodes data received from db.allMarshal() method (which we added to
* node-sqlite3). The data is a dictionary mapping column ids (including 'id') to arrays of
* values. This should be used for Grist data, which is encoded. For non-Grist data, use
* `marshal.loads()`.
*
* Note that we do NOT use this when loading data from a document, since the whole point of
* db.allMarshal() is to pass data directly to Python data engine without parsing in Node.
Expand Down Expand Up @@ -1446,8 +1446,9 @@ export class DocStorage implements ISQLiteDB, OnDemandStorage {
}

/**
* Delete attachments from _gristsys_Files that have no matching metadata row in _grist_Attachments.
* This leaves any attachment files in any remote attachment stores, which will be cleaned up separately.
* Delete attachments from _gristsys_Files that have no matching metadata row in
* _grist_Attachments. This leaves any attachment files in any remote attachment stores, which
* will be cleaned up separately.
*/
public async removeUnusedAttachments() {
const result = await this._getDB().run(`
Expand Down Expand Up @@ -1861,18 +1862,18 @@ export class DocStorage implements ISQLiteDB, OnDemandStorage {
return null;
}

private async _addBasicFileRecord(db: SQLiteDB, fileIdent: string): Promise<boolean> {
try {
// Try to insert a new record with the given ident. It'll fail UNIQUE constraint if exists.
await db.run('INSERT INTO _gristsys_Files (ident, data, storageId) VALUES (?)', fileIdent);
} catch(err) {
// If UNIQUE constraint failed, this ident must already exist.
if (/^(SQLITE_CONSTRAINT: )?UNIQUE constraint failed/.test(err.message)) {
return false;
} else {
throw err;
}
// This should be executed inside a transaction.
private async _addFileRecord(
db: SQLiteDB, fileIdent: string, fileData?: Buffer, storageId?: string
): Promise<boolean> {
const result = await db.get("SELECT 1 AS fileExists from _gristsys_Files WHERE ident = ?", fileIdent);
if (result?.fileExists) {
return false;
}
await db.run(
'INSERT INTO main._gristsys_Files (ident, data, storageId) VALUES (?, ?, ?)',
fileIdent, fileData, storageId
);
return true;
}

Expand All @@ -1881,6 +1882,7 @@ export class DocStorage implements ISQLiteDB, OnDemandStorage {
): Promise<void> {
await db.run('UPDATE _gristsys_Files SET data=?, storageId=? WHERE ident=?', fileData, storageId, fileIdent);
}

}

interface RebuildResult {
Expand All @@ -1905,7 +1907,8 @@ export interface IndexInfo extends IndexColumns {
}

/**
* Creates an index that allows fast SQL JOIN between _grist_Attachments.fileIdent and _gristsys_Files.ident.
* Creates an index that allows fast SQL JOIN between _grist_Attachments.fileIdent and
* _gristsys_Files.ident.
*/
export async function createAttachmentsIndex(db: ISQLiteDB) {
await db.exec(`CREATE INDEX _grist_Attachments_fileIdent ON _grist_Attachments(fileIdent)`);
Expand Down

0 comments on commit 227350c

Please sign in to comment.