model/query: don't use IN without an array (getodk#1449)

As shown by getodk#1450, using `IN` for a single value can have significant effects on the PostgreSQL query plan.

Author: alxndrsn

lib/model/query/analytics.js

@@ -245,7 +245,7 @@ from (
             THEN 1
             ELSE 0
     END) as "hasGeo",
-    max(CASE WHEN ff."type" in ('binary') AND ff."name" = 'audit'
+    max(CASE WHEN ff."type" = 'binary' AND ff."name" = 'audit'
             THEN 1
             ELSE 0
     END) as "hasAudit",
@@ -257,7 +257,7 @@ from (
             THEN 1
             ELSE 0
     END) as "hasGeoRecent",
-    max(CASE WHEN ff."type" in ('binary') AND ff."name" = 'audit' AND recentsubs."activeFormId" is not null
+    max(CASE WHEN ff."type" = 'binary' AND ff."name" = 'audit' AND recentsubs."activeFormId" is not null
             THEN 1
             ELSE 0
     END) as "hasAuditRecent"

lib/model/query/client-audits.js

@@ -8,7 +8,7 @@
 // except according to the terms contained in the LICENSE file.
 
 const { sql } = require('slonik');
-const { insertMany, QueryOptions } = require('../../util/db');
+const { insertMany, sqlInArray, QueryOptions } = require('../../util/db');
 const { odataFilter } = require('../../data/odata-filter');
 const { odataToColumnMap } = require('../../data/submission');
 const { streamBlobs } = require('../../util/blob');
@@ -19,10 +19,6 @@ const existsForBlob = (blobId) => ({ maybeOne }) =>
   maybeOne(sql`select true from client_audits where "blobId"=${blobId} limit 1`)
     .then((x) => x.isDefined());
 
-// TODO: copy-pasted from query/submission-attachments.js
-const keyIdCondition = (keyIds) =>
-  sql.join((((keyIds == null) || (keyIds.length === 0)) ? [ -1 ] : keyIds), sql`,`);
-
 const streamForExport = (formId, draft, keyIds, options = QueryOptions.none) => ({ s3, stream }) => stream(sql`
 select client_audits.*, blobs.id AS "blobId", blobs.s3_status, blobs.content, blobs.sha, submissions."instanceId", "localKey", "keyId", index, submissions."instanceId" from submission_defs
   inner join
@@ -38,7 +34,7 @@ select client_audits.*, blobs.id AS "blobId", blobs.s3_status, blobs.content, bl
   inner join form_defs on submission_defs."formDefId"=form_defs.id
   where ${odataFilter(options.filter, odataToColumnMap)}
     and current=true
-    and (form_defs."keyId" is null or form_defs."keyId" in (${keyIdCondition(keyIds)}))
+    and (form_defs."keyId" is null or ${sqlInArray(sql`form_defs."keyId"`, keyIds)})
   order by submission_defs."createdAt" asc, submission_defs.id asc`)
   .then(dbStream => streamBlobs(s3, dbStream));
 

lib/model/query/keys.js

@@ -11,7 +11,7 @@ const { sql } = require('slonik');
 const { map } = require('ramda');
 const { Key } = require('../frames');
 const { submissionDecryptor } = require('../../util/crypto');
-const { insert } = require('../../util/db');
+const { insert, sqlInArray } = require('../../util/db');
 const { resolve } = require('../../util/promise');
 const { construct } = require('../../util/util');
 
@@ -48,7 +48,7 @@ ORDER BY keys.id DESC`)
   .then(map(construct(Key)));
 
 const getManagedByIds = (ids) => ({ all }) =>
-  all(sql`select * from keys where managed=true and id in (${sql.join(ids, sql`,`)})`)
+  all(sql`select * from keys where managed=true and ${sqlInArray(sql`id`, ids)}`)
     .then(map(construct(Key)));
 
 const getDecryptor = (passphrases = {}) => ({ Keys }) => {

lib/model/query/submission-attachments.js

@@ -16,7 +16,7 @@ const { Audit, Blob, Submission } = require('../frames');
 const { odataFilter } = require('../../data/odata-filter');
 const { odataToColumnMap } = require('../../data/submission');
 const { submissionXmlToFieldStream } = require('../../data/submission');
-const { insertMany, QueryOptions } = require('../../util/db');
+const { insertMany, sqlInArray, QueryOptions } = require('../../util/db');
 const { resolve } = require('../../util/promise');
 const { isBlank, construct } = require('../../util/util');
 const { traverseXml, findAll, root, node, text } = require('../../util/xml');
@@ -189,10 +189,6 @@ const getBySubmissionDefIdAndName = (subDefId, name) => ({ maybeOne }) =>
 ////////////////////////////////////////////////////////////////////////////////
 // EXPORT
 
-// TODO: copy-pasted to query/client-audits.js
-const keyIdCondition = (keyIds) =>
-  sql.join((((keyIds == null) || (keyIds.length === 0)) ? [ -1 ] : keyIds), sql`,`);
-
 const streamForExport = (formId, draft, keyIds, options = QueryOptions.none) => ({ s3, stream }) => stream(sql`
 select submission_attachments.name, blobs.id AS "blobId", blobs.content, blobs.s3_status, blobs.sha, submission_attachments.index, form_defs."keyId", submissions."instanceId", submission_defs."localKey" from submission_defs
 inner join (select * from submissions where draft=${draft}) as submissions
@@ -206,7 +202,7 @@ where submission_defs.current=true
   and ${odataFilter(options.filter, odataToColumnMap)}
   and submission_attachments.name is distinct from submission_defs."encDataAttachmentName"
   and submission_attachments."isClientAudit" is not true
-  and (form_defs."keyId" is null or form_defs."keyId" in (${keyIdCondition(keyIds)}))`
+  and (form_defs."keyId" is null or ${sqlInArray(sql`form_defs."keyId"`, keyIds)})`
 )
   .then(dbStream => streamBlobs(s3, dbStream));
 

lib/model/query/submissions.js

@@ -13,7 +13,7 @@ const { Frame, table } = require('../frame');
 const { Actor, Form, Submission } = require('../frames');
 const { odataFilter, odataOrderBy, odataExcludeDeleted } = require('../../data/odata-filter');
 const { odataToColumnMap, odataSubTableToColumnMap } = require('../../data/submission');
-const { unjoiner, extender, sqlEquals, page, updater, QueryOptions, insertMany, markDeleted, markUndeleted } = require('../../util/db');
+const { unjoiner, extender, sqlEquals, page, updater, QueryOptions, insertMany, markDeleted, markUndeleted, sqlInArray } = require('../../util/db');
 const { blankStringToNull, construct } = require('../../util/util');
 const Problem = require('../../util/problem');
 const { streamEncBlobs } = require('../../util/blob');
@@ -395,7 +395,7 @@ inner join
   on edits."submissionId"=submission_defs."submissionId"
 ${joinOnSkiptoken(options.skiptoken, formId, draft)}
 where
-  ${encrypted ? sql`(form_defs."encKeyId" is null or form_defs."encKeyId" in (${sql.join(keyIds, sql`,`)})) and` : sql``}
+  ${encrypted ? sql`(form_defs."encKeyId" is null or ${sqlInArray(sql`form_defs."encKeyId"`, keyIds)}) and` : sql``}
   ${odataFilter(options.filter, options.isSubmissionsTable ? odataToColumnMap : odataSubTableToColumnMap)} and
   ${sqlEquals(options.condition)}
   and submission_defs.current=true and submissions."formId"=${formId} and ${odataExcludeDeleted(options.filter, options.isSubmissionsTable ? odataToColumnMap : odataSubTableToColumnMap)}

lib/util/db.js

@@ -587,10 +587,16 @@ const postgresErrorToProblem = (x) => {
   return reject(error);
 };
 
+const sqlInArray = (k, arr) => {
+  if (!arr?.length) return sql`FALSE`;
+  if (arr.length === 1) return sql`${k} = ${arr[0]}`;
+  return sql`${k} IN (${sql.join(arr, sql`,`)})`;
+};
+
 module.exports = {
   connectionString, knexConnection,
   unjoiner, extender, sqlEquals, page, queryFuncs,
-  insert, insertMany, updater, markDeleted, markUndeleted,
+  insert, insertMany, updater, markDeleted, markUndeleted, sqlInArray,
   QueryOptions,
   postgresErrorToProblem
 };