Checkpoint EOD: fixed a bunch more raw cases, some test failures to fix tomorrow.

packages/backend-core/src/sql/sql.ts

@@ -13,6 +13,7 @@ import SqlTableQueryBuilder from "./sqlTable"
 import {
   Aggregation,
   AnySearchFilter,
+  ArrayFilter,
   ArrayOperator,
   BasicOperator,
   BBReferenceFieldMetadata,
@@ -152,30 +153,24 @@ class InternalBuilder {
     return this.query.meta.table
   }
 
+  get knexClient(): Knex.Client {
+    return this.knex.client as Knex.Client
+  }
+
   getFieldSchema(key: string): FieldSchema | undefined {
     const { column } = this.splitter.run(key)
     return this.table.schema[column]
   }
 
   private quoteChars(): [string, string] {
-    switch (this.client) {
+    const wrapped = this.knexClient.wrapIdentifier("foo", {})
-      case SqlClient.ORACLE:
+    return [wrapped[0], wrapped[wrapped.length - 1]]
-      case SqlClient.POSTGRES:
-        return ['"', '"']
-      case SqlClient.MS_SQL:
-        return ["[", "]"]
-      case SqlClient.MARIADB:
-      case SqlClient.MY_SQL:
-      case SqlClient.SQL_LITE:
-        return ["`", "`"]
-    }
   }
 
-  // Takes a string like foo and returns a quoted string like [foo] for SQL Server
+  // Takes a string like foo and returns a quoted string like [foo] for SQL
-  // and "foo" for Postgres.
+  // Server and "foo" for Postgres.
   private quote(str: string): string {
-    const [start, end] = this.quoteChars()
+    return this.knexClient.wrapIdentifier(str, {})
-    return `${start}${str}${end}`
   }
 
   private isQuoted(key: string): boolean {
@@ -193,6 +188,21 @@ class InternalBuilder {
     return key.map(part => this.quote(part)).join(".")
   }
 
+  // Unfortuantely we cannot rely on knex's identifier escaping because it trims
+  // the identifier string before escaping it, which breaks cases for us where
+  // columns that start or end with a space aren't referenced correctly anymore.
+  //
+  // So whenever you're using an identifier binding in knex, e.g. knex.raw("??
+  // as ?", ["foo", "bar"]), you need to make sure you call this:
+  //
+  //   knex.raw("?? as ?", [this.quotedIdentifier("foo"), "bar"])
+  //
+  // Issue we filed against knex about this:
+  //   https://github.com/knex/knex/issues/6143
+  private rawQuotedIdentifier(key: string): Knex.Raw {
+    return this.knex.raw(this.quotedIdentifier(key))
+  }
+
   // Turns an identifier like a.b.c or `a`.`b`.`c` into ["a", "b", "c"]
   private splitIdentifier(key: string): string[] {
     const [start, end] = this.quoteChars()
@@ -261,7 +271,7 @@ class InternalBuilder {
           // TODO: figure out how to express this safely without string
           // interpolation.
           return this.knex.raw(`??::money::numeric as "${field}"`, [
-            [table, column].join("."),
+            this.rawQuotedIdentifier([table, column].join(".")),
             field,
           ])
         }
@@ -273,14 +283,14 @@ class InternalBuilder {
           // TODO: figure out how to express this safely without string
           // interpolation.
           return this.knex.raw(`CONVERT(varchar, ??, 108) as "${field}"`, [
-            field,
+            this.rawQuotedIdentifier(field),
           ])
         }
 
         if (table) {
-          return this.knex.raw("??", [`${table}.${column}`])
+          return this.rawQuotedIdentifier(`${table}.${column}`)
         } else {
-          return this.knex.raw("??", [field])
+          return this.rawQuotedIdentifier(field)
         }
       })
   }
