[Known Issue] Duplicate conditions on retrying SELECT calls under createOrFirst()

[mpyw]

Laravel Version

10.x (>10.28.0 <10.29.0: 4b4b1e4)

PHP Version

irrelevant

Database Driver & Version

irrelevant

Description

The known issue in:

• [10.x] Revival of the reverted changes in 10.25.0: firstOrCreate updateOrCreate improvement through createOrFirst + additional query tests  #48637

Notes

Some FIXME comments are remaining.

$model->getConnection()
    ->expects('select')
    // FIXME: duplicate conditions
    ->with('select * from "table" where ("attr" = ?) and ("attr" = ?) limit 1', ['foo', 'foo'], false)
    ->andReturn([[
        'id' => 123,
        'attr' => 'foo',
        'val' => 'bar',
        'created_at' => '2023-01-01 00:00:00',
        'updated_at' => '2023-01-01 00:00:00',
    ]]);

These indicate a problem with duplicate and useless conditions being given when retrying the SELECT, but do not significantly bother with the operation. Attempting to fix this using clone may yield some unexpected effects due to the Builder mutableness. Therefore, we'd like to recognize this as a known issue and leave it out of the scope of this PR.

@tonysm If you think you can fix this problem, could you please submit another PR? Thank you in advance.

In #48637, although we knew it was a known issue, did not fix it to minimize risk. We will probably clone Builder to fix it, but we need to carefully examine the scope of the impact.

There is no real harm in the operation, since it only adds useless conditions to the query. Therefore, this is more of a refactoring than a bug fix.

[mpyw]

If the impact is greater than expected, consider targeting for 11.x.

[driesvints]

I don't understand. Is there a bug here at all? Did you now create a bug issue for a PR you sent in yourself? Can you send in a PR with your suggested fix?

[mpyw]

@driesvints Sorry for the confusion. Actually:

• It is a bug, but actually does not affect real operations. It just needlessly duplicates the same WHERE conditions.
• It has nothing to do with [10.x] Revival of the reverted changes in 10.25.0: firstOrCreate updateOrCreate improvement through createOrFirst + additional query tests  #48637, but is a bug that has existed since the first introduction of createOrFirst by @tonysm: [10.x] Adds a createOrFirst method to Eloquent #47973. I noticed it while writing snapshot query tests of in my PR, but intentionally did not fix it there to keep the unrelated changes small.

[mpyw]

@tonysm If you have time, could you please make the fix? Thank you in advance.

[tonysm]

My personal life is quite demanding right now, but I'll try to find an hour or two to dig into this.

The issue doesn't seem to be on the createOrFirst, right? It sounds like when the createOrFirst is used inside the firstOrCreate, it carries the original where attributes from the first find attempt, which causes the duplication (firstOrCreate adds a ->where() which gets carried to the last find query used in createOrFirst when it attempts to find the record, duplicating the where clause). It's not a bug, because the query still works. But something we should clean up.

I think cloning the query builder (as suggested in the PR as a possible fix) on the first attempt should resolve the issue, but we need more testing.

[mpyw]

The issue doesn't seem to be on the createOrFirst, right? It sounds like when the createOrFirst is used inside the firstOrCreate, it carries the original where attributes from the first find attempt, which causes the duplication (firstOrCreate adds a ->where() which gets carried to the last find query used in createOrFirst when it attempts to find the record, duplicating the where clause).

Sorry, that's right. Actually #47973 contained the firstOrCreate improvement but already reverted, and most recently reappeared in #48637.

Greatly appreciate for you help.

[github-actions]

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!