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

fix(drizzle): indexes / unique with relationships (#8432)

Browse Source 
Fixes https://github.com/payloadcms/payload/issues/8413 and
https://github.com/payloadcms/payload/issues/6460

- Builds indexes for relationships by default in the SQL schema
- Fixes `unique: true` handling with Postgres / SQLite for every type of
relationships (non-polymorphic. hasMany, polymorphic, polymorphic
hasMany) _note_: disables unique for nested to arrays / blocks
relationships in the `_rels` table.
- adds tests

2.0 PR tha ports only indexes creation
https://github.com/payloadcms/payload/pull/8446, because `unique: true`
could be a breaking change if someone has incosistent unique data in the
database.
This commit is contained in:
Sasha
2024-10-11 20:13:54 +03:00
committed by GitHub
parent 256949e331
commit 1ffb6c3e13
7 changed files with 394 additions and 15 deletions
Download Patch File Download Diff File Expand all files Collapse all files

32
packages/db-sqlite/src/schema/build.ts
View File

@@ -24,6 +24,7 @@ import toSnakeCase from 'to-snake-case'
import type { GenericColumns, GenericTable, IDType, SQLiteAdapter } from '../types.js'
import { createIndex } from './createIndex.js'
import { getIDColumn } from './getIDColumn.js'
import { setColumnID } from './setColumnID.js'
import { traverseFields } from './traverseFields.js'
@@ -56,6 +57,7 @@ type Args = {
buildNumbers?: boolean
buildRelationships?: boolean
disableNotNull: boolean
disableRelsTableUnique?: boolean
disableUnique: boolean
fields: Field[]
joins?: SanitizedJoins
@@ -64,6 +66,7 @@ type Args = {
rootRelationsToBuild?: RelationMap
rootTableIDColType?: IDType
rootTableName?: string
rootUniqueRelationships?: Set<string>
tableName: string
timestamps?: boolean
versions: boolean
@@ -88,6 +91,7 @@ export const buildTable = ({
baseColumns = {},
baseExtraConfig = {},
disableNotNull,
disableRelsTableUnique,
disableUnique = false,
fields,
joins,
@@ -96,6 +100,7 @@ export const buildTable = ({
rootRelationsToBuild,
rootTableIDColType,
rootTableName: incomingRootTableName,
rootUniqueRelationships,
tableName,
timestamps,
versions,
@@ -114,6 +119,7 @@ export const buildTable = ({
// Relationships to the base collection
const relationships: Set<string> = rootRelationships || new Set()
const uniqueRelationships: Set<string> = rootUniqueRelationships || new Set()
let relationshipsTable: GenericTable | SQLiteTableWithColumns<any>
@@ -133,6 +139,7 @@ export const buildTable = ({
adapter,
columns,
disableNotNull,
disableRelsTableUnique,
disableUnique,
fields,
indexes,
@@ -147,6 +154,7 @@ export const buildTable = ({
rootRelationsToBuild: rootRelationsToBuild || relationsToBuild,
rootTableIDColType: rootTableIDColType || idColType,
rootTableName,
uniqueRelationships,
versions,
withinLocalizedArrayOrBlock,
})
@@ -393,7 +401,9 @@ export const buildTable = ({
colType = 'text'
}
relationshipColumns[`${relationTo}ID`] = getIDColumn({
const colName = `${relationTo}ID`
relationshipColumns[colName] = getIDColumn({
name: `${formattedRelationTo}_id`,
type: colType,
primaryKey: false,
@@ -402,9 +412,27 @@ export const buildTable = ({
relationExtraConfig[`${relationTo}IdFk`] = (cols) =>
foreignKey({
name: `${relationshipsTableName}_${toSnakeCase(relationTo)}_fk`,
columns: [cols[`${relationTo}ID`]],
columns: [cols[colName]],
foreignColumns: [adapter.tables[formattedRelationTo].id],
}).onDelete('cascade')
const indexName = [colName]
const unique = !disableUnique && uniqueRelationships.has(relationTo)
if (unique) {
indexName.push('path')
}
if (hasLocalizedRelationshipField) {
indexName.push('locale')
}
relationExtraConfig[`${relationTo}IdIdx`] = createIndex({
name: indexName,
columnName: `${formattedRelationTo}_id`,
tableName: relationshipsTableName,
unique,
})
})
relationshipsTable = sqliteTable(relationshipsTableName, relationshipColumns, (cols) => {

29
packages/db-sqlite/src/schema/traverseFields.ts
View File

@@ -36,6 +36,7 @@ type Args = {
columnPrefix?: string
columns: Record<string, SQLiteColumnBuilder>
disableNotNull: boolean
disableRelsTableUnique?: boolean
disableUnique?: boolean
fieldPrefix?: string
fields: (Field | TabAsField)[]
@@ -52,6 +53,7 @@ type Args = {
rootRelationsToBuild?: RelationMap
rootTableIDColType: IDType
rootTableName: string
uniqueRelationships: Set<string>
versions: boolean
/**
* Tracks whether or not this table is built
@@ -74,6 +76,7 @@ export const traverseFields = ({
columnPrefix,
columns,
disableNotNull,
disableRelsTableUnique,
disableUnique = false,
fieldPrefix,
fields,
@@ -90,6 +93,7 @@ export const traverseFields = ({
rootRelationsToBuild,
rootTableIDColType,
rootTableName,
uniqueRelationships,
versions,
withinLocalizedArrayOrBlock,
}: Args): Result => {
@@ -147,9 +151,10 @@ export const traverseFields = ({
}
if (
(field.unique || field.index) &&
!['array', 'blocks', 'group', 'point', 'relationship', 'upload'].includes(field.type) &&
!('hasMany' in field && field.hasMany === true)
(field.unique || field.index || ['relationship', 'upload'].includes(field.type)) &&
!['array', 'blocks', 'group', 'point'].includes(field.type) &&
!('hasMany' in field && field.hasMany === true) &&
!('relationTo' in field && Array.isArray(field.relationTo))
) {
const unique = disableUnique !== true && field.unique
if (unique) {
@@ -401,12 +406,14 @@ export const traverseFields = ({
baseColumns,
baseExtraConfig,
disableNotNull: disableNotNullFromHere,
disableRelsTableUnique: true,
disableUnique,
fields: disableUnique ? idToUUID(field.fields) : field.fields,
rootRelationships: relationships,
rootRelationsToBuild,
rootTableIDColType,
rootTableName,
rootUniqueRelationships: uniqueRelationships,
tableName: arrayTableName,
versions,
withinLocalizedArrayOrBlock: isLocalized,
@@ -540,12 +547,14 @@ export const traverseFields = ({
baseColumns,
baseExtraConfig,
disableNotNull: disableNotNullFromHere,
disableRelsTableUnique: true,
disableUnique,
fields: disableUnique ? idToUUID(block.fields) : block.fields,
rootRelationships: relationships,
rootRelationsToBuild,
rootTableIDColType,
rootTableName,
rootUniqueRelationships: uniqueRelationships,
tableName: blockTableName,
versions,
withinLocalizedArrayOrBlock: isLocalized,
@@ -664,6 +673,7 @@ export const traverseFields = ({
rootRelationsToBuild,
rootTableIDColType,
rootTableName,
uniqueRelationships,
versions,
withinLocalizedArrayOrBlock,
})
@@ -719,6 +729,7 @@ export const traverseFields = ({
rootRelationsToBuild,
rootTableIDColType,
rootTableName,
uniqueRelationships,
versions,
withinLocalizedArrayOrBlock: withinLocalizedArrayOrBlock || field.localized,
})
@@ -775,6 +786,7 @@ export const traverseFields = ({
rootRelationsToBuild,
rootTableIDColType,
rootTableName,
uniqueRelationships,
versions,
withinLocalizedArrayOrBlock,
})
@@ -831,6 +843,7 @@ export const traverseFields = ({
rootRelationsToBuild,
rootTableIDColType,
rootTableName,
uniqueRelationships,
versions,
withinLocalizedArrayOrBlock,
})
@@ -859,9 +872,17 @@ export const traverseFields = ({
case 'relationship':
case 'upload':
if (Array.isArray(field.relationTo)) {
field.relationTo.forEach((relation) => relationships.add(relation))
field.relationTo.forEach((relation) => {
relationships.add(relation)
if (field.unique && !disableUnique && !disableRelsTableUnique) {
uniqueRelationships.add(relation)
}
})
} else if (field.hasMany) {
relationships.add(field.relationTo)
if (field.unique && !disableUnique && !disableRelsTableUnique) {
uniqueRelationships.add(field.relationTo)
}
} else {
// simple relationships get a column on the targetTable with a foreign key to the relationTo table
const relationshipConfig = adapter.payload.collections[field.relationTo].config

36
packages/drizzle/src/postgres/schema/build.ts
View File

@@ -30,6 +30,7 @@ import type {
} from '../types.js'
import { createTableName } from '../../createTableName.js'
import { createIndex } from './createIndex.js'
import { parentIDColumnMap } from './parentIDColumnMap.js'
import { setColumnID } from './setColumnID.js'
import { traverseFields } from './traverseFields.js'
@@ -45,6 +46,7 @@ type Args = {
buildNumbers?: boolean
buildRelationships?: boolean
disableNotNull: boolean
disableRelsTableUnique?: boolean
disableUnique: boolean
fields: Field[]
joins?: SanitizedJoins
@@ -52,6 +54,7 @@ type Args = {
rootRelationsToBuild?: RelationMap
rootTableIDColType?: string
rootTableName?: string
rootUniqueRelationships?: Set<string>
tableName: string
timestamps?: boolean
versions: boolean
@@ -76,6 +79,7 @@ export const buildTable = ({
baseColumns = {},
baseExtraConfig = {},
disableNotNull,
disableRelsTableUnique = false,
disableUnique = false,
fields,
joins,
@@ -83,6 +87,7 @@ export const buildTable = ({
rootRelationsToBuild,
rootTableIDColType,
rootTableName: incomingRootTableName,
rootUniqueRelationships,
tableName,
timestamps,
versions,
@@ -102,6 +107,9 @@ export const buildTable = ({
// Relationships to the base collection
const relationships: Set<string> = rootRelationships || new Set()
// Unique relationships to the base collection
const uniqueRelationships: Set<string> = rootUniqueRelationships || new Set()
let relationshipsTable: GenericTable | PgTableWithColumns<any>
// Drizzle relations
@@ -120,6 +128,7 @@ export const buildTable = ({
adapter,
columns,
disableNotNull,
disableRelsTableUnique,
disableUnique,
fields,
indexes,
@@ -133,6 +142,7 @@ export const buildTable = ({
rootRelationsToBuild: rootRelationsToBuild || relationsToBuild,
rootTableIDColType: rootTableIDColType || idColType,
rootTableName,
uniqueRelationships,
versions,
withinLocalizedArrayOrBlock,
})
@@ -368,16 +378,34 @@ export const buildTable = ({
colType = 'varchar'
}
relationshipColumns[`${relationTo}ID`] = parentIDColumnMap[colType](
`${formattedRelationTo}_id`,
)
const colName = `${relationTo}ID`
relationshipColumns[colName] = parentIDColumnMap[colType](`${formattedRelationTo}_id`)
relationExtraConfig[`${relationTo}IdFk`] = (cols) =>
foreignKey({
name: `${relationshipsTableName}_${toSnakeCase(relationTo)}_fk`,
columns: [cols[`${relationTo}ID`]],
columns: [cols[colName]],
foreignColumns: [adapter.tables[formattedRelationTo].id],
}).onDelete('cascade')
const indexName = [colName]
const unique = !disableUnique && uniqueRelationships.has(relationTo)
if (unique) {
indexName.push('path')
}
if (hasLocalizedRelationshipField) {
indexName.push('locale')
}
relationExtraConfig[`${relationTo}IdIdx`] = createIndex({
name: indexName,
columnName: `${formattedRelationTo}_id`,
tableName: relationshipsTableName,
unique,
})
})
relationshipsTable = adapter.pgSchema.table(

29
packages/drizzle/src/postgres/schema/traverseFields.ts
View File

@@ -43,6 +43,7 @@ type Args = {
columnPrefix?: string
columns: Record<string, PgColumnBuilder>
disableNotNull: boolean
disableRelsTableUnique?: boolean
disableUnique?: boolean
fieldPrefix?: string
fields: (Field | TabAsField)[]
@@ -58,6 +59,7 @@ type Args = {
rootRelationsToBuild?: RelationMap
rootTableIDColType: string
rootTableName: string
uniqueRelationships: Set<string>
versions: boolean
/**
* Tracks whether or not this table is built
@@ -80,6 +82,7 @@ export const traverseFields = ({
columnPrefix,
columns,
disableNotNull,
disableRelsTableUnique,
disableUnique = false,
fieldPrefix,
fields,
@@ -95,6 +98,7 @@ export const traverseFields = ({
rootRelationsToBuild,
rootTableIDColType,
rootTableName,
uniqueRelationships,
versions,
withinLocalizedArrayOrBlock,
}: Args): Result => {
@@ -152,9 +156,10 @@ export const traverseFields = ({
}
if (
(field.unique || field.index) &&
!['array', 'blocks', 'group', 'point', 'relationship', 'upload'].includes(field.type) &&
!('hasMany' in field && field.hasMany === true)
(field.unique || field.index || ['relationship', 'upload'].includes(field.type)) &&
!['array', 'blocks', 'group', 'point'].includes(field.type) &&
!('hasMany' in field && field.hasMany === true) &&
!('relationTo' in field && Array.isArray(field.relationTo))
) {
const unique = disableUnique !== true && field.unique
if (unique) {
@@ -412,12 +417,14 @@ export const traverseFields = ({
baseColumns,
baseExtraConfig,
disableNotNull: disableNotNullFromHere,
disableRelsTableUnique: true,
disableUnique,
fields: disableUnique ? idToUUID(field.fields) : field.fields,
rootRelationships: relationships,
rootRelationsToBuild,
rootTableIDColType,
rootTableName,
rootUniqueRelationships: uniqueRelationships,
tableName: arrayTableName,
versions,
withinLocalizedArrayOrBlock: isLocalized,
@@ -547,12 +554,14 @@ export const traverseFields = ({
baseColumns,
baseExtraConfig,
disableNotNull: disableNotNullFromHere,
disableRelsTableUnique: true,
disableUnique,
fields: disableUnique ? idToUUID(block.fields) : block.fields,
rootRelationships: relationships,
rootRelationsToBuild,
rootTableIDColType,
rootTableName,
rootUniqueRelationships: uniqueRelationships,
tableName: blockTableName,
versions,
withinLocalizedArrayOrBlock: isLocalized,
@@ -670,6 +679,7 @@ export const traverseFields = ({
rootRelationsToBuild,
rootTableIDColType,
rootTableName,
uniqueRelationships,
versions,
withinLocalizedArrayOrBlock,
})
@@ -724,6 +734,7 @@ export const traverseFields = ({
rootRelationsToBuild,
rootTableIDColType,
rootTableName,
uniqueRelationships,
versions,
withinLocalizedArrayOrBlock: withinLocalizedArrayOrBlock || field.localized,
})
@@ -779,6 +790,7 @@ export const traverseFields = ({
rootRelationsToBuild,
rootTableIDColType,
rootTableName,
uniqueRelationships,
versions,
withinLocalizedArrayOrBlock,
})
@@ -834,6 +846,7 @@ export const traverseFields = ({
rootRelationsToBuild,
rootTableIDColType,
rootTableName,
uniqueRelationships,
versions,
withinLocalizedArrayOrBlock,
})
@@ -862,9 +875,17 @@ export const traverseFields = ({
case 'relationship':
case 'upload':
if (Array.isArray(field.relationTo)) {
field.relationTo.forEach((relation) => relationships.add(relation))
field.relationTo.forEach((relation) => {
relationships.add(relation)
if (field.unique && !disableUnique && !disableRelsTableUnique) {
uniqueRelationships.add(relation)
}
})
} else if (field.hasMany) {
relationships.add(field.relationTo)
if (field.unique && !disableUnique && !disableRelsTableUnique) {
uniqueRelationships.add(field.relationTo)
}
} else {
// simple relationships get a column on the targetTable with a foreign key to the relationTo table
const relationshipConfig = adapter.payload.collections[field.relationTo].config

46
test/fields/collections/Indexed/index.ts
View File

@@ -16,6 +16,52 @@ const IndexedFields: CollectionConfig = {
type: 'text',
unique: true,
},
{
name: 'uniqueRelationship',
type: 'relationship',
relationTo: 'text-fields',
unique: true,
},
{
name: 'uniqueHasManyRelationship',
type: 'relationship',
relationTo: 'text-fields',
unique: true,
hasMany: true,
},
{
name: 'uniqueHasManyRelationship_2',
type: 'relationship',
relationTo: 'text-fields',
hasMany: true,
unique: true,
},
{
name: 'uniquePolymorphicRelationship',
type: 'relationship',
relationTo: ['text-fields'],
unique: true,
},
{
name: 'uniquePolymorphicRelationship_2',
type: 'relationship',
relationTo: ['text-fields'],
unique: true,
},
{
name: 'uniqueHasManyPolymorphicRelationship',
type: 'relationship',
relationTo: ['text-fields'],
unique: true,
hasMany: true,
},
{
name: 'uniqueHasManyPolymorphicRelationship_2',
type: 'relationship',
relationTo: ['text-fields'],
unique: true,
hasMany: true,
},
{
name: 'uniqueRequiredText',
type: 'text',

214
test/fields/int.spec.ts
View File

@@ -1,9 +1,9 @@
import type { MongooseAdapter } from '@payloadcms/db-mongodb'
import type { IndexDirection, IndexOptions } from 'mongoose'
import type { PaginatedDocs, Payload } from 'payload'
import { reload } from '@payloadcms/next/utilities'
import path from 'path'
import { type PaginatedDocs, type Payload, ValidationError } from 'payload'
import { fileURLToPath } from 'url'
import type { NextRESTClient } from '../helpers/NextRESTClient.js'
@@ -1035,6 +1035,218 @@ describe('Fields', () => {
}).toBeDefined()
})
it('should throw validation error saving on unique relationship fields hasMany: false non polymorphic', async () => {
const textDoc = await payload.create({ collection: 'text-fields', data: { text: 'asd' } })
await payload
.create({
collection: 'indexed-fields',
data: {
localizedUniqueRequiredText: '1',
text: '2',
uniqueRequiredText: '3',
uniqueRelationship: textDoc.id,
},
})
// Skip mongodb uniuqe error because it threats localizedUniqueRequriedText.es as undefined
.then((doc) =>
payload.update({
locale: 'es',
collection: 'indexed-fields',
data: { localizedUniqueRequiredText: '20' },
id: doc.id,
}),
)
await expect(
payload.create({
collection: 'indexed-fields',
data: {
localizedUniqueRequiredText: '4',
text: '5',
uniqueRequiredText: '10',
uniqueRelationship: textDoc.id,
},
}),
).rejects.toBeTruthy()
})
it('should throw validation error saving on unique relationship fields hasMany: true', async () => {
const textDoc = await payload.create({ collection: 'text-fields', data: { text: 'asd' } })
await payload
.create({
collection: 'indexed-fields',
data: {
localizedUniqueRequiredText: '1',
text: '2',
uniqueRequiredText: '3',
uniqueHasManyRelationship: [textDoc.id],
},
})
// Skip mongodb uniuqe error because it threats localizedUniqueRequriedText.es as undefined
.then((doc) =>
payload.update({
locale: 'es',
collection: 'indexed-fields',
data: { localizedUniqueRequiredText: '40' },
id: doc.id,
}),
)
// Should allow the same relationship on a diferrent field!
await payload
.create({
collection: 'indexed-fields',
data: {
localizedUniqueRequiredText: '31',
text: '24',
uniqueRequiredText: '55',
uniqueHasManyRelationship_2: [textDoc.id],
},
})
// Skip mongodb uniuqe error because it threats localizedUniqueRequriedText.es as undefined
.then((doc) =>
payload.update({
locale: 'es',
collection: 'indexed-fields',
data: { localizedUniqueRequiredText: '30' },
id: doc.id,
}),
)
await expect(
payload.create({
collection: 'indexed-fields',
data: {
localizedUniqueRequiredText: '4',
text: '5',
uniqueRequiredText: '10',
uniqueHasManyRelationship: [textDoc.id],
},
}),
).rejects.toBeTruthy()
})
it('should throw validation error saving on unique relationship fields polymorphic', async () => {
const textDoc = await payload.create({ collection: 'text-fields', data: { text: 'asd' } })
await payload
.create({
collection: 'indexed-fields',
data: {
localizedUniqueRequiredText: '1',
text: '2',
uniqueRequiredText: '3',
uniquePolymorphicRelationship: { relationTo: 'text-fields', value: textDoc.id },
},
})
// Skip mongodb uniuqe error because it threats localizedUniqueRequriedText.es as undefined
.then((doc) =>
payload.update({
locale: 'es',
collection: 'indexed-fields',
data: { localizedUniqueRequiredText: '20' },
id: doc.id,
}),
)
// Should allow the same relationship on a diferrent field!
await payload
.create({
collection: 'indexed-fields',
data: {
localizedUniqueRequiredText: '31',
text: '24',
uniqueRequiredText: '55',
uniquePolymorphicRelationship_2: { relationTo: 'text-fields', value: textDoc.id },
},
})
// Skip mongodb uniuqe error because it threats localizedUniqueRequriedText.es as undefined
.then((doc) =>
payload.update({
locale: 'es',
collection: 'indexed-fields',
data: { localizedUniqueRequiredText: '100' },
id: doc.id,
}),
)
await expect(
payload.create({
collection: 'indexed-fields',
data: {
localizedUniqueRequiredText: '4',
text: '5',
uniqueRequiredText: '10',
uniquePolymorphicRelationship: { relationTo: 'text-fields', value: textDoc.id },
},
}),
).rejects.toBeTruthy()
})
it('should throw validation error saving on unique relationship fields polymorphic hasMany: true', async () => {
const textDoc = await payload.create({ collection: 'text-fields', data: { text: 'asd' } })
await payload
.create({
collection: 'indexed-fields',
data: {
localizedUniqueRequiredText: '1',
text: '2',
uniqueRequiredText: '3',
uniqueHasManyPolymorphicRelationship: [
{ relationTo: 'text-fields', value: textDoc.id },
],
},
})
.then((doc) =>
payload.update({
locale: 'es',
collection: 'indexed-fields',
data: { localizedUniqueRequiredText: '100' },
id: doc.id,
}),
)
// Should allow the same relationship on a diferrent field!
await payload
.create({
collection: 'indexed-fields',
data: {
localizedUniqueRequiredText: '31',
text: '24',
uniqueRequiredText: '55',
uniqueHasManyPolymorphicRelationship_2: [
{ relationTo: 'text-fields', value: textDoc.id },
],
},
})
// Skip mongodb uniuqe error because it threats localizedUniqueRequriedText.es as undefined
.then((doc) =>
payload.update({
locale: 'es',
collection: 'indexed-fields',
data: { localizedUniqueRequiredText: '300' },
id: doc.id,
}),
)
await expect(
payload.create({
collection: 'indexed-fields',
data: {
localizedUniqueRequiredText: '4',
text: '5',
uniqueRequiredText: '10',
uniqueHasManyPolymorphicRelationship: [
{ relationTo: 'text-fields', value: textDoc.id },
],
},
}),
).rejects.toBeTruthy()
})
it('should not throw validation error saving multiple null values for unique fields', async () => {
const data = {
localizedUniqueRequiredText: 'en1',

23
test/fields/payload-types.ts
View File

@@ -1020,6 +1020,29 @@ export interface IndexedField {
id: string;
text: string;
uniqueText?: string | null;
uniqueRelationship?: (string | null) | TextField;
uniqueHasManyRelationship?: (string | TextField)[] | null;
uniqueHasManyRelationship_2?: (string | TextField)[] | null;
uniquePolymorphicRelationship?: {
relationTo: 'text-fields';
value: string | TextField;
} | null;
uniquePolymorphicRelationship_2?: {
relationTo: 'text-fields';
value: string | TextField;
} | null;
uniqueHasManyPolymorphicRelationship?:
| {
relationTo: 'text-fields';
value: string | TextField;
}[]
| null;
uniqueHasManyPolymorphicRelationship_2?:
| {
relationTo: 'text-fields';
value: string | TextField;
}[]
| null;
uniqueRequiredText: string;
localizedUniqueRequiredText: string;
/**