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

Eloquent: firstWhere returns wrong result when passing an operator as value #41098

Closed
marvinrabe opened this issue Feb 18, 2022 · 9 comments · Fixed by #41099
Closed

Eloquent: firstWhere returns wrong result when passing an operator as value #41098

marvinrabe opened this issue Feb 18, 2022 · 9 comments · Fixed by #41099
Labels

Comments

@marvinrabe
Copy link
Contributor

marvinrabe commented Feb 18, 2022

  • Laravel Version: 8.83.x, 9.0.x
  • PHP Version: 8.1.1
  • Database Driver & Version: Postgresql 10

Description:

When calling the firstWhere() method of a Model with the second Parameter being "-" and when there is no database entry with "-" it returns a seemingly random Model object.

Probably it assumes "-" is the operator and value is "null". Which is not correct.

Steps To Reproduce:

Failing Test:

        User::factory()->create([
            "name" => 'Foo'
        ]);

        $user = User::firstWhere('name', '-');

        $this->assertNull($user);

This test is using the User model. But every other model will do.

When explicitly specifying an operator it works as expected:

$user = User::query()->firstWhere('name', '=', '-');
@marvinrabe marvinrabe changed the title Eloquent/DB: firstWhere returns Object instead of NULL Eloquent: firstWhere returns Object instead of NULL Feb 18, 2022
@driesvints
Copy link
Member

I think this is Postgres specific behavior? I get null back on MySQL.

@marvinrabe
Copy link
Contributor Author

marvinrabe commented Feb 18, 2022

@driesvints It is still a Laravel bug though.

Try something different (this one should work in MySQL too):

        User::factory()->create([
            "name" => '='
        ]);

        $user = User::firstWhere('name', '=');

        $this->assertNotNull($user); // FAIL

You are expecting it is looking for a user with name =. But it does something completely different. The query looks something like this:

SELECT * FROM 'users' WHERE name = null

When using where and first it gives you a different result:

        User::factory()->create([
            "name" => '='
        ]);

        $user = User::where('name', '=')->first();

        $this->assertNotNull($user); // PASS

This one does the correct query:

SELECT * FROM 'users' WHERE name = '='

Passing 2 arguments to a where function (even firstWhere) should result in the default operator being = in all cases. It is a problem with passing the arguments from the firstWhere to the actual where function.

The query builder will always receive more than 2 arguments when it is called through firstWhere:

[$value, $operator] = $this->prepareValueAndOperator(
$value, $operator, func_num_args() === 2
);

@driesvints
Copy link
Member

@marvinrabe I still get null for your first example. Please try a support channel.

@marvinrabe
Copy link
Contributor Author

marvinrabe commented Feb 18, 2022

@driesvints Please note the assertion is different that time.

        User::factory()->create([
            "name" => '='
        ]);

        $user = User::firstWhere('name', '=');

        $this->assertNotNull($user); // FAIL

This $user should not be NULL in this case.

@driesvints
Copy link
Member

Hah, I finally managed to reproduce that, thanks.

@driesvints driesvints reopened this Feb 18, 2022
@driesvints driesvints added the bug label Feb 18, 2022
@marvinrabe
Copy link
Contributor Author

The problem only occurs when the value (passing it as $operator) is a valid Operator according to the specific \Illuminate\Database\Query\Grammars\Grammar. Otherwise it is fixed on this line:

if ($this->invalidOperator($operator)) {
[$value, $operator] = [$operator, '='];
}

@marvinrabe marvinrabe changed the title Eloquent: firstWhere returns Object instead of NULL Eloquent: firstWhere returns wrong result when passing a valid operator as second argument without a third Feb 18, 2022
@marvinrabe marvinrabe changed the title Eloquent: firstWhere returns wrong result when passing a valid operator as second argument without a third Eloquent: firstWhere returns wrong result when passing a valid operator as value Feb 18, 2022
@marvinrabe marvinrabe changed the title Eloquent: firstWhere returns wrong result when passing a valid operator as value Eloquent: firstWhere returns wrong result when passing an operator as value Feb 18, 2022
@marvinrabe
Copy link
Contributor Author

This issue also effects passing enums to the firstWhere method:

enum SpecialPeople: string
{
    case Taylor = 'Taylor Otwell';
}

User::firstWhere('name', SpecialPeople::Taylor);

The above throws an error while validating SpecialPeople::Taylor as an operator:

TypeError : strtolower(): Argument #1 ($string) must be of type string, Tests\Feature\Suit given

This would not happen when the default operator is correctly applied on Line 706 (Builder).

@driesvints
Copy link
Member

@marvinrabe that seems more of a feature request to me.

@marvinrabe
Copy link
Contributor Author

marvinrabe commented Feb 18, 2022

@driesvints It would be a feature request if I wanted to pass an enum as an operator. But in this case the Enum should be the value. But because of the error in the default handling it is not working correctly. This is an additional unwanted side effect of always passing more than 2 parameters from firstWhere to where. Fixing this issue, fixes passing Enums as value to firstWhere too.

Please note that in this case everything works perfectly:

enum SpecialPeople: string
{
    case Taylor = 'Taylor Otwell';
}

User::firstWhere('name', '=', SpecialPeople::Taylor);

This error exists because of validating an operator, that should not be an operator in the first place.

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

Successfully merging a pull request may close this issue.

2 participants