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

Model::whereNot()->orWhere() incorrectly modifies query structure with SoftDeletes trait or inside a Scope. #50078

Closed
GusMilc opened this issue Feb 13, 2024 · 3 comments · Fixed by #50207

Comments

@GusMilc
Copy link

GusMilc commented Feb 13, 2024

Laravel Version

10.43.0

PHP Version

8.2.4

Database Driver & Version

No response

Description

I ran a command similar to this on my project and got an unexpected result:

Product::whereNot("status", "V")->orWhereNull("synced_at")->get();

I debugged using the function toRawSql()

Product::whereNot("status", "V")->orWhereNull("synced_at")->toRawSql();

and got this sql:

select * from "products" where not (not "status" = 'V' or "synced_at" is null) and "products"."deleted_at" is null

I notice that the problem happened after I added the "SoftDeletes" trait to my Model. If I remove the trait, the sql returns correctly:

select * from "products" where not "status" = 'V' or "synced_at" is null

I start debugging and checked that the issue also happens when using a scope, for example: Product::invalid()->get();

When I change the order of the conditions, the program returns the correct sql:

Product::whereNull("synced_at")->orWhereNot("status", "V")->toRawSql();

result: select * from "products" where "synced_at" is null or not "status" = 'V' and "products"."deleted_at" is null

It happens in:

  • whereNot()->orWhere() (inside a scope or a model with SoftDeletes trait) ❌
  • whereNot()->orWhereNull() (inside a scope or a model with SoftDeletes trait) ❌

It doesn't happen if you invert the order:

  • whereNull()->orWhereNot() ✅
  • where()->orWhereNot() ✅

It doesnt happen without SoftDeletes trait and directly outside of a scope. ✅

It doesn't happen with the "and" where:

  • whereNot()->where() ✅
  • whereNot()->whereNull() ✅

Here are some prints I took from my TinkerWell when I was debugging:

image

image

Steps To Reproduce

First, create a Product model with a scope that uses whereNot and orWhere (in my case, I used orWhereNull but you can use orWhere).

<?php

namespace App\Models;

use Illuminate\Database\Eloquent\Builder;
use Illuminate\Database\Eloquent\Model;

class Product extends Model
{

    protected $fillable = [
        'status',
        'synced_at',
    ];

    /*
    *   if not V or not synced, then it is invalid
    */
    public function scopeInvalid(Builder $query): void
    {
        $query->whereNot('status','V')->orWhereNull('synced_at');
    }

}

Now, create the query using whereNot and orWhereNot functions:

Product::whereNot("status", "V") ->orWhereNull("synced_at")->toRawSql();

The code above should result in the expected query:
select * from "products" where not "status" = 'V' or "synced_at" is null

Finally create the query using scope:

Product::invalid()->toRawSql();

The second query should resoult in an unexpected query with the condition inside a not ():

select * from "products" where not (not "status" = 'V' or "synced_at" is null)


Add the SoftDeletes trait to the model:

<?php

namespace App\Models;

use Illuminate\Database\Eloquent\Builder;
use Illuminate\Database\Eloquent\Model;
use Illuminate\Database\Eloquent\SoftDeletes;

class Product extends Model
{

    use SoftDeletes;

    protected $fillable = [
        'status',
        'synced_at',
    ];

    /*
    *   if not V or not synced, then it is invalid
    */
    public function scopeInvalid(Builder $query): void
    {
        $query->whereNot('status','V')->orWhere('synced_at',null);
    }

}

Let's run again the queries that we made:

Product::whereNot("status", "V") ->orWhereNull("synced_at")->toRawSql();

The code above should result in the unexpected query:
select * from "products" where not (not "status" = 'V' or "synced_at" is null) and "products"."deleted_at" is null

Again, using the scope:

Product::invalid()->toRawSql();

And we should get the incorrect query again.

select * from "products" where not (not "status" = 'V' or "synced_at" is null) and "products"."deleted_at" is null

Copy link

Thank you for reporting this issue!

As Laravel is an open source project, we rely on the community to help us diagnose and fix issues as it is not possible to research and fix every issue reported to us via GitHub.

If possible, please make a pull request fixing the issue you have described, along with corresponding tests. All pull requests are promptly reviewed by the Laravel team.

Thank you!

@amjpdevp
Copy link

To fix this issue, you can modify your scope to use explicit parentheses to control the logic of the conditions. Here's an adjusted version of your scopeInvalid method:

public function scopeInvalid(Builder $query): void
{
    $query->where(function ($query) {
        $query->whereNot('status', 'V')->orWhereNull('synced_at');
    })->whereNull('deleted_at');
}

With this modification, the conditions inside the where closure will be grouped together, ensuring the correct logic in the generated SQL query.

This should result in the expected SQL query:

select * from "products" where (not "status" = 'V' or "synced_at" is null) and "products"."deleted_at" is null

By explicitly grouping the conditions using the where closure, you can control the logical structure of the query and avoid unexpected behavior caused by the combination of SoftDeletes and the specific order of whereNot and orWhere.

@Guilhem-DELAITRE
Copy link
Contributor

I tried to write a fix, let me know your thoughts : #50207

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants