Skip to content

fix(drizzle): do not read an uncastable equals operand as a null check - #17841

Open
creativoma wants to merge 1 commit into
payloadcms:mainfrom
creativoma:fix/drizzle-invalid-uuid-equals-is-null
Open

fix(drizzle): do not read an uncastable equals operand as a null check#17841
creativoma wants to merge 1 commit into
payloadcms:mainfrom
creativoma:fix/drizzle-invalid-uuid-equals-is-null

Conversation

@creativoma

Copy link
Copy Markdown

What?

A scalar operand that cannot be cast to its column's type — a non-uuid against a uuid column, a non-numeric string against a numeric one — was coerced to null in sanitizeQueryValue. parseParams then compiles a null value for equals / not_equals into IS NULL / IS NOT NULL, so the query silently answered a different question than the one asked.

Those two operators now report the operand as unmatchable instead, and the clause resolves to "matches nothing" (not_equals: "matches everything"). Every other operator keeps coercing to null exactly as before.

Fixes #16483

Why?

On a nullable column the result is wrong data rather than an error. From the issue, on a uuid adapter with a relationship field:

  • equals: "<valid uuid, no matching documents>" → no documents. Correct.
  • equals: "invalid-something"every document whose relationship is empty.

Nobody asked for the empty ones. The caller asked for equals: "invalid-something", not equals: null.

Two things worth adding to the issue's own analysis:

  • number fields have the identical defect. equals: 'abc' on a number field is Number('abc')NaNnullIS NULL. This fixes both paths.
  • not_equals is affected too, and more loudly: it became IS NOT NULL, returning every document that has a value.

On an id column the old behaviour happens to be harmless, since an id is never null — IS NULL matches nothing, which is the right answer for a value that cannot exist. It is nullable columns where the conflation produces wrong rows.

Why not throw?

The issue thread proposed rejecting an invalid uuid with a validation error. I tried that first, and this repository's own tests rule it out:

  1. like - id should not crash (test/collections-rest/int.spec.ts) sends where[id][like]=words partial and asserts a 200. A free-text search box pointed at an id column is not a malformed query, and must not fail.
  2. Narrowing the throw to equals / not_equals does not help, because parseParams rewrites like to equals for number, relationship, upload and PgUUID columns before sanitizeQueryValue ever sees it. At the point of the fix the two are indistinguishable.

So the value has to resolve to a constraint, not an exception. "Matches nothing" also happens to be what the rewritten like wants: the search found nothing, which is true.

How?

UnmatchableValue, a new symbol in packages/drizzle/src/utilities/, marks an operand that cannot be cast. sanitizeQueryValue returns it early — before the branches that inspect the value, so nothing downstream reinterprets the symbol — and parseParams translates it next to the existing null checks:

if (queryValue === UnmatchableValue) {
  constraints.push(queryOperator === 'not_equals' ? sql`true` : sql`false`)
  break
}

A new signal is needed because neither existing mechanism fits: rawConstraint injects a raw value and bypasses sanitising altogether, and returning null from sanitizeQueryValue makes parseParams break and drop the constraint — which matches everything, the opposite of what is wanted.

Scoping it to equals / not_equals is deliberate, and keeps pre-existing failure modes intact. contains against a native uuid column, for one, still surfaces Postgres' own "cannot ILIKE a uuid" error rather than quietly matching nothing — does not wrap a native PgUUID id column in unaccent() pins that, and it stays green.

Two behaviour changes to call out explicitly, both beyond the letter of the issue:

  • not_equals with an uncastable operand goes from IS NOT NULL to matching every row, null ones included. "Not equal to a value that cannot exist" is every row; the old answer was wrong in a different direction.
  • The return contract of sanitizeQueryValue grows a third possible value, and that function is shared by all four drizzle adapters.

Tests

packages/drizzle/src/queries/sanitizeQueryValue.spec.ts (new) covers both column types: the unmatchable report, the operator preserved so the caller can negate, 'null' and '' still reading as null checks, valid operands untouched, and other operators still coercing to null.

Verified locally: build:core, test:unit, test:types, and the database, collections-rest, fields-relationship, joins, query-presets, group-by and versions int suites on postgres, postgres-uuid and sqlite — 1087 integration tests, no failures.


This targets main (v4); the 3.x branch carries the same code if a backport is wanted.

It also overlaps with #17839, which adds the same new spec file — that one fixes the array-operand half of this defect (in/not_in with an empty operand reaching the driver and 500ing). The two are independent and either can land first; I am happy to rebase whichever follows.

A scalar operand that cannot be cast to its column's type was coerced to
null, and parseParams compiles a null value for equals/not_equals into
IS NULL / IS NOT NULL. On a nullable column that silently answers a
different question: `equals: 'not-a-uuid'` returned every document whose
value was empty, and `not_equals` returned every document that had one.

equals/not_equals now report such an operand as unmatchable, so the clause
matches nothing rather than becoming a null check. Every other operator
keeps coercing to null, which preserves the existing failure modes -
notably `contains` against a native uuid column, and the free-text id
search that parseParams rewrites from `like` to `equals`.

Number fields had the identical defect via Number(val) -> NaN, and are
fixed on the same path.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Invalid UUID in treated as IS NULL and returns incorrect data

1 participant