Merge pull request #5419 from Budibase/fix/5411

Fix for SQL IN queries, trimming the inputs to remove excess whitespace

packages/builder/src/pages/builder/app/[application]/data/datasource/[selectedDatasource]/rest/[query]/index.svelte

@@ -136,7 +136,7 @@
         notifications.success("Request sent successfully")
       }
     } catch (error) {
-      notifications.error("Error running query")
+      notifications.error(`Query Error: ${error.message}`)
     }
   }
 

packages/server/scripts/integrations/mysql/init.sql

@@ -26,6 +26,7 @@ CREATE TABLE Products (
     updated time
 );
 INSERT INTO Persons (FirstName, LastName, Age, Address, City, CreatedAt) VALUES ('Mike', 'Hughes', 28.2, '123 Fake Street', 'Belfast', '2021-01-19 03:14:07');
+INSERT INTO Persons (FirstName, LastName, Age, Address, City, CreatedAt) VALUES ('Dave', 'Johnson', 29, '124 Fake Street', 'Belfast', '2022-04-01 00:11:11');
 INSERT INTO Tasks (PersonID, TaskName, CreatedAt) VALUES (1, 'assembling', '2020-01-01');
 INSERT INTO Tasks (PersonID, TaskName, CreatedAt) VALUES (2, 'processing', '2019-12-31');
 INSERT INTO Products (name, updated) VALUES ('Meat', '11:00:22'), ('Fruit', '10:00:00');

packages/server/src/integrations/queries/sql.ts

@@ -0,0 +1,125 @@
+import { findHBSBlocks, processStringSync } from "@budibase/string-templates"
+import { Integration } from "../../definitions/datasource"
+import { DatasourcePlus } from "../base/datasourcePlus"
+
+const CONST_CHAR_REGEX = new RegExp("'[^']*'", "g")
+
+export function enrichQueryFields(
+  fields: { [key: string]: any },
+  parameters = {}
+) {
+  const enrichedQuery: { [key: string]: any } = Array.isArray(fields) ? [] : {}
+
+  // enrich the fields with dynamic parameters
+  for (let key of Object.keys(fields)) {
+    if (fields[key] == null) {
+      continue
+    }
+    if (typeof fields[key] === "object") {
+      // enrich nested fields object
+      enrichedQuery[key] = enrichQueryFields(fields[key], parameters)
+    } else if (typeof fields[key] === "string") {
+      // enrich string value as normal
+      enrichedQuery[key] = processStringSync(fields[key], parameters, {
+        noEscaping: true,
+        noHelpers: true,
+        escapeNewlines: true,
+      })
+    } else {
+      enrichedQuery[key] = fields[key]
+    }
+  }
+  if (
+    enrichedQuery.json ||
+    enrichedQuery.customData ||
+    enrichedQuery.requestBody
+  ) {
+    try {
+      enrichedQuery.json = JSON.parse(
+        enrichedQuery.json ||
+          enrichedQuery.customData ||
+          enrichedQuery.requestBody
+      )
+    } catch (err) {
+      // no json found, ignore
+    }
+    delete enrichedQuery.customData
+  }
+  return enrichedQuery
+}
+
+export function interpolateSQL(
+  fields: { [key: string]: any },
+  parameters: { [key: string]: any },
+  integration: DatasourcePlus
+) {
+  let sql = fields.sql
+  if (!sql || typeof sql !== "string") {
+    return fields
+  }
+  const bindings = findHBSBlocks(sql)
+  let variables = [],
+    arrays = []
+  for (let binding of bindings) {
+    // look for array/list operations in the SQL statement, which will need handled later
+    const listRegexMatch = sql.match(
+      new RegExp(`(in|IN|In|iN)( )+[(]?${binding}[)]?`)
+    )
+    // check if the variable was used as part of a string concat e.g. 'Hello {{binding}}'
+    // start by finding all the instances of const character strings
+    const charConstMatch = sql.match(CONST_CHAR_REGEX) || []
+    // now look within them to see if a binding is used
+    const charConstBindingMatch = charConstMatch.find((string: any) =>
+      string.match(new RegExp(`'[^']*${binding}[^']*'`))
+    )
+    if (charConstBindingMatch) {
+      let [part1, part2] = charConstBindingMatch.split(binding)
+      part1 = `'${part1.substring(1)}'`
+      part2 = `'${part2.substring(0, part2.length - 1)}'`
+      sql = sql.replace(
+        charConstBindingMatch,
+        integration.getStringConcat([
+          part1,
+          integration.getBindingIdentifier(),
+          part2,
+        ])
+      )
+    }
+    // generate SQL parameterised array
+    else if (listRegexMatch) {
+      arrays.push(binding)
+      // determine the length of the array
+      const value = enrichQueryFields([binding], parameters)[0]
+        .split(",")
+        .map((val: string) => val.trim())
+      // build a string like ($1, $2, $3)
+      let replacement = `${Array.apply(null, Array(value.length))
+        .map(() => integration.getBindingIdentifier())
+        .join(",")}`
+      // check if parentheses are needed
+      if (!listRegexMatch[0].includes(`(${binding})`)) {
+        replacement = `(${replacement})`
+      }
+      sql = sql.replace(binding, replacement)
+    } else {
+      sql = sql.replace(binding, integration.getBindingIdentifier())
+    }
+    variables.push(binding)
+  }
+  // replicate the knex structure
+  fields.sql = sql
+  fields.bindings = enrichQueryFields(variables, parameters)
+  // check for arrays in the data
+  let updated: string[] = []
+  for (let i = 0; i < variables.length; i++) {
+    if (arrays.includes(variables[i])) {
+      updated = updated.concat(
+        fields.bindings[i].split(",").map((val: string) => val.trim())
+      )
+    } else {
+      updated.push(fields.bindings[i])
+    }
+  }
+  fields.bindings = updated
+  return fields
+}

