feat: Always add LIMIT 1 to findOne queries (#14549)

[stuartnelson3]

Pull Request Checklist

• Have you added new tests to prevent regressions? No. This removes an undocumented behavior, and it deletes the five tests that asserted it, exactly as the v7 commit did.
• Does npm run test or npm run test-DIALECT pass with this change (including linting)? The unit suite passes, 1387 tests. eslint src/model.js test/unit/model/find-one.test.js is clean. See below.
• Is a documentation update included? No. This removes an undocumented behavior.
• Did you update the typescript typings accordingly? Not applicable. No signature changes.
• Does the description below contain a link to an existing issue? feat: Always add LIMIT 1 to findOne queries #14549, findOne does not add 'LIMIT 1' when first column of composite primary key is passed to 'where' option #13479, findOne doesn't generate a corrected SQL with partial primary key #9859.
• Did you follow the commit message conventions explained in CONTRIBUTING.md? Yes, the original commit message is preserved.

Description Of Change

This is a cherry-pick of 9950b4b, "feat: Always add LIMIT 1 to findOne queries" (#14549),
onto the v6 branch. It carries both files from the original commit: src/model.js and
the test removal.

#17726 already cherry-picked the same source change. @WikiRik noted in March 2025 that the
tests removed in 9950b4b are also present on v6, and that PR touches src/model.js only,
so those tests still fail. This PR closes that gap. I am happy to close this one if you
prefer to keep #17726 and land the test removal there instead.

Why the current behavior is wrong

findOne keeps LIMIT 1 off the query when the where clause names primaryKeyAttribute.
On a composite primary key, primaryKeyAttribute is only the first column. One value of
that column matches many rows, so the query returns all of them.

The same block already guards the equivalent case for unique keys. It filters uniqueKeys
down to c.fields.length === 1, so a multi-column unique key keeps its LIMIT. A
multi-column primary key gets no such guard:

sequelize/src/model.js

Lines 1984 to 1994 in cb7f99a

	if (options.limit === undefined) {
	const uniqueSingleColumns = _.chain(this.uniqueKeys).values().filter(c => c.fields.length === 1).map('column').value();
	// Don't add limit if querying directly on the pk or a unique column
	if (!options.where || !_.some(options.where, (value, key) =>
	(key === this.primaryKeyAttribute || uniqueSingleColumns.includes(key)) &&
	(Utils.isPrimitive(value) || Buffer.isBuffer(value))
	)) {
	options.limit = 1;
	}
	}

Two earlier reports of this closed without a fix. #9859 (2018) closed with "Sequelize at
this time does not support partial primary keys, we recommend id as primary key", which no
longer holds. #13479 (2021) described the composite primary key case with an SSCCE, and the
stale bot closed it.

Reproduction on 6.37.8

const RunItem = sequelize.define('RunItem', {
  runId: {type: DataTypes.STRING, primaryKey: true},
  itemId: {type: DataTypes.STRING, primaryKey: true},
  status: {type: DataTypes.STRING},
}, {tableName: 'run_items', timestamps: false})

// three rows share runId 'r1' and status 'enqueued'
await RunItem.findOne({where: {runId: 'r1', status: 'enqueued'}})           // 1
await RunItem.findOne({where: {status: 'enqueued'}})                        // 2
await RunItem.findOne({where: {runId: 'r1', status: 'enqueued'}, limit: 1}) // 3

primaryKeyAttribute  : runId
primaryKeyAttributes : ["runId","itemId"]

1. SELECT ... WHERE "runId" = 'r1' AND "status" = 'enqueued';            <- no LIMIT
2. SELECT ... WHERE "status" = 'enqueued' LIMIT 1;
3. SELECT ... WHERE "runId" = 'r1' AND "status" = 'enqueued' LIMIT 1;

Case 2 is the proof. Drop the primary key column from the where clause and the LIMIT
comes back.

What it cost us

We used findOne as an existence probe on a table with the three-column primary key
(run_id, subject_id, subject_type). The where clause named run_id and a status, so
every call read every row of the run and built a model instance for each one.

EXPLAIN (ANALYZE, BUFFERS) on a copy of the production table, 180,687 matching rows:

Query	Execution time	Rows returned
count(*)	25.488 ms	1
findOne, no LIMIT	22.442 ms	180,687
findOne, limit: 1	0.015 ms	1

All three plans take the same index-only scan with Heap Fetches: 0. No plan flip. Only the
row count differs. The probe ran about 147,000 times per nightly job, which added about
1,750 s of database time and 10 minutes of wall-clock to a 613-minute run.

The row count is also why this is more than a wire-format nicety. Those rows became 180,687
model instances per call, and no database metric showed it. Per-call database time rose only
from 17 ms to 29 ms.

Verification

The full unit suite on this branch:

$ node ./build.js && DIALECT=postgres npx mocha -r ./test/registerEsbuild "test/unit/**/*.test.[tj]s"
  1387 passing (500ms)

The two remaining cases in test/unit/model/find-one.test.js still pass, because both assert
that limit is present:

  [POSTGRES] Model
    method findOne
      ✓ should add limit when using { $ gt on the primary key
      ✓ should add limit when using multi-column unique key
  2 passing (8ms)

Restore the deleted tests and keep the source change, and exactly the five expected cases
fail, which is the gap #17726 leaves open:

        1) with id primary key
        2) with custom primary key
        3) with blob primary key
        4) with custom unique key
        5) with blob unique key
  2 passing
  5 failing
     AssertionError: expected { where: { id: 42 }, limit: 1, …(1) } to not have property 'limit'

grep -rn "uniqueSingleColumns" src/ test/ returns nothing after this change, and no other
test on v6 asserts the removed behavior.

Notes

• An explicit limit still wins. The if (options.limit === undefined) guard is unchanged,
  so the temporary workaround for Deleting PK column does not prevent MSSQL from ordering by PK on LIMIT 1 queries. #14618 carries over exactly as written in 9950b4b.
• I did not touch the integration suite. I have no MSSQL or DB2 instance to hand, and Deleting PK column does not prevent MSSQL from ordering by PK on LIMIT 1 queries. #14618
  is the known MSSQL interaction, already handled by the guard above.

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 75ba54d3-4a0e-44c3-9ee4-1e6edf3292f6

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[stuartnelson3]

44 pass, 7 fail. Four failures are not from this diff, three are. All MSSQL legs pass, so
#14618 stays handled by the options.limit === undefined guard.

Postgres native on Node 10, four legs. Not this diff. ci.yml:168 runs
yarn add pg-native --ignore-engines, so CI installs pg-native unpinned. Version 3.9.0
shipped on 2026-08-08 and uses ??, which Node 10 cannot parse. It crashes at module load,
before mocha runs a test. The last CI run on v6 predates that release, so the branch never
showed it. Every pull request against v6 now fails these four legs.

Oracle, three legs. This diff. ORA-02014 on the row locking tests. The Oracle branch
at test/integration/transaction.test.js:830 uses findByPk to avoid FOR UPDATE with
FETCH. findByPk calls findOne, so the heuristic kept the LIMIT off. It no longer
does, and Oracle rejects the pair. This is a real conflict rather than a test artifact: a
locking single-row read is now unreachable on Oracle.

v7 never met this. Its Oracle dialect declares lock: false, so supports.lock gates the
whole block out. v6 declares lock: true at src/dialects/oracle/index.js:31, and the
Oracle dialect arrived after #14549 merged.

Do you want lock: false on v6 Oracle, to match v7? One line, and CI goes green. The cost
is that findAll({lock: true}) on Oracle stops emitting FOR UPDATE. That is your
compatibility call, so I left it out.