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

using orWhere on a soft delete model returns deleted items? #1945

Closed
adamkearsley opened this issue Jul 25, 2013 · 16 comments
Closed

using orWhere on a soft delete model returns deleted items? #1945

adamkearsley opened this issue Jul 25, 2013 · 16 comments
Labels

Comments

@adamkearsley
Copy link

I'm having an issue using an eloquent query to search for my users.

$users = User::where('email', 'like', '%'.$search.'%')->orWhere('brid', 'like', '%'.$search.'%')->orderBy('brid', 'ASC')->paginate(5);

This is returning deleted and un-deleted users.

I am not using the withTrashed() function, so i should be just getting my un-deleted users in the result.

If i remove the orWhere() it then works correctly.

Is this something that can be fixed in Eloquent?

@adamkearsley
Copy link
Author

I can get passed the issue using

$users = User::where(function($query) use ($search)
            {
                $query->where('email', 'like', '%'.$search.'%')
                      ->orWhere('brid', 'like', '%'.$search.'%');
            })
            ->orderBy('brid', 'ASC')->paginate(5);

But this still seems like it should be done within the soft delete function.

@WMeldon
Copy link
Contributor

WMeldon commented Jul 25, 2013

Just out of curiosity, what is the query you get with your original statement? (var_dump(DB::getQueryLog()))

@adamkearsley
Copy link
Author

select * from `users` where `users`.`deleted_at` is null and `email` like ? or `brid` like ? order by `brid` ASC limit 5 offset 0

the or 'brid' like ? part is overriding the deleted_at.

@JoostK
Copy link
Contributor

JoostK commented Jul 25, 2013

You have to use:

$users = User::where(function($query)
{
    $query->where('email', 'like', '%'.$search.'%')->orWhere('brid', 'like', '%'.$search.'%');
})->orderBy('brid', 'ASC')->paginate(5);

This way your wheres are properly enclosed in parenthesis.

@adamkearsley
Copy link
Author

No offence JoostK, but you've pretty much just copied and pasted my own reply above...
Except you missed abit.
I'm aware of how to go around the problem, but that doesn't fix the bug.
Wherever the extra where clause is for the soft delete, needs to be enclosed on its own.
Or something similar.
I have got passed the issue as posted above, but it's a bridge, not a fix!
Thanks
Adam

@crynobone
Copy link
Member

I do agree with @adamkearsley that there is a flaw with the generated sql.
👍 nice catch.

On Friday, July 26, 2013, adamkearsley wrote:

No offence JoostK, but you've pretty much just copied and pasted my own
reply above...
Except you missed abit.
I'm aware of how to go around the problem, but that doesn't fix the bug.
Wherever the extra where clause is for the soft delete, needs to be
enclosed on its own.
Or something similar.
I have got passed the issue as posted above, but it's a bridge, not a fix!
Thanks
Adam


Reply to this email directly or view it on GitHubhttps://github.com//issues/1945#issuecomment-21586354
.

@JoostK
Copy link
Contributor

JoostK commented Jul 25, 2013

I'm sorry, totally missed that post. I can't think of a nice solution that would fix this gotcha.

@taylorotwell
Copy link
Member

I honestly don't think this is a "bug", but just a logical way SQL works. The "work-around" should be used to group the statements logically.

@NicholasTurner
Copy link

Ran into this issue today. I don't see how it isn't a bug, but if it isn't going to be fixed, could you at least update the documentation to reflect soft deletion not working with 'orWhere' statements?

@grumpysi
Copy link

I just ran into this problem and I agree with Taylor, just logically group. It's not up to the soft delete trait to do this.

->where(function($query) use ($name)
    {
        $query->where('firstname', '=', $name);
        $query->orWhere('lastname', '=', $name);
    })

@mediascreen
Copy link

While this is a logical result of the way the queries are built, the documentation is seriously misleading:

When querying a model that uses soft deletes, the "deleted" models will not be included in query results.

With automagical features like this, you kind of expect them to work as described. If they don't, maybe it's better to just explain the technical details of what they do.

@mustafaaloko
Copy link
Contributor

@taylorotwell Is it worth it to update the docs for this issue? if yes, I will do it.

@taylorotwell
Copy link
Member

Sure send it over.

On Mon, Nov 2, 2015 at 4:48 AM, Mustafa Ehsan Alokozay
notifications@github.com wrote:

@taylorotwell Is it worth it to update the docs for this issue? if yes, I will do it.

Reply to this email directly or view it on GitHub:
#1945 (comment)

@antocorso
Copy link

antocorso commented Aug 11, 2016

@taylorotwell I think that this is a terrible bug (of course not an sql bug but an eloquent one), why in the above example should be retrieved deleted items? It doesn't make sense!!!
Eloquent in this case generate a not expected and very dangerous query!

@KonstantinObuhov
Copy link

I can get passed the issue using
$query->whereNull('deleted_at');
at the end of the query before calling
$query->paginate($perPage, $page);

@EmadBrown
Copy link

EmadBrown commented Apr 7, 2018

I solved it like that after each orWhere(...)->withoutTrashed(); The same things in case of onlyTrashed();
You can solve it like that :
$users = User::where('email', 'like', '%'.$search.'%')->orWhere('brid', 'like', '%'.$search.'%') ->withoutTrashed()->orderBy('brid', 'ASC')->paginate(5);

@KonstantinObuhov But in case if you have more then orWhere in side de query, it doesn't work . But you can use also after each orWhere(....)->whereNull('deleted_at') it did work well!

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

No branches or pull requests