From ea04aeded4064d79ab3599c77b39056afc612f0a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bar=C4=B1=C5=9F=20Soner=20U=C5=9Fakl=C4=B1?=
 <baris@nodebb.org>
Date: Fri, 1 Oct 2021 15:24:19 -0400
Subject: [PATCH] perf: convert promise.all to single query (#9851)

---
 src/database/postgres/hash.js | 81 ++++++++++++++++++++++++++---------
 test/database/hash.js         |  7 +++
 2 files changed, 67 insertions(+), 21 deletions(-)

diff --git a/src/database/postgres/hash.js b/src/database/postgres/hash.js
index 04b38713c2..015154aa42 100644
--- a/src/database/postgres/hash.js
+++ b/src/database/postgres/hash.js
@@ -16,7 +16,20 @@ module.exports = function (module) {
 		}
 		await module.transaction(async (client) => {
 			const dataString = JSON.stringify(data);
-			async function setOne(key) {
+
+			if (Array.isArray(key)) {
+				await helpers.ensureLegacyObjectsType(client, key, 'hash');
+				await client.query({
+					name: 'setObjectKeys',
+					text: `
+	INSERT INTO "legacy_hash" ("_key", "data")
+	SELECT k, $2::TEXT::JSONB
+	FROM UNNEST($1::TEXT[]) vs(k)
+	ON CONFLICT ("_key")
+	DO UPDATE SET "data" = "legacy_hash"."data" || $2::TEXT::JSONB`,
+					values: [key, dataString],
+				});
+			} else {
 				await helpers.ensureLegacyObjectType(client, key, 'hash');
 				await client.query({
 					name: 'setObject',
@@ -28,11 +41,6 @@ module.exports = function (module) {
 					values: [key, dataString],
 				});
 			}
-			if (Array.isArray(key)) {
-				await Promise.all(key.map(k => setOne(k)));
-			} else {
-				await setOne(key);
-			}
 		});
 	};
 
@@ -40,8 +48,36 @@ module.exports = function (module) {
 		if (!keys.length || !data.length) {
 			return;
 		}
-		// TODO: single query?
-		await Promise.all(keys.map((k, i) => module.setObject(k, data[i])));
+		await module.transaction(async (client) => {
+			keys = keys.slice();
+			data = data.filter((d, i) => {
+				if (d.hasOwnProperty('')) {
+					delete d[''];
+				}
+				const keep = !!Object.keys(d).length;
+				if (!keep) {
+					keys.splice(i, 1);
+				}
+				return keep;
+			});
+
+			if (!keys.length) {
+				return;
+			}
+
+			await helpers.ensureLegacyObjectsType(client, keys, 'hash');
+			const dataStrings = data.map(JSON.stringify);
+			await client.query({
+				name: 'setObjectBulk',
+				text: `
+			INSERT INTO "legacy_hash" ("_key", "data")
+			SELECT k, d
+			FROM UNNEST($1::TEXT[], $2::TEXT::JSONB[]) vs(k, d)
+			ON CONFLICT ("_key")
+			DO UPDATE SET "data" = "legacy_hash"."data" || EXCLUDED.data`,
+				values: [keys, dataStrings],
+			});
+		});
 	};
 
 	module.setObjectField = async function (key, field, value) {
@@ -51,7 +87,9 @@ module.exports = function (module) {
 
 		await module.transaction(async (client) => {
 			const valueString = JSON.stringify(value);
-			async function setOne(key) {
+			if (Array.isArray(key)) {
+				await module.setObject(key, { [field]: value });
+			} else {
 				await helpers.ensureLegacyObjectType(client, key, 'hash');
 				await client.query({
 					name: 'setObjectField',
@@ -63,12 +101,6 @@ module.exports = function (module) {
 					values: [key, field, valueString],
 				});
 			}
-
-			if (Array.isArray(key)) {
-				await Promise.all(key.map(k => setOne(k)));
-			} else {
-				await setOne(key);
-			}
 		});
 	};
 
@@ -269,7 +301,19 @@ SELECT (h."data" ? $2::TEXT AND h."data"->>$2::TEXT IS NOT NULL) b
 		if (!key || (Array.isArray(key) && !key.length) || !Array.isArray(fields) || !fields.length) {
 			return;
 		}
-		async function delKey(key, fields) {
+
+		if (Array.isArray(key)) {
+			await module.pool.query({
+				name: 'deleteObjectFieldsKeys',
+				text: `
+	UPDATE "legacy_hash"
+	   SET "data" = COALESCE((SELECT jsonb_object_agg("key", "value")
+								FROM jsonb_each("data")
+							   WHERE "key" <> ALL ($2::TEXT[])), '{}')
+	 WHERE "_key" = ANY($1::TEXT[])`,
+				values: [key, fields],
+			});
+		} else {
 			await module.pool.query({
 				name: 'deleteObjectFields',
 				text: `
@@ -281,11 +325,6 @@ SELECT (h."data" ? $2::TEXT AND h."data"->>$2::TEXT IS NOT NULL) b
 				values: [key, fields],
 			});
 		}
-		if (Array.isArray(key)) {
-			await Promise.all(key.map(k => delKey(k, fields)));
-		} else {
-			await delKey(key, fields);
-		}
 	};
 
 	module.incrObjectField = async function (key, field) {
diff --git a/test/database/hash.js b/test/database/hash.js
index 650afae8bf..e2e2ee7364 100644
--- a/test/database/hash.js
+++ b/test/database/hash.js
@@ -90,6 +90,13 @@ describe('Hash methods', () => {
 			assert.deepStrictEqual(result, [{ foo: '1' }, null]);
 		});
 
+		it('should update existing object on second call', async () => {
+			await db.setObjectBulk(['bulkKey3.5'], [{ foo: '1' }]);
+			await db.setObjectBulk(['bulkKey3.5'], [{ baz: '2' }]);
+			const result = await db.getObject('bulkKey3.5');
+			assert.deepStrictEqual(result, { foo: '1', baz: '2' });
+		});
+
 		it('should not error if object is empty', async () => {
 			const keys = ['bulkKey5'];
 			const data = [{ }];