fix(db-d1-sqlite): avoid bound parameter limit when querying relationships and inserting rows (#14099)

Fixes
https://discord.com/channels/967097582721572934/1422639568808841329/1425037080051978261

This PR avoids bound parameters usage for `IN` and `NOT_IN` querying on
`id` to respect the D1 limit
https://developers.cloudflare.com/d1/platform/limits/
And also batches inserts when inserting arrays/blocks/hasMany
relationships etc. This is needed because we can't avoid using bound
parameters there, but still want to respect the 100 variables limit per
query.

Author: r1tsuu

packages/db-d1-sqlite/src/index.ts

@@ -109,6 +109,7 @@ export function sqliteD1Adapter(args: Args): DatabaseAdapterObj<SQLiteD1Adapter>
       }),
       idType: sqliteIDType,
       initializing,
+      limitedBoundParameters: true,
       localesSuffix: args.localesSuffix || '_locales',
       logger: args.logger,
       operators,

packages/drizzle/src/queries/parseParams.ts

@@ -3,13 +3,14 @@ import type { FlattenedField, Operator, Sort, Where } from 'payload'
 
 import { and, isNotNull, isNull, ne, notInArray, or, sql } from 'drizzle-orm'
 import { PgUUID } from 'drizzle-orm/pg-core'
-import { QueryError } from 'payload'
+import { APIError, QueryError } from 'payload'
 import { validOperatorSet } from 'payload/shared'
 
 import type { DrizzleAdapter, GenericColumn } from '../types.js'
 import type { BuildQueryJoinAliases } from './buildQuery.js'
 
 import { getNameFromDrizzleTable } from '../utilities/getNameFromDrizzleTable.js'
+import { isValidStringID } from '../utilities/isValidStringID.js'
 import { DistinctSymbol } from '../utilities/rawConstraint.js'
 import { buildAndOrConditions } from './buildAndOrConditions.js'
 import { getTableColumnFromPath } from './getTableColumnFromPath.js'
