[10.x] Fix createOrFirst on transactions

[tonysm]

Added

• Adds a new withSavepointIfNeeded(fn) method to the Eloquent query Builder that wraps the given scope in a transaction if there's a transaction going on

Changed

• Wraps the create or attach parts of the createOrFirst within a save point transaction using the withSavePointIfNeeded method

---

Fixes #48143

Looks like we need to wrap the insert part of a the createOrFirst within a save point transaction for PostgreSQL, because otherwise the find part will fail.

[tonysm]

Not sure about adding this public method. I thought about just wrapping the insert part in a DB::transaction(fn), but not sure what y'all would think of that. I initially had it as protected, but I needed the same logic in the relationship part of the feature.

Let me know what y'all think. Open to suggestions.

[mpyw]

For MySQL/SQLServer/SQLite, this is not necessary; Postgres just needs it.

[tonysm]

Yeah, I figured it wouldn't be a problem to wrap in savepoint transactions for the other drivers as well. It doesn't seem to hurt.

[tonysm]

I really liked how it read before: createOrFirst was essentially calling create() then first(), now it has more noise. I thought about pushing the savepoint to the create() method, which would keep this part as it was, but then it would be a bigger change.

[crynobone]

Marking this as draft as there some failing tests need to be updated.

[tonysm]

@crynobone fixed the tests. Not sure why the SQLServer suite is failing, but seems to be failing on other PRs too, btw. Should I mark this PR as ready for review?

[mpyw]

We may need this: mpyw/laravel-unique-violation-detector@0049281

[mpyw]

While this feature is highly beneficial, it seems to have a considerable impact. Personally, I feel it might have been better suited for Laravel 11. Even though it's already been incorporated into 10.20.0, might we consider reverting it and then reintroducing it as a target for version 11?

[mpyw]

@taylorotwell Can I have your opinion?

[tonysm]

We may need this: mpyw/laravel-unique-violation-detector@0049281

Yeah, I think I had to comment out this line here to run the SQLServer tests (unrelated to the changes in createOrFirst, though. I'm not on my computer right now to check), but the suite was passing in the original PR. 🤔

[taylorotwell]

@tonysm any ideas on what to do about the SQL Server tests?

[tonysm]

@taylorotwell I just tested it over here. If I comment out this line, the SQLServer tests pass locally. With that in, it looks like all of them are broken.

I found this reference from Drupal, where they are using a try/catch when setting the PDO attributes. It looks like something changed in the SQL Server extension. Some other references from php-src and a Laravel issue from 3 weeks ago. A PR was merged already in the mssql extension, but I still see the issue. It looks like they are arranging a hotfix. In the meantime, the solution described in the package referenced in this comment seems to be similar to what the Drupal folks did.

Either way, it seems unrelated to the changes in the createOrFirst.

[tonysm]

Looks like there are some flaky tests also unrelated to createOrFirst and the changes here. Might have some time later to fix them, but it might be better to handle that in a separate PR. I cannot re-run the action myself, but that should "fix" it for now if anyone with the permission to do that can do it.