+1

what is being corrected here is an "anti-feature"

I agree, not adding LIMIT 1 if we're comparing the primary key sounds weird to me. I'd expect Model.findOne() to be equivalent to (await Model.findAll({ limit: 1 }))[0].

Strangely enough, some mssql tests are failing with this change.
If you could ping me once these tests are passing (or if you need help), I'll review this PR once they're green (db2 tests are unstable and can be ignored)

This is the only comment I could find that mentioned this behavior as intended #4416 (comment)

@ephys thanks for looking a this. I reran the mssql tests locally on main and observed the same error. I'll check through the commit history and see if I can't find if it broke with a prior commit.

@ephys , sorry to say I have been able to identify a commit where these tests started failing. I'm wondering if that has come up in other PRs?

To Reiterate, the MSSQL tests fail locally for me on main

are you using start-mssql to start the server?

@ephys, I'm trying to make sure I'm properly running the test locally.
I'll get back to you once I'm certain that I'm getting the same results as CI.

Thanks for your patience.

I've added the conditional pointed out by @WikiRik, and opened up issue #14618

I found the reason why LIMIT was not included #9248. I'm documenting it here for future PRs but I disagree with the premise of that PR and I agree LIMIT should be specified.

Edit: Changed the commit type to feat as it was an intended behavior we're changing instead of a bug

I'll run the tests again after GitHub solves the caching outage https://www.githubstatus.com/incidents/6r4sms3hcj3x

🎉 This PR is included in version 7.0.0-alpha.14 🎉

The release is available on:

• npm package (@release-v7 dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

Hello, I have my doubts that this is correct. Are we sure that findOne queries should really only ever return one row? We are using findOne in conjunction with includes and then, because of outer joins happening, we do get and expect multiple rows. Please correct me if I'm wrong. Thx

Hi :)

In find{One,All}, limit doesn't necessarily mean only one row in the database is going to be fetched. We want limit to only apply to the level it's on, in the redesign. So if you wrote the following:

User.findOne({
  include: [User.associations.projects],
});

It would return a single user, but include all of the user's projects

We know that limit currently behaves in very inconsistent ways, but it's something I'm working on addressing