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

Merge 2.20.x up into 3.3.x #11664

Merged
merged 13 commits into from
Oct 12, 2024
Merged

Merge 2.20.x up into 3.3.x #11664

merged 13 commits into from
Oct 12, 2024

Conversation

greg0ire
Copy link
Member

No description provided.

mpdude and others added 12 commits October 9, 2024 15:37
catch blocks are not supposed to fail. If you want to do something
despite an exception happening, you should do it in a finally block.

Closes doctrine#7545
It maybe happen that the SQL COMMIT statement is successful, but then
something goes wrong. In that kind of case, you do not want to attempt a
rollback.

This was implemented in UnitOfWork::commit(), but for some reason not in
the similar EntityManager methods.
This should make a warning we have in the CI go away.

>  fatal: No url found for submodule path 'docs/en/_theme' in .gitmodules
…ctrine#11188)

* Add a test covering the doctrine#11112 issue

* Add new OutputWalker and SqlFinalizer interfaces

* Add a SingleSelectSqlFinalizer that can take care of adding offset/limit as well as locking mode statements to a given SQL query.

Add a FinalizedSelectExecutor that executes given, finalized SQL statements.

* In SqlWalker, split SQL query generation into the two parts that shall happen before and after the finalization phase.

Move the part that generates "pre-finalization" SQL into a dedicated method. Use a side channel in SingleSelectSqlFinalizer to access the "finalization" logic and avoid duplication.

* Fix CS violations

* Skip the GH11112 test while applying refactorings

* Avoid a Psalm complaint due to invalid (?) docblock syntax

* Establish alternate code path - queries can obtain the sql executor through the finalizer, parser knows about output walkers yielding finalizers

* Remove a possibly premature comment

* Re-enable the doctrine#11112 test

* Fix CS

* Make RootTypeWalker inherit from SqlOutputWalker so it becomes finalizer-aware

* Update QueryCacheTest, since first/max results no longer need extra cache entries

* Fix ParserResultSerializationTest by forcing the parser to produce a ParserResult of the old kind (with the executor already constructed)

* Fix WhereInWalkerTest

* Update lib/Doctrine/ORM/Query/Exec/PreparedExecutorFinalizer.php

Co-authored-by: Grégoire Paris <[email protected]>

* Fix tests

* Fix a Psalm complaint

* Fix a test

* Fix CS

* Make the NullSqlWalker an instance of SqlOutputWalker

* Avoid multiple cache entries caused by LimitSubqueryOutputWalker

* Fix Psalm complaints

* Fix static analysis complaints

* Remove experimental code that I committed accidentally

* Remove unnecessary baseline entry

* Make AddUnknownQueryComponentWalker subclass SqlOutputWalker

That way, we have no remaining classes in the codebase subclassing SqlWalker but not SqlOutputWalker

* Use more expressive exception classes

* Add a deprecation message

* Move SqlExecutor creation to ParserResult, to minimize public methods available on it

* Avoid keeping the SqlExecutor in the Query, since it must be generated just in time (e. g. in case Query parameters change)

* Address PHPStan complaints

* Fix tests

* Small refactorings

* Add an upgrade notice

* Small refactorings

* Update the Psalm baseline

* Add a missing namespace import

* Update Psalm baseline

* Fix CS

* Fix Psalm baseline

---------

Co-authored-by: Grégoire Paris <[email protected]>
…eservesTheOriginalExceptionOnRollbackFailure()`
test: cover all transactional methods in `EntityManagerTest::testItPreservesTheOriginalExceptionOnRollbackFailure()`
@greg0ire

This comment was marked as resolved.

@greg0ire greg0ire merged commit a321331 into doctrine:3.3.x Oct 12, 2024
87 checks passed
@greg0ire greg0ire deleted the 3.3.x branch October 12, 2024 09:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants