diff --git a/packages/drizzle/src/queries/parseParams.ts b/packages/drizzle/src/queries/parseParams.ts index cc717c3e57f..a13df95f68c 100644 --- a/packages/drizzle/src/queries/parseParams.ts +++ b/packages/drizzle/src/queries/parseParams.ts @@ -14,6 +14,7 @@ import { escapeSQLValue } from '../utilities/escapeSQLValue.js' import { getNameFromDrizzleTable } from '../utilities/getNameFromDrizzleTable.js' import { isValidStringID } from '../utilities/isValidStringID.js' import { DistinctSymbol } from '../utilities/rawConstraint.js' +import { UnmatchableValue } from '../utilities/unmatchableValue.js' import { buildAndOrConditions } from './buildAndOrConditions.js' import { buildOperatorConstraint } from './buildOperatorConstraint.js' import { getTableColumnFromPath } from './getTableColumnFromPath.js' @@ -276,6 +277,15 @@ export function parseParams({ value: queryValue, } = sanitizedQueryValue + // An operand that cannot be cast to the column's type can never equal a row, so + // the clause matches nothing - and everything for the one negated operator that + // reaches here. Previously such a value arrived as null and was read as a null + // check, which on a nullable column answered a different question entirely. + if (queryValue === UnmatchableValue) { + constraints.push(queryOperator === 'not_equals' ? sql`true` : sql`false`) + break + } + // Handle polymorphic relationships by value if (queryColumns) { if (!queryColumns.length) { diff --git a/packages/drizzle/src/queries/sanitizeQueryValue.spec.ts b/packages/drizzle/src/queries/sanitizeQueryValue.spec.ts new file mode 100644 index 00000000000..ddbd4cf4078 --- /dev/null +++ b/packages/drizzle/src/queries/sanitizeQueryValue.spec.ts @@ -0,0 +1,83 @@ +import type { Field } from 'payload' + +import { describe, expect, it } from 'vitest' + +import type { DrizzleAdapter } from '../types.js' + +import { UnmatchableValue } from '../utilities/unmatchableValue.js' +import { sanitizeQueryValue } from './sanitizeQueryValue.js' + +const uuid = '3f2504e0-4f89-11d3-9a0c-0305e82c3301' + +const sanitizeUUID = (operator: string, val: unknown) => + sanitizeQueryValue({ + adapter: { idType: 'uuid' } as unknown as DrizzleAdapter, + field: { name: 'id', type: 'text' } as unknown as Field, + isUUID: true, + operator, + relationOrPath: 'id', + val, + }) + +const sanitizeNumber = (operator: string, val: unknown) => + sanitizeQueryValue({ + adapter: { idType: 'serial' } as unknown as DrizzleAdapter, + field: { name: 'age', type: 'number' } as unknown as Field, + isUUID: false, + operator, + relationOrPath: 'age', + val, + }) + +describe('sanitizeQueryValue', () => { + describe('an operand that cannot be cast to a uuid column', () => { + it('reports it as unmatchable rather than coercing it to null', () => { + expect(sanitizeUUID('equals', 'not-a-uuid').value).toBe(UnmatchableValue) + }) + + it('reports it for not_equals too', () => { + expect(sanitizeUUID('not_equals', 'not-a-uuid').value).toBe(UnmatchableValue) + }) + + it('preserves the operator so the caller can negate the clause', () => { + expect(sanitizeUUID('not_equals', 'not-a-uuid').operator).toStrictEqual('not_equals') + }) + + it('still reads "null" as the null check', () => { + expect(sanitizeUUID('equals', 'null').value).toBeNull() + }) + + it('still reads an empty string as the null check', () => { + expect(sanitizeUUID('equals', '').value).toBeNull() + }) + + it('leaves a valid uuid untouched', () => { + expect(sanitizeUUID('equals', uuid).value).toStrictEqual(uuid) + }) + + it('leaves other operators coercing to null, preserving their existing failure modes', () => { + expect(sanitizeUUID('greater_than', 'not-a-uuid').value).toBeNull() + // `contains` then wraps that null for a LIKE - odd, but what it did before this change, + // and what keeps Postgres' own "cannot ILIKE a uuid" error the reported failure. + expect(sanitizeUUID('contains', 'not-a-uuid').value).toStrictEqual('%null%') + }) + }) + + describe('an operand that cannot be cast to a number column', () => { + it('reports it as unmatchable rather than coercing it to null', () => { + expect(sanitizeNumber('equals', 'abc').value).toBe(UnmatchableValue) + }) + + it('still reads "null" as the null check', () => { + expect(sanitizeNumber('equals', 'null').value).toBeNull() + }) + + it('leaves a numeric string untouched', () => { + expect(sanitizeNumber('equals', '5').value).toStrictEqual(5) + }) + + it('leaves other operators coercing to null, preserving their existing failure modes', () => { + expect(sanitizeNumber('greater_than', 'abc').value).toBeNull() + }) + }) +}) diff --git a/packages/drizzle/src/queries/sanitizeQueryValue.ts b/packages/drizzle/src/queries/sanitizeQueryValue.ts index 5959a02722c..0a40f5df487 100644 --- a/packages/drizzle/src/queries/sanitizeQueryValue.ts +++ b/packages/drizzle/src/queries/sanitizeQueryValue.ts @@ -10,6 +10,7 @@ import { getCollectionIdType } from '../utilities/getCollectionIdType.js' import { isPolymorphicRelationship } from '../utilities/isPolymorphicRelationship.js' import { isUUIDType } from '../utilities/isUUIDType.js' import { isRawConstraint } from '../utilities/rawConstraint.js' +import { UnmatchableValue } from '../utilities/unmatchableValue.js' type SanitizeQueryValueArgs = { adapter: DrizzleAdapter @@ -97,16 +98,41 @@ export const sanitizeQueryValue = ({ } } - if (field.type === 'number' && typeof formattedValue === 'string') { - formattedValue = Number(val) + // Only `equals`/`not_equals` are reported as unmatchable, because they are the only operators + // parseParams compiles to IS NULL / IS NOT NULL when the value is null - the conflation this + // fixes. Every other operator keeps coercing to null as before, so the pre-existing failure + // modes stay exactly as they were: `contains` against a native uuid column, for instance, + // still surfaces Postgres' own "cannot ILIKE a uuid" error rather than quietly matching + // nothing. Returning early keeps the symbol out of the branches below, which inspect the + // value. + const reportsUnmatchable = operator === 'equals' || operator === 'not_equals' - if (Number.isNaN(formattedValue)) { + if (field.type === 'number' && typeof formattedValue === 'string') { + if (val === 'null') { formattedValue = null + } else { + formattedValue = Number(val) + + if (Number.isNaN(formattedValue)) { + if (reportsUnmatchable) { + return { operator, value: UnmatchableValue } + } + + formattedValue = null + } } } if (isUUID && typeof formattedValue === 'string') { - if (!uuidValidate(val)) { + // `'null'` and `''` are how a null check arrives as a query string, as the date branch + // above also reads them. + if (val === 'null' || val === '') { + formattedValue = null + } else if (!uuidValidate(val)) { + if (reportsUnmatchable) { + return { operator, value: UnmatchableValue } + } + formattedValue = null } } diff --git a/packages/drizzle/src/utilities/unmatchableValue.ts b/packages/drizzle/src/utilities/unmatchableValue.ts new file mode 100644 index 00000000000..fd7b559aaba --- /dev/null +++ b/packages/drizzle/src/utilities/unmatchableValue.ts @@ -0,0 +1,11 @@ +/** + * Returned by `sanitizeQueryValue` when a scalar operand cannot be cast to its column's type - + * a non-uuid against a `uuid` column, a non-numeric string against a numeric one. + * + * Such an operand used to be coerced to `null`, which `parseParams` then compiles to + * `IS NULL` / `IS NOT NULL` for `equals` / `not_equals`. That silently answers a different + * question: on a nullable column, `equals: 'not-a-uuid'` returned every document whose value + * was empty. This symbol keeps "the caller asked for null" and "this value can never match" + * apart, so only the former still reaches the null checks. + */ +export const UnmatchableValue = Symbol('UnmatchableValue')