@@ -387,10 +388,59 @@ export function parseParams({
                   orConditions.push(isNull(resolvedColumn))
                   resolvedQueryValue = queryValue.filter((v) => v !== null)
                 }
+
                 let constraint = adapter.operators[queryOperator](
                   resolvedColumn,
                   resolvedQueryValue,
                 )
+
+                if (
+                  adapter.limitedBoundParameters &&
+                  (operator === 'in' || operator === 'not_in') &&
+                  relationOrPath === 'id' &&
+                  Array.isArray(queryValue)
+                ) {
+                  let isInvalid = false
+                  for (const val of queryValue) {
+                    if (typeof val === 'number' || val === null) {
+                      continue
+                    }
+                    if (typeof val === 'string') {
+                      if (!isValidStringID(val)) {
+                        isInvalid = true
+                        break
+                      } else {
+                        continue
+                      }
+                    }
+                    isInvalid = true
+                    break
+                  }
+
+                  if (isInvalid) {
+                    throw new APIError(`Invalid ID value in ${JSON.stringify(queryValue)}`)
+                  }
+
+                  constraints.push(
+                    sql.raw(
+                      `${resolvedColumn.name} ${operator === 'in' ? 'IN' : 'NOT IN'} (${queryValue
+                        .map((e) => {
+                          if (e === null) {
+                            return `NULL`
+                          }
+
+                          if (typeof e === 'number') {
+                            return e
+                          }
+
+                          return `'${e}'`
+                        })
+                        .join(',')})`,
+                    ),
+                  )
+                  break
+                }
+
                 if (orConditions.length) {
                   orConditions.push(constraint)
                   constraint = or(...orConditions)

packages/drizzle/src/sqlite/insert.ts

@@ -8,6 +8,26 @@ export const insert: Insert = async function (
 ): Promise<Record<string, unknown>[]> {
   const table = this.tables[tableName]
 
+  // Batch insert if limitedBoundParameters: true
+  if (this.limitedBoundParameters && Array.isArray(values)) {
+    const results: Record<string, unknown>[] = []
+    const colsPerRow = Object.keys(values[0]).length
+    const maxParams = 100
+    const maxRowsPerBatch = Math.max(1, Math.floor(maxParams / colsPerRow))
+
+    for (let i = 0; i < values.length; i += maxRowsPerBatch) {
+      const batch = values.slice(i, i + maxRowsPerBatch)
+
+      const batchResult = onConflictDoUpdate
+        ? await db.insert(table).values(batch).onConflictDoUpdate(onConflictDoUpdate).returning()
+        : await db.insert(table).values(batch).returning()
+
+      results.push(...(batchResult as Record<string, unknown>[]))
+    }
+
+    return results
+  }
+
   const result = onConflictDoUpdate
     ? await db.insert(table).values(values).onConflictDoUpdate(onConflictDoUpdate).returning()
     : await db.insert(table).values(values).returning()

packages/drizzle/src/types.ts

@@ -344,8 +344,8 @@ export interface DrizzleAdapter extends BaseDatabaseAdapter {
   drizzle: LibSQLDatabase | PostgresDB
   dropDatabase: DropDatabase
   enums?: never | Record<string, unknown>
-
   execute: Execute<unknown>
+
   features: {
     json?: boolean
   }
@@ -358,6 +358,7 @@ export interface DrizzleAdapter extends BaseDatabaseAdapter {
   indexes: Set<string>
   initializing: Promise<void>
   insert: Insert
+  limitedBoundParameters?: boolean
   localesSuffix?: string
   logger: DrizzleConfig['logger']
   operators: Operators

packages/drizzle/src/utilities/isValidStringID.spec.ts

@@ -0,0 +1,14 @@
+import { isValidStringID } from './isValidStringID.js'
+
+describe('isValidStringID', () => {
+  it('should pass', () => {
+    expect(isValidStringID('1')).toBe(true)
+    expect(isValidStringID('a_b_c')).toBe(true)
+    expect(isValidStringID('8cc2df6d-6e07-4da4-be48-5fa747c3b92b')).toBe(true)
+  })
+
+  it('should not pass', () => {
+    expect(isValidStringID('1 2 3')).toBe(false)
+    expect(isValidStringID('1@')).toBe(false)
+  })
+})

packages/drizzle/src/utilities/isValidStringID.ts

@@ -0,0 +1,3 @@
+export function isValidStringID(value: string) {
+  return /^[\w-]+$/.test(value)
+}

test/database/sqlite-bound-parameters-limit.int.spec.ts

@@ -0,0 +1,99 @@
+import type { Payload } from 'payload'
+
+/* eslint-disable jest/require-top-level-describe */
+import path from 'path'
+import { fileURLToPath } from 'url'
+
+import { initPayloadInt } from '../helpers/initPayloadInt.js'
+
+const filename = fileURLToPath(import.meta.url)
+const dirname = path.dirname(filename)
+
+const describeSqlite = process.env.PAYLOAD_DATABASE?.startsWith('sqlite') ? describe : describe.skip
+
+let payload: Payload
+
+describeSqlite('database - sqlite bound parameters limit', () => {
+  beforeAll(async () => {
+    ;({ payload } = await initPayloadInt(dirname))
+  })
+
+  afterAll(async () => {
+    await payload.destroy()
+  })
+
+  it('should not use bound parameters for where querying on ID with IN if limitedBoundParameters: true', async () => {
+    const defaultExecute = payload.db.drizzle.$client.execute.bind(payload.db.drizzle.$client)
+
+    // Limit bounds parameters length
+    payload.db.drizzle.$client.execute = async function execute(...args) {
+      const res = await defaultExecute(...args)
+      const [{ args: boundParameters }] = args as [{ args: any[] }]
+
+      // eslint-disable-next-line jest/no-conditional-in-test
+      if (boundParameters.length > 100) {
+        throw new Error('Exceeded limit of bound parameters!')
+      }
+      return res
+    }
+
+    payload.db.limitedBoundParameters = false
+
+    const IN = Array.from({ length: 300 }, (_, i) => i)
+
+    // Should fail here because too the length exceeds the limit
+    await expect(
+      payload.find({
+        collection: 'simple',
+        pagination: false,
+        where: { id: { in: IN } },
+      }),
+    ).rejects.toBeTruthy()
+
+    // Should fail here because too the length exceeds the limit
+    await expect(
+      payload.find({
+        collection: 'simple',
+        pagination: false,
+        where: { id: { not_in: IN } },
+      }),
+    ).rejects.toBeTruthy()
+
+    payload.db.limitedBoundParameters = true
+
+    // Should not fail because limitedBoundParameters: true
+    await expect(
+      payload.find({
+        collection: 'simple',
+        pagination: false,
+        where: { id: { in: IN } },
+      }),
+    ).resolves.toBeTruthy()
+
+    // Should not fail because limitedBoundParameters: true
+    await expect(
+      payload.find({
+        collection: 'simple',
+        pagination: false,
+        where: { id: { not_in: IN } },
+      }),
+    ).resolves.toBeTruthy()
+
+    // Verify that "in" still works properly
+
+    const docs = await Promise.all(
+      Array.from({ length: 300 }, () => payload.create({ collection: 'simple', data: {} })),
+    )
+
+    const res = await payload.find({
+      collection: 'simple',
+      pagination: false,
+      where: { id: { in: docs.map((e) => e.id) } },
+    })
+
+    expect(res.totalDocs).toBe(300)
+    for (const docInRes of res.docs) {
+      expect(docs.some((doc) => doc.id === docInRes.id)).toBeTruthy()
+    }
+  })
+})