Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[11.x] Add whereNotAll method to the query builder #52361

Closed
wants to merge 2 commits into from

Conversation

lmottasin
Copy link
Contributor

@lmottasin lmottasin commented Aug 2, 2024

Overview

In PR #50344, the whereAll and whereAny methods were introduced to enhance the framework's querying capabilities. Subsequently, PR #52260 introduced the whereNone method, which complements whereAny by providing a negated alternative.

To further complete the logical set, this pull request introduces the whereNotAll method. This method serves as the opposite of whereAll and allows users to query for records where not all of the specified columns meet the given condition. By including whereNotAll, we finalize the logical quartet of "any," "all," "none," and "notAll."

This addition provides a comprehensive set of methods for handling complex constraints across multiple columns, enhancing the flexibility of the query builder.

Usage

The whereNoAll method works as follows:

$jobs = DB::table('jobs')
    ->where('active', true)
    ->whereNotAll(['job_type', 'employment_type'], 'LIKE', ['Remote', 'Part-Time'])
    ->get();
SELECT *
FROM jobs
WHERE active = true AND NOT (
  job_type LIKE 'Remote' AND
  employment_type LIKE 'Part-Time'
)

@lmottasin
Copy link
Contributor Author

If merged I will open a PR to doc.

@driesvints
Copy link
Member

Please rebase so tests pass, thanks!

@driesvints driesvints marked this pull request as draft August 2, 2024 11:55
@lmottasin lmottasin marked this pull request as ready for review August 2, 2024 15:15
@einar-hansen
Copy link
Contributor

einar-hansen commented Aug 3, 2024

This PR got me really thinking about the naming of the whereNone query that was published in the last release. With the introduction of whereNotAll, I realized we might want to reconsider the name of whereNone for clarity and consistency, as none is the opposite to both "any" and "all".

I propose renaming whereNone to whereNotAny. Here's why:

  1. It's clearer what the method does in relation to whereAny.
  2. It follows the naming pattern of whereNotAll, and most of the other queries in the framework, for example; whereIn/whereNotIn, whereLike/whereNotLike, whereBetween/whereNotBetween and so on...
  3. It would be easier for developers to understand the relationship between all these methods.

If we make this change, our method names would be:

  • whereAll: Return rows where all conditions are true
  • whereNotAll: Return rows excluding where all conditions are true
  • whereAny: Return rows where at least one condition is true
  • whereNotAny: Return rows excluding where at least one condition is true (currently whereNone)

The name whereNotAny might be a bit clunkier, but I think it provides a clearer developer experience, especially when considered alongside whereNotAll.

If you agree, I'll create a PR with the whereNotAny method, make whereNone deprecated and point to whereNotAny, and update the docs accordingly.

@lmottasin
Copy link
Contributor Author

@einar-hansen Yup. I agree with you even when I was thinking about the whereNotAll method it crossed my mind. Changing to whereNotAny will help in clarity, consistency, and ease of use.

@einar-hansen
Copy link
Contributor

@lmottasin you might want to update the docblocks to accept Expression and set operator to mixed. Take a look here:

https://github.com/laravel/framework/pull/52383/files#diff-2ed94a0ea151404a12f3c0d52ae9fb5742348578ec4a8ff79d079fa598ff145d

     * @param  \Illuminate\Contracts\Database\Query\Expression[]|string[]  $columns
     * @param  mixed  $operator

@taylorotwell
Copy link
Member

I honestly find this entire method really confusing and wouldn't use it in any apps because I have to think so hard about the negations. 😅

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.

4 participants