Skip to content

Commit 4f5a9c2

Browse files
r1tsuur1tsuu
authored
fix(drizzle): d1 sqlite IN querying of id when any other join is present in the query (#15290)
Fixes #15196
1 parent 3dbaed8 commit 4f5a9c2

3 files changed

Lines changed: 121 additions & 74 deletions

File tree

‎packages/drizzle/src/queries/parseParams.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import type { SQL, Table } from 'drizzle-orm'
22
import type { FlattenedField, Operator, Sort, Where } from 'payload'
33

4-
import { and, isNotNull, isNull, ne, notInArray, or, sql } from 'drizzle-orm'
4+
import { and, getTableName, isNotNull, isNull, ne, notInArray, or, sql } from 'drizzle-orm'
55
import { PgUUID } from 'drizzle-orm/pg-core'
66
import { APIError, QueryError } from 'payload'
77
import { validOperatorSet } from 'payload/shared'
@@ -423,7 +423,7 @@ export function parseParams({
423423

424424
constraints.push(
425425
sql.raw(
426-
`${resolvedColumn.name} ${operator === 'in' ? 'IN' : 'NOT IN'} (${queryValue
426+
`"${getTableName(resolvedColumn.table)}"."${resolvedColumn.name}" ${operator === 'in' ? 'IN' : 'NOT IN'} (${queryValue
427427
.map((e) => {
428428
if (e === null) {
429429
return `NULL`

‎test/database/getConfig.ts‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,20 @@ export const getConfig: () => Partial<Config> = () => ({
107107
},
108108
],
109109
},
110+
{
111+
slug: 'simple-localized',
112+
fields: [
113+
{
114+
type: 'text',
115+
localized: true,
116+
name: 'text',
117+
},
118+
{
119+
type: 'number',
120+
name: 'number',
121+
},
122+
],
123+
},
110124
{
111125
slug: 'categories-custom-id',
112126
versions: { drafts: true },

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

Lines changed: 105 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -2,97 +2,130 @@ import type { Payload } from 'payload'
22

33
import path from 'path'
44
import { fileURLToPath } from 'url'
5-
import { afterAll, beforeAll, describe, expect, it } from 'vitest'
5+
import { afterAll, beforeAll, expect, it } from 'vitest'
66

77
import { initPayloadInt } from '../helpers/initPayloadInt.js'
8+
import { describe } from '../helpers/vitest.js'
89

910
const filename = fileURLToPath(import.meta.url)
1011
const dirname = path.dirname(filename)
1112

12-
const describeSqlite = process.env.PAYLOAD_DATABASE?.startsWith('sqlite') ? describe : describe.skip
13-
1413
let payload: Payload
1514

16-
describeSqlite('database - sqlite bound parameters limit', () => {
17-
beforeAll(async () => {
18-
;({ payload } = await initPayloadInt(dirname))
19-
})
15+
describe(
16+
'database - sqlite bound parameters limit',
17+
{ db: (type) => type.startsWith('sqlite') },
18+
() => {
19+
beforeAll(async () => {
20+
;({ payload } = await initPayloadInt(dirname))
21+
})
2022

21-
afterAll(async () => {
22-
await payload.destroy()
23-
})
23+
afterAll(async () => {
24+
await payload.destroy()
25+
})
2426

25-
it('should not use bound parameters for where querying on ID with IN if limitedBoundParameters: true', async () => {
26-
const defaultExecute = payload.db.drizzle.$client.execute.bind(payload.db.drizzle.$client)
27+
it('should not use bound parameters for where querying on ID with IN if limitedBoundParameters: true', async () => {
28+
const defaultExecute = payload.db.drizzle.$client.execute.bind(payload.db.drizzle.$client)
2729

28-
// Limit bounds parameters length
29-
payload.db.drizzle.$client.execute = async function execute(...args) {
30-
const res = await defaultExecute(...args)
31-
const [{ args: boundParameters }] = args as [{ args: any[] }]
30+
// Limit bounds parameters length
31+
payload.db.drizzle.$client.execute = async function execute(...args) {
32+
const res = await defaultExecute(...args)
33+
const [{ args: boundParameters }] = args as [{ args: any[] }]
3234

33-
if (boundParameters.length > 100) {
34-
throw new Error('Exceeded limit of bound parameters!')
35+
if (boundParameters.length > 100) {
36+
throw new Error('Exceeded limit of bound parameters!')
37+
}
38+
return res
3539
}
36-
return res
37-
}
38-
39-
payload.db.limitedBoundParameters = false
4040

41-
const IN = Array.from({ length: 300 }, (_, i) => i)
42-
43-
// Should fail here because too the length exceeds the limit
44-
await expect(
45-
payload.find({
41+
payload.db.limitedBoundParameters = false
42+
43+
const IN = Array.from({ length: 300 }, (_, i) => i)
44+
45+
// Should fail here because too the length exceeds the limit
46+
await expect(
47+
payload.find({
48+
collection: 'simple',
49+
pagination: false,
50+
where: { id: { in: IN } },
51+
}),
52+
).rejects.toBeTruthy()
53+
54+
// Should fail here because too the length exceeds the limit
55+
await expect(
56+
payload.find({
57+
collection: 'simple',
58+
pagination: false,
59+
where: { id: { not_in: IN } },
60+
}),
61+
).rejects.toBeTruthy()
62+
63+
payload.db.limitedBoundParameters = true
64+
65+
// Should not fail because limitedBoundParameters: true
66+
await expect(
67+
payload.find({
68+
collection: 'simple',
69+
pagination: false,
70+
where: { id: { in: IN } },
71+
}),
72+
).resolves.toBeTruthy()
73+
74+
// Should not fail because limitedBoundParameters: true
75+
await expect(
76+
payload.find({
77+
collection: 'simple',
78+
pagination: false,
79+
where: { id: { not_in: IN } },
80+
}),
81+
).resolves.toBeTruthy()
82+
83+
// Verify that "in" still works properly
84+
85+
const docs = await Promise.all(
86+
Array.from({ length: 300 }, () => payload.create({ collection: 'simple', data: {} })),
87+
)
88+
89+
const res = await payload.find({
4690
collection: 'simple',
4791
pagination: false,
48-
where: { id: { in: IN } },
49-
}),
50-
).rejects.toBeTruthy()
92+
where: { id: { in: docs.map((e) => e.id) } },
93+
})
5194

52-
// Should fail here because too the length exceeds the limit
53-
await expect(
54-
payload.find({
55-
collection: 'simple',
56-
pagination: false,
57-
where: { id: { not_in: IN } },
58-
}),
59-
).rejects.toBeTruthy()
95+
expect(res.totalDocs).toBe(300)
96+
for (const docInRes of res.docs) {
97+
expect(docs.some((doc) => doc.id === docInRes.id)).toBeTruthy()
98+
}
99+
})
60100

61-
payload.db.limitedBoundParameters = true
101+
it('should avoid ambiguous column name errors when limitedBoundParameters: true and multiple joins are present', async () => {
102+
payload.db.limitedBoundParameters = true
62103

63-
// Should not fail because limitedBoundParameters: true
64-
await expect(
65-
payload.find({
66-
collection: 'simple',
67-
pagination: false,
68-
where: { id: { in: IN } },
69-
}),
70-
).resolves.toBeTruthy()
104+
const simpleLocalizedDoc = await payload.create({
105+
collection: 'simple-localized',
106+
data: {
107+
text: 'Test',
108+
},
109+
locale: 'en',
110+
})
71111

72-
// Should not fail because limitedBoundParameters: true
73-
await expect(
74-
payload.find({
75-
collection: 'simple',
112+
const res = await payload.find({
113+
collection: 'simple-localized',
76114
pagination: false,
77-
where: { id: { not_in: IN } },
78-
}),
79-
).resolves.toBeTruthy()
80-
81-
// Verify that "in" still works properly
82-
83-
const docs = await Promise.all(
84-
Array.from({ length: 300 }, () => payload.create({ collection: 'simple', data: {} })),
85-
)
86-
87-
const res = await payload.find({
88-
collection: 'simple',
89-
pagination: false,
90-
where: { id: { in: docs.map((e) => e.id) } },
115+
where: {
116+
text: {
117+
equals: 'Test',
118+
},
119+
id: {
120+
in: [simpleLocalizedDoc.id],
121+
},
122+
},
123+
locale: 'en',
124+
})
125+
126+
expect(res.totalDocs).toBe(1)
127+
expect(res.docs[0].id).toBe(simpleLocalizedDoc.id)
128+
expect(res.docs[0].text).toBe('Test')
91129
})
92-
93-
expect(res.totalDocs).toBe(300)
94-
for (const docInRes of res.docs) {
95-
expect(docs.some((doc) => doc.id === docInRes.id)).toBeTruthy()
96-
}
97-
})
98-
})
130+
},
131+
)

0 commit comments

Comments
 (0)