packages/server/src/threads/index.js

@@ -36,6 +36,7 @@ class Thread {
         maxConcurrentWorkers: this.count,
       }
       if (opts.timeoutMs) {
+        this.timeoutMs = opts.timeoutMs
         workerOpts.maxCallTime = opts.timeoutMs
       }
       this.workers = workerFarm(workerOpts, typeToFile(type))
@@ -43,6 +44,7 @@ class Thread {
   }
 
   run(data) {
+    const timeoutMs = this.timeoutMs
     return new Promise((resolve, reject) => {
       let fncToCall
       // if in test then don't use threading
@@ -52,7 +54,11 @@ class Thread {
         fncToCall = this.workers
       }
       fncToCall(data, (err, response) => {
-        if (err) {
+        if (err && err.type === "TimeoutError") {
+          reject(
+            new Error(`Query response time exceeded ${timeoutMs}ms timeout.`)
+          )
+        } else if (err) {
           reject(err)
         } else {
           resolve(response)

packages/server/src/threads/query.js

@@ -2,14 +2,13 @@ const threadUtils = require("./utils")
 threadUtils.threadSetup()
 const ScriptRunner = require("../utilities/scriptRunner")
 const { integrations } = require("../integrations")
-const {
+const { processStringSync } = require("@budibase/string-templates")
-  processStringSync,
-  findHBSBlocks,
-} = require("@budibase/string-templates")
 const { doInAppContext, getAppDB } = require("@budibase/backend-core/context")
 const { isSQL } = require("../integrations/utils")
-
+const {
-const CONST_CHAR_REGEX = new RegExp("'[^']*'", "g")
+  enrichQueryFields,
+  interpolateSQL,
+} = require("../integrations/queries/sql")
 
 class QueryRunner {
   constructor(input, flags = { noRecursiveQuery: false }) {
@@ -29,76 +28,6 @@ class QueryRunner {
     this.hasRerun = false
   }
 
-  interpolateSQL(fields, parameters, integration) {
-    let sql = fields.sql
-    if (!sql) {
-      return fields
-    }
-    const bindings = findHBSBlocks(sql)
-    let variables = [],
-      arrays = []
-    for (let binding of bindings) {
-      // look for array/list operations in the SQL statement, which will need handled later
-      const listRegexMatch = sql.match(
-        new RegExp(`(in|IN|In|iN)( )+[(]?${binding}[)]?`)
-      )
-      // check if the variable was used as part of a string concat e.g. 'Hello {{binding}}'
-      // start by finding all the instances of const character strings
-      const charConstMatch = sql.match(CONST_CHAR_REGEX) || []
-      // now look within them to see if a binding is used
-      const charConstBindingMatch = charConstMatch.find(string =>
-        string.match(new RegExp(`'[^']*${binding}[^']*'`))
-      )
-      if (charConstBindingMatch) {
-        let [part1, part2] = charConstBindingMatch.split(binding)
-        part1 = `'${part1.substring(1)}'`
-        part2 = `'${part2.substring(0, part2.length - 1)}'`
-        sql = sql.replace(
-          charConstBindingMatch,
-          integration.getStringConcat([
-            part1,
-            integration.getBindingIdentifier(),
-            part2,
-          ])
-        )
-      }
-      // generate SQL parameterised array
-      else if (listRegexMatch) {
-        arrays.push(binding)
-        // determine the length of the array
-        const value = this.enrichQueryFields([binding], parameters)[0].split(
-          ","
-        )
-        // build a string like ($1, $2, $3)
-        let replacement = `${Array.apply(null, Array(value.length))
-          .map(() => integration.getBindingIdentifier())
-          .join(",")}`
-        // check if parentheses are needed
-        if (!listRegexMatch[0].includes(`(${binding})`)) {
-          replacement = `(${replacement})`
-        }
-        sql = sql.replace(binding, replacement)
-      } else {
-        sql = sql.replace(binding, integration.getBindingIdentifier())
-      }
-      variables.push(binding)
-    }
-    // replicate the knex structure
-    fields.sql = sql
-    fields.bindings = this.enrichQueryFields(variables, parameters)
-    // check for arrays in the data
-    let updated = []
-    for (let i = 0; i < variables.length; i++) {
-      if (arrays.includes(variables[i])) {
-        updated = updated.concat(fields.bindings[i].split(","))
-      } else {
-        updated.push(fields.bindings[i])
-      }
-    }
-    fields.bindings = updated
-    return fields
-  }
-
   async execute() {
     let { datasource, fields, queryVerb, transformer } = this
     const Integration = integrations[datasource.source]
@@ -112,9 +41,9 @@ class QueryRunner {
     let query
     // handle SQL injections by interpolating the variables
     if (isSQL(datasource)) {
-      query = this.interpolateSQL(fields, parameters, integration)
+      query = interpolateSQL(fields, parameters, integration)
     } else {
-      query = this.enrichQueryFields(fields, parameters)
+      query = enrichQueryFields(fields, parameters)
     }
 
     // Add pagination values for REST queries
@@ -259,47 +188,6 @@ class QueryRunner {
     }
     return parameters
   }
-
-  enrichQueryFields(fields, parameters = {}) {
-    const enrichedQuery = Array.isArray(fields) ? [] : {}
-
-    // enrich the fields with dynamic parameters
-    for (let key of Object.keys(fields)) {
-      if (fields[key] == null) {
-        continue
-      }
-      if (typeof fields[key] === "object") {
-        // enrich nested fields object
-        enrichedQuery[key] = this.enrichQueryFields(fields[key], parameters)
-      } else if (typeof fields[key] === "string") {
-        // enrich string value as normal
-        enrichedQuery[key] = processStringSync(fields[key], parameters, {
-          noEscaping: true,
-          noHelpers: true,
-          escapeNewlines: true,
-        })
-      } else {
-        enrichedQuery[key] = fields[key]
-      }
-    }
-    if (
-      enrichedQuery.json ||
-      enrichedQuery.customData ||
-      enrichedQuery.requestBody
-    ) {
-      try {
-        enrichedQuery.json = JSON.parse(
-          enrichedQuery.json ||
-            enrichedQuery.customData ||
-            enrichedQuery.requestBody
-        )
-      } catch (err) {
-        // no json found, ignore
-      }
-      delete enrichedQuery.customData
-    }
-    return enrichedQuery
-  }
 }
 
 module.exports = (input, callback) => {