@@ -300,7 +310,7 @@ class InternalBuilder {
     const parts = this.splitIdentifier(field)
     const col = parts.pop()!
     const schema = this.table.schema[col]
-    let identifier = this.knex.raw("??", [field])
+    let identifier = this.rawQuotedIdentifier(field)
 
     if (
       schema.type === FieldType.STRING ||
@@ -311,7 +321,10 @@ class InternalBuilder {
       schema.type === FieldType.BARCODEQR
     ) {
       if (opts?.forSelect) {
-        identifier = this.knex.raw("to_char(??) as ??", [identifier, col])
+        identifier = this.knex.raw("to_char(??) as ??", [
+          identifier,
+          this.rawQuotedIdentifier(col),
+        ])
       } else {
         identifier = this.knex.raw("to_char(??)", [identifier])
       }
@@ -437,7 +450,6 @@ class InternalBuilder {
     filterKey: string,
     whereCb: (filterKey: string, query: Knex.QueryBuilder) => Knex.QueryBuilder
   ): Knex.QueryBuilder {
-    const mainKnex = this.knex
     const { relationships, endpoint, tableAliases: aliases } = this.query
     const tableName = endpoint.entityId
     const fromAlias = aliases?.[tableName] || tableName
@@ -459,8 +471,8 @@ class InternalBuilder {
         relationship.to &&
         relationship.tableName
       ) {
-        const joinTable = mainKnex
+        const joinTable = this.knex
-          .select(mainKnex.raw(1))
+          .select(this.knex.raw(1))
           .from({ [toAlias]: relatedTableName })
         let subQuery = joinTable.clone()
         const manyToMany = validateManyToMany(relationship)
@@ -495,7 +507,7 @@ class InternalBuilder {
             .where(
               `${throughAlias}.${manyToMany.from}`,
               "=",
-              mainKnex.raw(`??`, [`${fromAlias}.${manyToMany.fromPrimary}`])
+              this.rawQuotedIdentifier(`${fromAlias}.${manyToMany.fromPrimary}`)
             )
           // in SQS the same junction table is used for different many-to-many relationships between the
           // two same tables, this is needed to avoid rows ending up in all columns
@@ -524,7 +536,7 @@ class InternalBuilder {
           subQuery = subQuery.where(
             toKey,
             "=",
-            mainKnex.raw("??", [foreignKey])
+            this.rawQuotedIdentifier(foreignKey)
           )
 
           query = query.where(q => {
@@ -641,15 +653,30 @@ class InternalBuilder {
         this.client === SqlClient.ORACLE ||
         this.client === SqlClient.SQL_LITE
       ) {
-        return q.whereRaw(`LOWER(??) LIKE ?`, [key, `%${value.toLowerCase()}%`])
+        return q.whereRaw(`LOWER(??) LIKE ?`, [
+          this.rawQuotedIdentifier(key),
+          `%${value.toLowerCase()}%`,
+        ])
       }
-      return q.whereILike(key, this.knex.raw("?", [`%${value}%`]))
+      return q.whereILike(
+        // @ts-expect-error knex types are wrong, raw is fine here
+        this.rawQuotedIdentifier(key),
+        this.knex.raw("?", [`%${value}%`])
+      )
     }
 
-    const contains = (mode: AnySearchFilter, any: boolean = false) => {
+    const contains = (mode: ArrayFilter, any = false) => {
-      const rawFnc = allOr ? "orWhereRaw" : "whereRaw"
+      function addModifiers<T extends {}, Q>(q: Knex.QueryBuilder<T, Q>) {
-      const not = mode === filters?.notContains ? "NOT " : ""
+        if (allOr || mode === filters?.containsAny) {
-      function stringifyArray(value: Array<any>, quoteStyle = '"'): string {
+          q = q.or
+        }
+        if (mode === filters?.notContains) {
+          q = q.not
+        }
+        return q
+      }
+
+      function stringifyArray(value: any[], quoteStyle = '"'): string {
         for (let i in value) {
           if (typeof value[i] === "string") {
             value[i] = `${quoteStyle}${value[i]}${quoteStyle}`
@@ -657,60 +684,64 @@ class InternalBuilder {
         }
         return `[${value.join(",")}]`
       }
+
       if (this.client === SqlClient.POSTGRES) {
         iterate(mode, ArrayOperator.CONTAINS, (q, key, value) => {
           const wrap = any ? "" : "'"
           const op = any ? "\\?| array" : "@>"
-          const fieldNames = key.split(/\./g)
+
-          const table = fieldNames[0]
+          const stringifiedArray = stringifyArray(value, any ? "'" : '"')
-          const col = fieldNames[1]
+          return addModifiers(q).whereRaw(
-          return q[rawFnc](
+            `COALESCE(??::jsonb ${op} ${wrap}${stringifiedArray}${wrap}, FALSE)`,
-            `${not}COALESCE("${table}"."${col}"::jsonb ${op} ${wrap}${stringifyArray(
+            [this.rawQuotedIdentifier(key)]
-              value,
-              any ? "'" : '"'
-            )}${wrap}, FALSE)`
           )
         })
       } else if (
         this.client === SqlClient.MY_SQL ||
         this.client === SqlClient.MARIADB
       ) {
-        const jsonFnc = any ? "JSON_OVERLAPS" : "JSON_CONTAINS"
         iterate(mode, ArrayOperator.CONTAINS, (q, key, value) => {
-          return q[rawFnc](
+          return addModifiers(q).whereRaw(`COALESCE(?(??, ?), FALSE)`, [
-            `${not}COALESCE(${jsonFnc}(${key}, '${stringifyArray(
+            this.knex.raw(any ? "JSON_OVERLAPS" : "JSON_CONTAINS"),
-              value
+            this.rawQuotedIdentifier(key),
-            )}'), FALSE)`
+            stringifyArray(value),
-          )
+          ])
         })
       } else {
-        const andOr = mode === filters?.containsAny ? " OR " : " AND "
         iterate(mode, ArrayOperator.CONTAINS, (q, key, value) => {
-          let statement = ""
+          if (value.length === 0) {
-          const identifier = this.quotedIdentifier(key)
-          for (let i in value) {
-            if (typeof value[i] === "string") {
-              value[i] = `%"${value[i].toLowerCase()}"%`
-            } else {
-              value[i] = `%${value[i]}%`
-            }
-            statement += `${
-              statement ? andOr : ""
-            }COALESCE(LOWER(${identifier}), '') LIKE ?`
-          }
-
-          if (statement === "") {
             return q
           }
 
-          if (not) {
+          q = addModifiers(q).where(subQuery => {
-            return q[rawFnc](
+            subQuery.where(subSubQuery => {
-              `(NOT (${statement}) OR ${identifier} IS NULL)`,
+              for (const elem of value) {
-              value
+                if (mode === filters?.containsAny) {
-            )
+                  subSubQuery = subSubQuery.or
-          } else {
+                } else {
-            return q[rawFnc](statement, value)
+                  subSubQuery = subSubQuery.and
+                }
+
+                const lower =
+                  typeof elem === "string" ? `"${elem.toLowerCase()}"` : elem
+
+                subSubQuery = subSubQuery.whereLike(
+                  // @ts-expect-error knex types are wrong, raw is fine here
+                  this.knex.raw(`COALESCE(LOWER(??), '')`, [
+                    this.rawQuotedIdentifier(key),
+                  ]),
+                  `%${lower}%`
+                )
+              }
+            })
+          })
+
+          if (mode === filters?.notContains) {
+            // @ts-expect-error knex types are wrong, raw is fine here
+            q.or.whereNull(this.rawQuotedIdentifier(key))
           }
+
+          return q
         })
       }
     }
@@ -764,7 +795,10 @@ class InternalBuilder {
           return q[fnc](key, "ilike", `${value}%`)
         } else {
           const fnc = allOr ? "orWhereRaw" : "whereRaw"
-          return q[fnc](`LOWER(??) LIKE ?`, [key, `${value.toLowerCase()}%`])
+          return q[fnc](`LOWER(??) LIKE ?`, [
+            this.rawQuotedIdentifier(key),
+            `${value.toLowerCase()}%`,
+          ])
         }
       })
     }
@@ -801,7 +835,9 @@ class InternalBuilder {
           this.client === SqlClient.SQL_LITE &&
           schema?.type === FieldType.BIGINT
         ) {
-          rawKey = this.knex.raw("CAST(?? AS INTEGER)", [key])
+          rawKey = this.knex.raw("CAST(?? AS INTEGER)", [
+            this.rawQuotedIdentifier(key),
+          ])
           high = this.knex.raw("CAST(? AS INTEGER)", [value.high])
           low = this.knex.raw("CAST(? AS INTEGER)", [value.low])
         }
@@ -1082,7 +1118,9 @@ class InternalBuilder {
     }
 
     const separator = this.client === SqlClient.ORACLE ? " VALUE " : ","
-    return this.knex.raw(`?${separator}??`, [unaliased, tableField]).toString()
+    return this.knex
+      .raw(`?${separator}??`, [unaliased, this.rawQuotedIdentifier(tableField)])
+      .toString()
   }
 
   maxFunctionParameters() {
@@ -1178,7 +1216,7 @@ class InternalBuilder {
       subQuery = subQuery.where(
         correlatedTo,
         "=",
-        knex.raw("??", [correlatedFrom])
+        this.rawQuotedIdentifier(correlatedFrom)
       )
 
       const standardWrap = (select: Knex.Raw): Knex.QueryBuilder => {
@@ -1228,7 +1266,7 @@ class InternalBuilder {
 
           wrapperQuery = knex.raw(
             `(SELECT ?? = (${comparatorQuery} FOR JSON PATH))`,
-            [toAlias]
+            [this.rawQuotedIdentifier(toAlias)]
           )
           break
         }

packages/server/src/api/routes/tests/search.spec.ts

@@ -691,7 +691,7 @@ describe.each([
           ])
         })
 
-        it("should not match the session user id in a multi user field", async () => {
+        it.only("should not match the session user id in a multi user field", async () => {
           await expectQuery({
             notContains: { multi_user: ["{{ [user]._id }}"] },
             notEmpty: { multi_user: true },
@@ -793,7 +793,7 @@ describe.each([
       })
 
     const stringTypes = [FieldType.STRING, FieldType.LONGFORM] as const
-    describe.only.each(stringTypes)("%s", type => {
+    describe.each(stringTypes)("%s", type => {
       beforeAll(async () => {
         tableOrViewId = await createTableOrView({
           name: { name: "name", type },

packages/types/src/sdk/search.ts

@@ -49,7 +49,7 @@ type BasicFilter<T = any> = Record<string, T> & {
   [InternalSearchFilterOperator.COMPLEX_ID_OPERATOR]?: never
 }
 
-type ArrayFilter = Record<string, any[]> & {
+export type ArrayFilter = Record<string, any[]> & {
   [InternalSearchFilterOperator.COMPLEX_ID_OPERATOR]?: {
     id: string[]
     values: string[]