tabshift/payloadcms
1
0
Fork 0
Code Issues Pull Requests Actions 3 Packages Projects Releases Wiki Activity

fix(db-postgres): sort by localized fields (#8839)

Browse Source 
### What?
Fixes https://github.com/payloadcms/payload/issues/5152 issue related to
sorting by a localized field with SQLite / Postgres database adapters.

### Why?
It was an incorrect behaviour.


### How?
Modifies the `getTableColumnFromPath` file to have correct join
conditions. Previously if you had this structure in the _locales table
_locale title parent
en          A    1
es          B     1
we sorted by everything that's here, but we need to sort only by the
passed locale.

Additionally fixes a typescript error in `dev.ts` that I added here
https://github.com/payloadcms/payload/pull/8834

Also, removes the condition with `joins.length` in `countDistinct`. It
was there as for this issue
https://github.com/payloadcms/payload/issues/4889 because sorting by a
localized property caused duplication. This can simnifically improve
performance for `.find` with nested querying/sorting on large data sets,
because `count(*)` is faster than `count(DISTINCT id)`
This commit is contained in:
Sasha
2024-10-24 21:47:58 +03:00
committed by GitHub
parent 2e11068f49
commit 9069bd3fac
5 changed files with 192 additions and 93 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
packages/db-sqlite/src/countDistinct.ts
View File

@@ -1,7 +1,7 @@
import type { ChainedMethods } from '@payloadcms/drizzle/types'
import { chainMethods } from '@payloadcms/drizzle'
import { count, sql } from 'drizzle-orm'
import { count } from 'drizzle-orm'
import type { CountDistinct, SQLiteAdapter } from './types.js'
@@ -22,11 +22,7 @@ export const countDistinct: CountDistinct = async function countDistinct(
methods: chainedMethods,
query: db
.select({
count:
joins.length > 0
? sql`count
(DISTINCT ${this.tables[tableName].id})`.mapWith(Number)
: count(),
count: count(),
})
.from(this.tables[tableName])
.where(where),

8
packages/drizzle/src/postgres/countDistinct.ts
View File

@@ -1,4 +1,4 @@
import { count, sql } from 'drizzle-orm'
import { count } from 'drizzle-orm'
import type { ChainedMethods, TransactionPg } from '../types.js'
import type { BasePostgresAdapter, CountDistinct } from './types.js'
@@ -22,11 +22,7 @@ export const countDistinct: CountDistinct = async function countDistinct(
methods: chainedMethods,
query: (db as TransactionPg)
.select({
count:
joins.length > 0
? sql`count
(DISTINCT ${this.tables[tableName].id})`.mapWith(Number)
: count(),
count: count(),
})
.from(this.tables[tableName])
.where(where),

151
packages/drizzle/src/queries/getTableColumnFromPath.ts
View File

@@ -186,18 +186,17 @@ export const getTableColumnFromPath = ({
if (locale && field.localized && adapter.payload.config.localization) {
newTableName = `${tableName}${adapter.localesSuffix}`
let condition = eq(adapter.tables[tableName].id, adapter.tables[newTableName]._parentID)
if (locale !== 'all') {
condition = and(condition, eq(adapter.tables[newTableName]._locale, locale))
}
addJoinTable({
condition: eq(adapter.tables[tableName].id, adapter.tables[newTableName]._parentID),
condition,
joins,
table: adapter.tables[newTableName],
})
if (locale !== 'all') {
constraints.push({
columnName: '_locale',
table: adapter.tables[newTableName],
value: locale,
})
}
}
return getTableColumnFromPath({
adapter,
@@ -225,21 +224,20 @@ export const getTableColumnFromPath = ({
)
if (locale && field.localized && adapter.payload.config.localization) {
const conditions = [
eq(adapter.tables[tableName].id, adapter.tables[newTableName].parent),
eq(adapter.tables[newTableName]._locale, locale),
]
if (locale !== 'all') {
conditions.push(eq(adapter.tables[newTableName]._locale, locale))
}
addJoinTable({
condition: and(
eq(adapter.tables[tableName].id, adapter.tables[newTableName].parent),
eq(adapter.tables[newTableName]._locale, locale),
),
condition: and(...conditions),
joins,
table: adapter.tables[newTableName],
})
if (locale !== 'all') {
constraints.push({
columnName: '_locale',
table: adapter.tables[newTableName],
value: locale,
})
}
} else {
addJoinTable({
condition: eq(adapter.tables[tableName].id, adapter.tables[newTableName].parent),
@@ -274,18 +272,16 @@ export const getTableColumnFromPath = ({
]
if (locale && field.localized && adapter.payload.config.localization) {
const conditions = [...joinConstraints]
if (locale !== 'all') {
conditions.push(eq(adapter.tables[newTableName]._locale, locale))
}
addJoinTable({
condition: and(...joinConstraints, eq(adapter.tables[newTableName]._locale, locale)),
condition: and(...conditions),
joins,
table: adapter.tables[newTableName],
})
if (locale !== 'all') {
constraints.push({
columnName: 'locale',
table: adapter.tables[newTableName],
value: locale,
})
}
} else {
addJoinTable({
condition: and(...joinConstraints),
@@ -313,21 +309,16 @@ export const getTableColumnFromPath = ({
constraintPath = `${constraintPath}${field.name}.%.`
if (locale && field.localized && adapter.payload.config.localization) {
const conditions = [eq(arrayParentTable.id, adapter.tables[newTableName]._parentID)]
if (locale !== 'all') {
conditions.push(eq(adapter.tables[newTableName]._locale, locale))
}
addJoinTable({
condition: and(
eq(arrayParentTable.id, adapter.tables[newTableName]._parentID),
eq(adapter.tables[newTableName]._locale, locale),
),
condition: and(...conditions),
joins,
table: adapter.tables[newTableName],
})
if (locale !== 'all') {
constraints.push({
columnName: '_locale',
table: adapter.tables[newTableName],
value: locale,
})
}
} else {
addJoinTable({
condition: eq(arrayParentTable.id, adapter.tables[newTableName]._parentID),
@@ -417,23 +408,21 @@ export const getTableColumnFromPath = ({
constraints = constraints.concat(blockConstraints)
selectFields = { ...selectFields, ...blockSelectFields }
if (field.localized && adapter.payload.config.localization) {
joins.push({
condition: and(
eq(
(aliasTable || adapter.tables[tableName]).id,
adapter.tables[newTableName]._parentID,
),
eq(adapter.tables[newTableName]._locale, locale),
const conditions = [
eq(
(aliasTable || adapter.tables[tableName]).id,
adapter.tables[newTableName]._parentID,
),
]
if (locale !== 'all') {
conditions.push(eq(adapter.tables[newTableName]._locale, locale))
}
joins.push({
condition: and(...conditions),
table: adapter.tables[newTableName],
})
if (locale) {
constraints.push({
columnName: '_locale',
table: adapter.tables[newTableName],
value: locale,
})
}
} else {
joins.push({
condition: eq(
@@ -471,21 +460,18 @@ export const getTableColumnFromPath = ({
// Join in the relationships table
if (locale && field.localized && adapter.payload.config.localization) {
const conditions = [
eq((aliasTable || adapter.tables[rootTableName]).id, aliasRelationshipTable.parent),
like(aliasRelationshipTable.path, `${constraintPath}${field.name}`),
]
if (locale !== 'all') {
conditions.push(eq(aliasRelationshipTable.locale, locale))
}
joins.push({
condition: and(
eq((aliasTable || adapter.tables[rootTableName]).id, aliasRelationshipTable.parent),
eq(aliasRelationshipTable.locale, locale),
like(aliasRelationshipTable.path, `${constraintPath}${field.name}`),
),
condition: and(...conditions),
table: aliasRelationshipTable,
})
if (locale !== 'all') {
constraints.push({
columnName: 'locale',
table: aliasRelationshipTable,
value: locale,
})
}
} else {
// Join in the relationships table
joins.push({
@@ -660,15 +646,22 @@ export const getTableColumnFromPath = ({
tableName: `${rootTableName}${adapter.localesSuffix}`,
})
joins.push({
condition: and(
eq(aliasLocaleTable._parentID, adapter.tables[rootTableName].id),
eq(aliasLocaleTable._locale, locale),
),
table: aliasLocaleTable,
const condtions = [eq(aliasLocaleTable._parentID, adapter.tables[rootTableName].id)]
if (locale !== 'all') {
condtions.push(eq(aliasLocaleTable._locale, locale))
}
const localesTable = adapter.tables[`${rootTableName}${adapter.localesSuffix}`]
addJoinTable({
condition: and(...condtions),
joins,
table: localesTable,
})
joins.push({
condition: eq(aliasLocaleTable[columnName], newAliasTable.id),
condition: eq(localesTable[columnName], newAliasTable.id),
table: newAliasTable,
})
} else {
@@ -716,21 +709,19 @@ export const getTableColumnFromPath = ({
newTable = adapter.tables[newTableName]
let condition = eq(parentTable.id, newTable._parentID)
if (locale !== 'all') {
condition = and(condition, eq(newTable._locale, locale))
}
addJoinTable({
condition: eq(parentTable.id, newTable._parentID),
condition,
joins,
table: newTable,
})
aliasTable = undefined
if (locale !== 'all') {
constraints.push({
columnName: '_locale',
table: newTable,
value: locale,
})
}
}
const targetTable = aliasTable || newTable

1
test/dev.ts
View File

@@ -53,6 +53,7 @@ if (args.o) {
const port = process.env.PORT ? Number(process.env.PORT) : 3000
// @ts-expect-error the same as in test/helpers/initPayloadE2E.ts
const app = nextImport({
dev: true,
hostname: 'localhost',

117
test/localization/int.spec.ts
View File

@@ -3,7 +3,7 @@ import { type Payload, type Where } from 'payload'
import { fileURLToPath } from 'url'
import type { NextRESTClient } from '../helpers/NextRESTClient.js'
import type { LocalizedPost, WithLocalizedRelationship } from './payload-types.js'
import type { LocalizedPost, LocalizedSort, WithLocalizedRelationship } from './payload-types.js'
import { idToString } from '../helpers/idToString.js'
import { initPayloadInt } from '../helpers/initPayloadInt.js'
@@ -369,6 +369,7 @@ describe('Localization', () => {
describe('Localized Sort Count', () => {
const expectedTotalDocs = 5
const posts: LocalizedSort[] = []
beforeAll(async () => {
for (let i = 1; i <= expectedTotalDocs; i++) {
const post = await payload.create({
@@ -380,6 +381,8 @@ describe('Localization', () => {
locale: englishLocale,
})
posts.push(post)
await payload.update({
id: post.id,
collection: localizedSortSlug,
@@ -419,6 +422,118 @@ describe('Localization', () => {
expect(sortByTitleQuery.totalDocs).toEqual(expectedTotalDocs)
expect(sortByDateQuery.totalDocs).toEqual(expectedTotalDocs)
})
it('should return correct order when sorted by localized fields', async () => {
const { docs: docsAsc } = await payload.find({ collection: localizedSortSlug, sort: 'title' })
docsAsc.forEach((doc, i) => {
expect(posts[i].id).toBe(doc.id)
})
const { docs: docsDesc } = await payload.find({
collection: localizedSortSlug,
sort: '-title',
})
docsDesc.forEach((doc, i) => {
expect(posts.at(posts.length - i - 1).id).toBe(doc.id)
})
// Test with words
const randomWords = [
'sunset',
'whisper',
'lighthouse',
'harmony',
'crystal',
'thunder',
'meadow',
'voyage',
'echo',
'quicksand',
]
const randomWordsSpanish = [
'atardecer',
'susurro',
'faro',
'armonía',
'cristal',
'trueno',
'pradera',
'viaje',
'eco',
'arenas movedizas',
]
expect(randomWords).toHaveLength(randomWordsSpanish.length)
const randomWordsPosts: (number | string)[] = []
for (let i = 0; i < randomWords.length; i++) {
const en = randomWords[i]
const post = await payload.create({ collection: 'localized-sort', data: { title: en } })
const es = randomWordsSpanish[i]
await payload.update({
collection: 'localized-sort',
data: { title: es },
id: post.id,
locale: 'es',
})
randomWordsPosts.push(post.id)
}
const ascSortedWordsEn = randomWords.toSorted((a, b) => a.localeCompare(b))
const descSortedWordsEn = randomWords.toSorted((a, b) => b.localeCompare(a))
const q = { id: { in: randomWordsPosts } }
const { docs: randomWordsEnAsc } = await payload.find({
collection: localizedSortSlug,
sort: 'title',
where: q,
})
randomWordsEnAsc.forEach((doc, i) => {
expect(ascSortedWordsEn[i]).toBe(doc.title)
})
const { docs: randomWordsEnDesc } = await payload.find({
collection: localizedSortSlug,
sort: '-title',
where: q,
})
randomWordsEnDesc.forEach((doc, i) => {
expect(descSortedWordsEn[i]).toBe(doc.title)
})
// Test sorting for Spanish locale
const ascSortedWordsEs = randomWordsSpanish.toSorted((a, b) => a.localeCompare(b))
const descSortedWordsEs = randomWordsSpanish.toSorted((a, b) => b.localeCompare(a))
// Fetch sorted words in Spanish (ascending)
const { docs: randomWordsEsAsc } = await payload.find({
collection: localizedSortSlug,
sort: 'title',
where: q,
locale: 'es',
})
randomWordsEsAsc.forEach((doc, i) => {
expect(ascSortedWordsEs[i]).toBe(doc.title)
})
// Fetch sorted words in Spanish (descending)
const { docs: randomWordsEsDesc } = await payload.find({
collection: localizedSortSlug,
sort: '-title',
where: q,
locale: 'es',
})
randomWordsEsDesc.forEach((doc, i) => {
expect(descSortedWordsEs[i]).toBe(doc.title)
})
})
})
describe('Localized Relationship', () => {