fix(pg-core): align .desc() index NULLS ordering with desc() order-by (#5978) - #6142
Open
aliabbas-muhammadi wants to merge 1 commit into
Open
Conversation
A .desc() index pinned nulls:'last' and emitted DESC NULLS LAST, but the desc() order-by helper emits a bare DESC (Postgres default NULLS FIRST). Postgres matches indexes to ORDER BY on pathkeys including the nulls flag, so the index could never serve the ORDER BY ... DESC LIMIT it was built for. Stop pinning nulls:'last' so drizzle-kit applies Postgres's direction default (desc => first), making a bare .desc() index emit DESC NULLS FIRST to match the query. asc columns and explicit nullsLast/nullsFirst unchanged. Fixes drizzle-team#5978
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #5978.
What / Why
A
.desc()index and thedesc()order-by helper disagree on NULLS ordering, so Postgres silently refuses to use the index for theORDER BY ... DESC LIMITit was created for.nulls: 'last'for every column, so a.desc()index emits... DESC NULLS LAST.desc()order-by helper emits a baredesc, which Postgres reads with itsDESCdefault ofNULLS FIRST.Postgres matches an index to an
ORDER BYon pathkeys that include the nulls flag (and does not consultNOT NULL), so aDESC NULLS LASTindex can never serveORDER BY col DESC— the planner falls back to a full scan + sort. That is the exact "index ignored forORDER BY ... DESC LIMIT" symptom in #5978.Change
drizzle-orm/src/pg-core/columns/common.ts: stop pinningnulls: 'last'in the index column config. Leaving it undefined lets the drizzle-kit serializer apply Postgres's own direction-aware default (desc => first,asc => last), so a bare.desc()index now emitsDESC NULLS FIRST— matching whatdesc()order-by produces.Only bare
.desc()changes:.asc()/ default columns still resolve toNULLS LAST(unchanged)..desc().nullsLast()/.nullsFirst()are honored as before.This is the reporter's preferred alignment direction (align the index to the query default), and it leaves the
desc()order-by output — and therefore every existing query's NULL placement — untouched.Tests
drizzle-kit/tests/indexes/pg.test.ts: added a regression test asserting the.desc()index DDL is... DESC NULLS FIRSTand thatdesc()order-by emits a baredesc(the sameNULLS FIRSTdefault), so the two agree. Updated the existing index tests (and the DB-gated push test) that encoded the oldDESC NULLS LASToutput for implicit.desc()columns to the correctedNULLS FIRST.Note on generated migrations
This changes generated DDL for existing implicit
.desc()indexes: the nextdrizzle-kit generatedrops + recreates them asDESC NULLS FIRST. That is the intended correction (the old index was unusable for the query it was built for), but flagging it so the snapshot change is expected.gel-corehas the identical latent line (gel-core/columns/common.ts); left untouched to keep this fix scoped to the Postgres issue, but happy to mirror it.I based this on
main; glad to retarget torc5if you prefer — the affected line is identical there (common.ts:583).