Adding test cases for interpolation of SQL, confirming that the context correctly gets cleaned up before passing into bindings.

packages/server/src/api/controllers/query/index.ts

@@ -105,6 +105,12 @@ export async function save(ctx: UserCtx<Query, Query>) {
     query.nullDefaultSupport = true
     eventFn = () => events.query.created(datasource, query)
   } else {
+    // check if flag has previously been set, don't let it change
+    // allow it to be explicitly set to false via API incase this is ever needed
+    const existingQuery = await db.get<Query>(query._id)
+    if (existingQuery.nullDefaultSupport && query.nullDefaultSupport == null) {
+      query.nullDefaultSupport = true
+    }
     eventFn = () => events.query.updated(datasource, query)
   }
   const response = await db.put(query)
@@ -285,6 +291,7 @@ export async function preview(
       parameters: enrichParameters(query),
       transformer: query.transformer,
       schema: query.schema,
+      nullDefaultSupport: query.nullDefaultSupport,
       queryId,
       datasource,
       // have to pass down to the thread runner - can't put into context now
@@ -353,6 +360,7 @@ async function execute(
       queryId: ctx.params.queryId,
       // have to pass down to the thread runner - can't put into context now
       environmentVariables: envVars,
+      nullDefaultSupport: query.nullDefaultSupport,
       ctx: {
         user: ctx.user,
         auth: { ...authConfigCtx },

packages/server/src/api/routes/tests/queries/generic-sql.spec.ts

@@ -12,31 +12,51 @@ const createTableSQL: Record<string, string> = {
     CREATE TABLE test_table (
         id serial PRIMARY KEY,
         name VARCHAR ( 50 ) NOT NULL,
-        birthday TIMESTAMP
+        birthday TIMESTAMP,
+        number INT
     );`,
   [SourceName.MYSQL]: `
     CREATE TABLE test_table (
         id INT AUTO_INCREMENT PRIMARY KEY,
         name VARCHAR(50) NOT NULL,
-        birthday TIMESTAMP
+        birthday TIMESTAMP,
+        number INT
     );`,
   [SourceName.SQL_SERVER]: `
     CREATE TABLE test_table (
         id INT IDENTITY(1,1) PRIMARY KEY,
         name NVARCHAR(50) NOT NULL,
-        birthday DATETIME
+        birthday DATETIME,
+        number INT
     );`,
 }
 
 const insertSQL = `INSERT INTO test_table (name) VALUES ('one'), ('two'), ('three'), ('four'), ('five')`
 const dropTableSQL = `DROP TABLE test_table;`
 
+const POSTGRES_SPECIFICS = {
+  nullError: 'invalid input syntax for type integer: ""',
+}
+
+const MYSQL_SPECIFICS = {
+  nullError: "Incorrect integer value: '' for column 'number' at row 1",
+}
+
+const MSSQL_SPECIFICS = {
+  nullError: "Cannot convert undefined or null to object",
+}
+
+const MARIADB_SPECIFICS = {
+  nullError:
+    "Incorrect integer value: '' for column `mysql`.`test_table`.`number` at row 1",
+}
+
 describe.each([
-  ["postgres", databaseTestProviders.postgres],
+  ["postgres", databaseTestProviders.postgres, POSTGRES_SPECIFICS],
-  ["mysql", databaseTestProviders.mysql],
+  ["mysql", databaseTestProviders.mysql, MYSQL_SPECIFICS],
-  ["mssql", databaseTestProviders.mssql],
+  ["mssql", databaseTestProviders.mssql, MSSQL_SPECIFICS],
-  ["mariadb", databaseTestProviders.mariadb],
+  ["mariadb", databaseTestProviders.mariadb, MARIADB_SPECIFICS],
-])("queries (%s)", (__, dsProvider) => {
+])("queries (%s)", (__, dsProvider, testSpecificInformation) => {
   const config = setup.getConfig()
   let datasource: Datasource
 
@@ -51,7 +71,7 @@ describe.each([
       transformer: "return data",
       readable: true,
     }
-    return await config.api.query.create({ ...defaultQuery, ...query })
+    return await config.api.query.save({ ...defaultQuery, ...query })
   }
 
   async function rawQuery(sql: string): Promise<any> {
@@ -398,4 +418,51 @@ describe.each([
       expect(rows).toHaveLength(0)
     })
   })
+
+  // this parameter really only impacts SQL queries
+  describe("confirm nullDefaultSupport", () => {
+    const queryParams = {
+      fields: {
+        sql: "INSERT INTO test_table (name, number) VALUES ({{ bindingName }}, {{ bindingNumber }})",
+      },
+      parameters: [
+        {
+          name: "bindingName",
+          default: "",
+        },
+        {
+          name: "bindingNumber",
+          default: "",
+        },
+      ],
+      queryVerb: "create",
+    }
+
+    it("should error for old queries", async () => {
+      const query = await createQuery(queryParams)
+      await config.api.query.save({ ...query, nullDefaultSupport: false })
+      let error: string | undefined
+      try {
+        await config.api.query.execute(query._id!, {
+          parameters: {
+            bindingName: "testing",
+          },
+        })
+      } catch (err: any) {
+        error = err.message
+      }
+      expect(error).toBeDefined()
+      expect(error).toContain(testSpecificInformation.nullError)
+    })
+
+    it("should not error for new queries", async () => {
+      const query = await createQuery(queryParams)
+      const results = await config.api.query.execute(query._id!, {
+        parameters: {
+          bindingName: "testing",
+        },
+      })
+      expect(results).toEqual({ data: [{ created: true }] })
+    })
+  })
 })

packages/server/src/api/routes/tests/queries/mongodb.spec.ts

@@ -33,7 +33,7 @@ describe("/queries", () => {
     ) {
       combinedQuery.fields.extra.collection = collection
     }
-    return await config.api.query.create(combinedQuery)
+    return await config.api.query.save(combinedQuery)
   }
 
   async function withClient(

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

@@ -1,13 +1,15 @@
 import { findHBSBlocks } from "@budibase/string-templates"
 import { DatasourcePlus } from "@budibase/types"
 import sdk from "../../sdk"
+import { enrichArrayContext } from "../../sdk/app/queries/queries"
 
 const CONST_CHAR_REGEX = new RegExp("'[^']*'", "g")
 
 export async function interpolateSQL(
-  fields: { [key: string]: any },
+  fields: { sql: string; bindings: any[] },
   parameters: { [key: string]: any },
-  integration: DatasourcePlus
+  integration: DatasourcePlus,
+  opts: { nullDefaultSupport: boolean }
 ) {
   let sql = fields.sql
   if (!sql || typeof sql !== "string") {
@@ -64,7 +66,14 @@ export async function interpolateSQL(
   }
   // replicate the knex structure
   fields.sql = sql
-  fields.bindings = await sdk.queries.enrichContext(variables, parameters)
+  fields.bindings = await sdk.queries.enrichArrayContext(variables, parameters)
+  if (opts.nullDefaultSupport) {
+    for (let index in fields.bindings) {
+      if (fields.bindings[index] === "") {
+        fields.bindings[index] = null
+      }
+    }
+  }
   // check for arrays in the data
   let updated: string[] = []
   for (let i = 0; i < variables.length; i++) {

packages/server/src/sdk/app/queries/queries.ts

@@ -65,11 +65,27 @@ export async function fetch(opts: { enrich: boolean } = { enrich: true }) {
   return updateSchemas(queries)
 }
 
+export async function enrichArrayContext(
+  fields: any[],
+  inputs = {}
+): Promise<any[]> {
+  const map: Record<string, any> = {}
+  for (let [key, value] of Object.entries(fields)) {
+    map[key] = value
+  }
+  const output = await enrichContext(map, inputs)
+  const outputArray: any[] = []
+  for (let [key, value] of Object.entries(output)) {
+    outputArray[parseInt(key)] = value
+  }
+  return outputArray
+}
+
 export async function enrichContext(
   fields: Record<string, any>,
   inputs = {}
 ): Promise<Record<string, any>> {
-  const enrichedQuery: Record<string, any> = Array.isArray(fields) ? [] : {}
+  const enrichedQuery: Record<string, any> = {}
   if (!fields || !inputs) {
     return enrichedQuery
   }

packages/server/src/tests/utilities/api/query.ts

@@ -8,7 +8,7 @@ import {
 import { TestAPI } from "./base"
 
 export class QueryAPI extends TestAPI {
-  create = async (body: Query): Promise<Query> => {
+  save = async (body: Query): Promise<Query> => {
     return await this._post<Query>(`/api/queries`, { body })
   }
 

packages/server/src/threads/query.ts

@@ -26,10 +26,11 @@ class QueryRunner {
   fields: any
   parameters: any
   pagination: any
-  transformer: string
+  transformer: string | null
   cachedVariables: any[]
   ctx: any
   queryResponse: any
+  nullDefaultSupport: boolean
   noRecursiveQuery: boolean
   hasRerun: boolean
   hasRefreshedOAuth: boolean
@@ -45,6 +46,7 @@ class QueryRunner {
     this.transformer = input.transformer
     this.queryId = input.queryId!
     this.schema = input.schema
+    this.nullDefaultSupport = !!input.nullDefaultSupport
     this.noRecursiveQuery = flags.noRecursiveQuery
     this.cachedVariables = []
     // Additional context items for enrichment
@@ -59,7 +61,14 @@ class QueryRunner {
   }
 
   async execute(): Promise<QueryResponse> {
-    let { datasource, fields, queryVerb, transformer, schema } = this
+    let {
+      datasource,
+      fields,
+      queryVerb,
+      transformer,
+      schema,
+      nullDefaultSupport,
+    } = this
     let datasourceClone = cloneDeep(datasource)
     let fieldsClone = cloneDeep(fields)
 
@@ -103,7 +112,9 @@ class QueryRunner {
     let query
     // handle SQL injections by interpolating the variables
     if (isSQL(datasourceClone)) {
-      query = await interpolateSQL(fieldsClone, enrichedContext, integration)
+      query = await interpolateSQL(fieldsClone, enrichedContext, integration, {
+        nullDefaultSupport,
+      })
     } else {
       query = await sdk.queries.enrichContext(fieldsClone, enrichedContext)
     }
@@ -137,7 +148,9 @@ class QueryRunner {
         data: rows,
         params: enrichedParameters,
       }
-      rows = vm.withContext(ctx, () => vm.execute(transformer))
+      if (transformer != null) {
+        rows = vm.withContext(ctx, () => vm.execute(transformer!))
+      }
     }
 
     // if the request fails we retry once, invalidating the cached value
@@ -191,13 +204,15 @@ class QueryRunner {
     })
     return new QueryRunner(
       {
-        datasource,
+        schema: query.schema,
         queryVerb: query.queryVerb,
         fields: query.fields,
-        parameters,
         transformer: query.transformer,
-        queryId,
+        nullDefaultSupport: query.nullDefaultSupport,
         ctx: this.ctx,
+        parameters,
+        datasource,
+        queryId,
       },
       { noRecursiveQuery: true }
     ).execute()