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.28.0] ->orWhere() behaviour changed with array input #53184

Closed
Dmitrev opened this issue Oct 16, 2024 · 9 comments
Closed

[11.28.0] ->orWhere() behaviour changed with array input #53184

Dmitrev opened this issue Oct 16, 2024 · 9 comments

Comments

@Dmitrev
Copy link

Dmitrev commented Oct 16, 2024

Laravel Version

11.28.0

PHP Version

8.2.24

Database Driver & Version

No response

Description

I'm not sure if this was a bug and is now fixed, but either way it's a breaking change and thought it was worth creating an issue for.

The Query builder ->orWhere() command by default now combines statements using the OR rather than AND keywords

I found an closed PR that is related to this:
#53147

The following change broke the functionality:
https://github.com/laravel/framework/pull/53147/files#diff-2ed94a0ea151404a12f3c0d52ae9fb5742348578ec4a8ff79d079fa598ff145d

Perhaps this needed to be patch for Query builder as well? PR was only targeting Eloquent

I think the Query builder should probably have the same functionality of passing a boolean param to avoid having the functionality be inconsistent between the two.

There is a clear workaround in this case, which is to not use the array syntax and instead use the orWhere with a callback like this:

$query->orWhere(function(Builder $query) {
    $query->where(...)->where(...);
});

Steps To Reproduce

Run the following in tinker (should work on any clean install of Laravel):

DB::table('users')->orWhere([['id', '=', 1], ['id', '=', 2]])->toRawSql();

output Laravel 11.28.0:

= "select * from `users` where (`id` = 1 or `id` = 2)"

output Laravel 11.23.5 (notice the and instead of or)

= "select * from `users` where (`id` = 1 and `id` = 2)"
@Dmitrev Dmitrev changed the title ->orWhere() behaviour changed with array input between version 11.23.5 & 11.28.0 [11.28.0] ->orWhere() behaviour changed with array input Oct 16, 2024
@Dmitrev
Copy link
Author

Dmitrev commented Oct 16, 2024

I was able to do some additional testing and narrowed the issue down to the commit that caused the issue.

Remove the boolean here, reverts back the behaviour.
https://github.com/laravel/framework/pull/53147/files#diff-2ed94a0ea151404a12f3c0d52ae9fb5742348578ec4a8ff79d079fa598ff145d

@sivabalan02
Copy link

sivabalan02 commented Oct 16, 2024

I also face the same issue. Reverting back to 11.22 helps

@hafezdivandari
Copy link
Contributor

@timacdonald you may want to take a look at this.

@zero-to-prod
Copy link

zero-to-prod commented Oct 16, 2024

The problem is that the elements in the array are or joined instead of and joined.

Take a look at the $boolean in the where() method.

This is the line that introduced the change of behavior:

$query->{$method}(...array_values($value), boolean: $boolean);

Passing the boolean: $boolean argument does this.

@taylorotwell
Copy link
Member

Hmm, this is a tough one. The inconsistent behavior that @timacdonald fixed makes this feel like a bugfix to me, but it is breaking since people have relied on the inconsistent behavior.

@zero-to-prod
Copy link

ave relied on the inconsistent behavior.

Definitly feels like a Hyrum's Law thing.

@hafezdivandari
Copy link
Contributor

@taylorotwell It seem that the changes on the PR are invalid, the boolean applies to the outer constraint not the nested ones?

User::where('active', true)->orWhere(['name' => 'Tim', 'verified' => true])->toRawSql();

// This one is wrong?
// select * from "users" where "active" = 1 or ("name" = 'Tim' or "verified" = 1)"

User::where('active', true)->orWhere([['name', 'Tim'], ['verified', true]])->toRawSql();

// This one was correct, but broken in the PR?

// Before the PR:
// select * from "users" where "active" = 1 or ("name" = 'Tim' and "verified" = 1)

// After the PR:
// select * from "users" where "active" = 1 or ("name" = 'Tim' or "verified" = 1)

@timacdonald
Copy link
Member

Got a fix up for this. Unfortunately the array syntax results in inconsistent queries depending on which one you use.

Sorry for the troubles here, folks. I've added a heap of additional tests to hopefully ensure we don't hit this again.

#53197

@Afrowson
Copy link

@taylorotwell We also relied on this "feature" since 2021... now we have to fix our code... that is OK. I can see why it was a "bug" but please... if you decide to not revert the breaking change, at least comment it on the Laravel11 Migration guide... I spent far to much time debugging the root issue of my failing tests...

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

No branches or pull requests

8 participants