Skip to content

Commit 72ab319

Browse files
authored
fix(db-*): ensure consistent sorting even when sorting on non-unique fields or no sort parameters at all (#12447)
The databases do not keep track of document order internally so when sorting by non-unique fields such as shared `order` number values, the returned order will be random and not consistent. While this issue is far more noticeable on mongo it could also occur in postgres on certain environments. This combined with pagination can lead to the perception of duplicated or inconsistent data. This PR adds a second sort parameter to queries so that we always have a fallback, `-createdAt` will be used by default or `id` if timestamps are disabled.
1 parent 2a929cf commit 72ab319

File tree

6 files changed

+268
-25
lines changed

6 files changed

+268
-25
lines changed

packages/db-mongodb/src/queries/buildSortParam.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,18 @@ export const buildSortParam = ({
150150
sort = [sort]
151151
}
152152

153+
// In the case of Mongo, when sorting by a field that is not unique, the results are not guaranteed to be in the same order each time.
154+
// So we add a fallback sort to ensure that the results are always in the same order.
155+
let fallbackSort = '-id'
156+
157+
if (timestamps) {
158+
fallbackSort = '-createdAt'
159+
}
160+
161+
if (!(sort.includes(fallbackSort) || sort.includes(fallbackSort.replace('-', '')))) {
162+
sort.push(fallbackSort)
163+
}
164+
153165
const sorting = sort.reduce<Record<string, string>>((acc, item) => {
154166
let sortProperty: string
155167
let sortDirection: SortDirection

packages/drizzle/src/queries/buildOrderBy.ts

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,9 @@ export const buildOrderBy = ({
3939
}: Args): BuildQueryResult['orderBy'] => {
4040
const orderBy: BuildQueryResult['orderBy'] = []
4141

42+
const createdAt = adapter.tables[tableName]?.createdAt
43+
4244
if (!sort) {
43-
const createdAt = adapter.tables[tableName]?.createdAt
4445
if (createdAt) {
4546
sort = '-createdAt'
4647
} else {
@@ -52,6 +53,18 @@ export const buildOrderBy = ({
5253
sort = [sort]
5354
}
5455

56+
// In the case of Mongo, when sorting by a field that is not unique, the results are not guaranteed to be in the same order each time.
57+
// So we add a fallback sort to ensure that the results are always in the same order.
58+
let fallbackSort = '-id'
59+
60+
if (createdAt) {
61+
fallbackSort = '-createdAt'
62+
}
63+
64+
if (!(sort.includes(fallbackSort) || sort.includes(fallbackSort.replace('-', '')))) {
65+
sort.push(fallbackSort)
66+
}
67+
5568
for (const sortItem of sort) {
5669
let sortProperty: string
5770
let sortDirection: 'asc' | 'desc'
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
import type { CollectionConfig } from 'payload'
2+
3+
export const nonUniqueSortSlug = 'non-unique-sort'
4+
5+
export const NonUniqueSortCollection: CollectionConfig = {
6+
slug: nonUniqueSortSlug,
7+
fields: [
8+
{
9+
name: 'title',
10+
type: 'text',
11+
},
12+
{
13+
name: 'order',
14+
type: 'number',
15+
},
16+
],
17+
}

test/sort/config.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,14 @@ import type { CollectionSlug, Payload } from 'payload'
22

33
import { fileURLToPath } from 'node:url'
44
import path from 'path'
5+
import { wait } from 'payload/shared'
56

67
import { buildConfigWithDefaults } from '../buildConfigWithDefaults.js'
78
import { devUser } from '../credentials.js'
89
import { DefaultSortCollection } from './collections/DefaultSort/index.js'
910
import { DraftsCollection } from './collections/Drafts/index.js'
1011
import { LocalizedCollection } from './collections/Localized/index.js'
12+
import { NonUniqueSortCollection, nonUniqueSortSlug } from './collections/NonUniqueSort/index.js'
1113
import { OrderableCollection } from './collections/Orderable/index.js'
1214
import { OrderableJoinCollection } from './collections/OrderableJoin/index.js'
1315
import { PostsCollection } from './collections/Posts/index.js'
@@ -19,6 +21,7 @@ export default buildConfigWithDefaults({
1921
PostsCollection,
2022
DraftsCollection,
2123
DefaultSortCollection,
24+
NonUniqueSortCollection,
2225
LocalizedCollection,
2326
OrderableCollection,
2427
OrderableJoinCollection,
@@ -87,6 +90,28 @@ async function seedSortable(payload: Payload) {
8790

8891
await payload.create({ collection: 'orderable-join', data: { title: 'Join B' } })
8992

93+
// Create 10 items to be sorted by non-unique field
94+
for (const i of Array.from({ length: 10 }, (_, index) => index)) {
95+
let order = 1
96+
97+
if (i > 3) {
98+
order = 2
99+
} else if (i > 6) {
100+
order = 3
101+
}
102+
103+
await payload.create({
104+
collection: nonUniqueSortSlug,
105+
data: {
106+
title: `Post ${i}`,
107+
order,
108+
},
109+
})
110+
111+
// Wait 2 seconds to guarantee that the createdAt date is different
112+
// await wait(2000)
113+
}
114+
90115
return new Response(JSON.stringify({ success: true }), {
91116
headers: { 'Content-Type': 'application/json' },
92117
status: 200,

test/sort/int.spec.ts

Lines changed: 149 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import type { Draft, Orderable, OrderableJoin } from './payload-types.js'
88

99
import { initPayloadInt } from '../helpers/initPayloadInt.js'
1010
import { draftsSlug } from './collections/Drafts/index.js'
11+
import { nonUniqueSortSlug } from './collections/NonUniqueSort/index.js'
1112
import { orderableSlug } from './collections/Orderable/index.js'
1213
import { orderableJoinSlug } from './collections/OrderableJoin/index.js'
1314

@@ -133,6 +134,154 @@ describe('Sort', () => {
133134
})
134135
})
135136

137+
describe('Non-unique sorting', () => {
138+
// There are situations where the sort order is not guaranteed to be consistent, such as when sorting by a non-unique field in MongoDB which does not keep an internal order of items
139+
// As a result, every time you fetch, including fetching specific pages, the order of items may change and appear as duplicated to some users.
140+
it('should always be consistent when sorting', async () => {
141+
const posts = await payload.find({
142+
collection: nonUniqueSortSlug,
143+
sort: 'order',
144+
})
145+
146+
const initialMap = posts.docs.map((post) => post.title)
147+
148+
const dataFetches = await Promise.all(
149+
Array.from({ length: 3 }).map(() =>
150+
payload.find({
151+
collection: nonUniqueSortSlug,
152+
sort: 'order',
153+
}),
154+
),
155+
)
156+
157+
const [fetch1, fetch2, fetch3] = dataFetches.map((fetch) =>
158+
fetch.docs.map((post) => post.title),
159+
)
160+
161+
expect(fetch1).toEqual(initialMap)
162+
163+
expect(fetch2).toEqual(initialMap)
164+
165+
expect(fetch3).toEqual(initialMap)
166+
})
167+
168+
it('should always be consistent when sorting - with limited pages', async () => {
169+
const posts = await payload.find({
170+
collection: nonUniqueSortSlug,
171+
sort: 'order',
172+
limit: 5,
173+
page: 2,
174+
})
175+
176+
const initialMap = posts.docs.map((post) => post.title)
177+
178+
const dataFetches = await Promise.all(
179+
Array.from({ length: 3 }).map(() =>
180+
payload.find({
181+
collection: nonUniqueSortSlug,
182+
sort: 'order',
183+
limit: 5,
184+
page: 2,
185+
}),
186+
),
187+
)
188+
189+
const [fetch1, fetch2, fetch3] = dataFetches.map((fetch) =>
190+
fetch.docs.map((post) => post.title),
191+
)
192+
193+
expect(fetch1).toEqual(initialMap)
194+
195+
expect(fetch2).toEqual(initialMap)
196+
197+
expect(fetch3).toEqual(initialMap)
198+
})
199+
200+
it('should sort by createdAt as fallback', async () => {
201+
// This is the (reverse - newest first) order that the posts are created in so this should remain consistent as the sort should fallback to '-createdAt'
202+
const postsInOrder = ['Post 9', 'Post 8', 'Post 7', 'Post 6']
203+
204+
const dataFetches = await Promise.all(
205+
Array.from({ length: 3 }).map(() =>
206+
payload.find({
207+
collection: nonUniqueSortSlug,
208+
sort: 'order',
209+
page: 2,
210+
limit: 4,
211+
}),
212+
),
213+
)
214+
215+
const [fetch1, fetch2, fetch3] = dataFetches.map((fetch) =>
216+
fetch.docs.map((post) => post.title),
217+
)
218+
219+
console.log({ fetch1, fetch2, fetch3 })
220+
221+
expect(fetch1).toEqual(postsInOrder)
222+
223+
expect(fetch2).toEqual(postsInOrder)
224+
225+
expect(fetch3).toEqual(postsInOrder)
226+
})
227+
228+
it('should always be consistent without sort params in the query', async () => {
229+
const posts = await payload.find({
230+
collection: nonUniqueSortSlug,
231+
})
232+
233+
const initialMap = posts.docs.map((post) => post.title)
234+
235+
const dataFetches = await Promise.all(
236+
Array.from({ length: 3 }).map(() =>
237+
payload.find({
238+
collection: nonUniqueSortSlug,
239+
}),
240+
),
241+
)
242+
243+
const [fetch1, fetch2, fetch3] = dataFetches.map((fetch) =>
244+
fetch.docs.map((post) => post.title),
245+
)
246+
247+
expect(fetch1).toEqual(initialMap)
248+
249+
expect(fetch2).toEqual(initialMap)
250+
251+
expect(fetch3).toEqual(initialMap)
252+
})
253+
254+
it('should always be consistent without sort params in the query - with limited pages', async () => {
255+
const posts = await payload.find({
256+
collection: nonUniqueSortSlug,
257+
page: 2,
258+
limit: 4,
259+
})
260+
261+
const initialMap = posts.docs.map((post) => post.title)
262+
263+
const dataFetches = await Promise.all(
264+
Array.from({ length: 3 }).map(() =>
265+
payload.find({
266+
collection: nonUniqueSortSlug,
267+
page: 2,
268+
limit: 4,
269+
}),
270+
),
271+
)
272+
273+
const [fetch1, fetch2, fetch3] = dataFetches.map((fetch) =>
274+
fetch.docs.map((post) => post.title),
275+
)
276+
277+
expect(fetch1).toEqual(initialMap)
278+
279+
expect(fetch2).toEqual(initialMap)
280+
281+
expect(fetch3).toEqual(initialMap)
282+
})
283+
})
284+
136285
describe('Sort by multiple fields', () => {
137286
it('should sort posts by multiple fields', async () => {
138287
const posts = await payload.find({

0 commit comments

Comments
 (0)