Merge origin/2.20.x into 3.3.x (using imerge)

UPGRADE.md

@@ -733,6 +733,15 @@ Use `toIterable()` instead.
 
 # Upgrade to 2.20
 
+## Add `Doctrine\ORM\Query\OutputWalker` interface, deprecate `Doctrine\ORM\Query\SqlWalker::getExecutor()`
+
+Output walkers should implement the new `\Doctrine\ORM\Query\OutputWalker` interface and create
+`Doctrine\ORM\Query\Exec\SqlFinalizer` instances instead of `Doctrine\ORM\Query\Exec\AbstractSqlExecutor`s.
+The output walker must not base its workings on the query `firstResult`/`maxResult` values, so that the 
+`SqlFinalizer` can be kept in the query cache and used regardless of the actual `firstResult`/`maxResult` values.
+Any operation dependent on `firstResult`/`maxResult` should take place within the `SqlFinalizer::createExecutor()`
+method. Details can be found at https://github.com/doctrine/orm/pull/11188.
+
 ## Explictly forbid property hooks
 
 Property hooks are not supported yet by Doctrine ORM. Until support is added,
@@ -741,10 +750,16 @@ change in behavior.
 
 Progress on this is tracked at https://github.com/doctrine/orm/issues/11624 .
 
-## PARTIAL DQL syntax is undeprecated for non-object hydration
+## PARTIAL DQL syntax is undeprecated 
+
+Use of the PARTIAL keyword is not deprecated anymore in DQL, because we will be
+able to support PARTIAL objects with PHP 8.4 Lazy Objects and
+Symfony/VarExporter in a better way. When we decided to remove this feature
+these two abstractions did not exist yet.
 
-Use of the PARTIAL keyword is not deprecated anymore in DQL when used with a hydrator
-that is not creating entities, such as the ArrayHydrator.
+WARNING: If you want to upgrade to 3.x and still use PARTIAL keyword in DQL
+with array or object hydrators, then you have to directly migrate to ORM 3.3.x or higher.
+PARTIAL keyword in DQL is not available in 3.0, 3.1 and 3.2 of ORM.
 
 ## Deprecate `\Doctrine\ORM\Query\Parser::setCustomOutputTreeWalker()`
 

psalm-baseline.xml

@@ -212,14 +212,14 @@
     </ParamNameMismatch>
   </file>
   <file src="src/Internal/Hydration/AbstractHydrator.php">
-    <ReferenceConstraintViolation>
-      <code><![CDATA[return $rowData;]]></code>
-      <code><![CDATA[return $rowData;]]></code>
-    </ReferenceConstraintViolation>
     <PossiblyUndefinedArrayOffset>
       <code><![CDATA[$newObject['args']]]></code>
       <code><![CDATA[$newObject['args']]]></code>
     </PossiblyUndefinedArrayOffset>
+    <ReferenceConstraintViolation>
+      <code><![CDATA[return $rowData;]]></code>
+      <code><![CDATA[return $rowData;]]></code>
+    </ReferenceConstraintViolation>
   </file>
   <file src="src/Internal/Hydration/ArrayHydrator.php">
     <PossiblyInvalidArgument>
@@ -923,6 +923,9 @@
     <ArgumentTypeCoercion>
       <code><![CDATA[$stringPattern]]></code>
     </ArgumentTypeCoercion>
+    <DeprecatedMethod>
+      <code><![CDATA[setSqlExecutor]]></code>
+    </DeprecatedMethod>
     <InvalidNullableReturnType>
       <code><![CDATA[AST\SelectStatement|AST\UpdateStatement|AST\DeleteStatement]]></code>
     </InvalidNullableReturnType>
@@ -1113,6 +1116,12 @@
     </RedundantConditionGivenDocblockType>
   </file>
   <file src="src/Tools/Pagination/LimitSubqueryOutputWalker.php">
+    <InvalidReturnStatement>
+      <code><![CDATA[$abstractSqlExecutor->getSqlStatements()]]></code>
+    </InvalidReturnStatement>
+    <InvalidReturnType>
+      <code><![CDATA[string]]></code>
+    </InvalidReturnType>
     <PossiblyFalseArgument>
       <code><![CDATA[strrpos($orderByItemString, ' ')]]></code>
     </PossiblyFalseArgument>

src/EntityManager.php

@@ -24,7 +24,6 @@
 use Doctrine\ORM\Query\FilterCollection;
 use Doctrine\ORM\Query\ResultSetMapping;
 use Doctrine\ORM\Repository\RepositoryFactory;
-use Throwable;
 
 use function array_keys;
 use function is_array;
@@ -178,18 +177,24 @@ public function wrapInTransaction(callable $func): mixed
     {
         $this->conn->beginTransaction();
 
+        $successful = false;
+
         try {
             $return = $func($this);
 
             $this->flush();
             $this->conn->commit();
 
-            return $return;
-        } catch (Throwable $e) {
-            $this->close();
-            $this->conn->rollBack();
+            $successful = true;
 
-            throw $e;
+            return $return;
+        } finally {
+            if (! $successful) {
+                $this->close();
+                if ($this->conn->isTransactionActive()) {
+                    $this->conn->rollBack();
+                }
+            }
         }
     }
 

src/Query.php

@@ -7,11 +7,14 @@
 use Doctrine\DBAL\LockMode;
 use Doctrine\DBAL\Result;
 use Doctrine\DBAL\Types\Type;
+use Doctrine\Deprecations\Deprecation;
 use Doctrine\ORM\Mapping\ClassMetadata;
 use Doctrine\ORM\Query\AST\DeleteStatement;
 use Doctrine\ORM\Query\AST\SelectStatement;
 use Doctrine\ORM\Query\AST\UpdateStatement;
 use Doctrine\ORM\Query\Exec\AbstractSqlExecutor;
+use Doctrine\ORM\Query\Exec\SqlFinalizer;
+use Doctrine\ORM\Query\OutputWalker;
 use Doctrine\ORM\Query\Parameter;
 use Doctrine\ORM\Query\ParameterTypeInferer;
 use Doctrine\ORM\Query\Parser;
@@ -27,6 +30,7 @@
 use function count;
 use function get_debug_type;
 use function in_array;
+use function is_a;
 use function ksort;
 use function md5;
 use function reset;
@@ -163,7 +167,7 @@ class Query extends AbstractQuery
      */
     public function getSQL(): string|array
     {
-        return $this->parse()->getSqlExecutor()->getSqlStatements();
+        return $this->getSqlExecutor()->getSqlStatements();
     }
 
     /**
@@ -242,7 +246,7 @@ private function parse(): ParserResult
 
     protected function _doExecute(): Result|int
     {
-        $executor = $this->parse()->getSqlExecutor();
+        $executor = $this->getSqlExecutor();
 
         if ($this->queryCacheProfile) {
             $executor->setQueryCacheProfile($this->queryCacheProfile);
@@ -656,11 +660,31 @@ protected function getQueryCacheId(): string
     {
         ksort($this->hints);
 
+        if (! $this->hasHint(self::HINT_CUSTOM_OUTPUT_WALKER)) {
+            // Assume Parser will create the SqlOutputWalker; save is_a call, which might trigger a class load
+            $firstAndMaxResult = '';
+        } else {
+            $outputWalkerClass = $this->getHint(self::HINT_CUSTOM_OUTPUT_WALKER);
+            if (is_a($outputWalkerClass, OutputWalker::class, true)) {
+                $firstAndMaxResult = '';
+            } else {
+                Deprecation::trigger(
+                    'doctrine/orm',
+                    'https://github.com/doctrine/orm/pull/11188/',
+                    'Your output walker class %s should implement %s in order to provide a %s. This also means the output walker should not use the query firstResult/maxResult values, which should be read from the query by the SqlFinalizer only.',
+                    $outputWalkerClass,
+                    OutputWalker::class,
+                    SqlFinalizer::class,
+                );
+                $firstAndMaxResult = '&firstResult=' . $this->firstResult . '&maxResult=' . $this->maxResults;
+            }
+        }
+
         return md5(
             $this->getDQL() . serialize($this->hints) .
             '&platform=' . get_debug_type($this->getEntityManager()->getConnection()->getDatabasePlatform()) .
             ($this->em->hasFilters() ? $this->em->getFilters()->getHash() : '') .
-            '&firstResult=' . $this->firstResult . '&maxResult=' . $this->maxResults .
+            $firstAndMaxResult .
             '&hydrationMode=' . $this->hydrationMode . '&types=' . serialize($this->parsedTypes) . 'DOCTRINE_QUERY_CACHE_SALT',
         );
     }
@@ -679,4 +703,9 @@ public function __clone()
 
         $this->state = self::STATE_DIRTY;
     }
+
+    private function getSqlExecutor(): AbstractSqlExecutor
+    {
+        return $this->parse()->prepareSqlExecutor($this);
+    }
 }

src/Query/Exec/FinalizedSelectExecutor.php

@@ -0,0 +1,29 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Doctrine\ORM\Query\Exec;
+
+use Doctrine\DBAL\Connection;
+use Doctrine\DBAL\Result;
+
+/**
+ * SQL executor for a given, final, single SELECT SQL query
+ *
+ * @method string getSqlStatements()
+ */
+class FinalizedSelectExecutor extends AbstractSqlExecutor
+{
+    public function __construct(string $sql)
+    {
+        $this->sqlStatements = $sql;
+    }
+
+    /**
+     * {@inheritDoc}
+     */
+    public function execute(Connection $conn, array $params, array $types): Result
+    {
+        return $conn->executeQuery($this->getSqlStatements(), $params, $types, $this->queryCacheProfile);
+    }
+}

src/Query/Exec/PreparedExecutorFinalizer.php

@@ -0,0 +1,27 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Doctrine\ORM\Query\Exec;
+
+use Doctrine\ORM\Query;
+
+/**
+ * PreparedExecutorFinalizer is a wrapper for the SQL finalization
+ * phase that does nothing - it is constructed with the sql executor
+ * already.
+ */
+final class PreparedExecutorFinalizer implements SqlFinalizer
+{
+    private AbstractSqlExecutor $executor;
+
+    public function __construct(AbstractSqlExecutor $exeutor)
+    {
+        $this->executor = $exeutor;
+    }
+
+    public function createExecutor(Query $query): AbstractSqlExecutor
+    {
+        return $this->executor;
+    }
+}

src/Query/Exec/SingleSelectSqlFinalizer.php

@@ -0,0 +1,60 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Doctrine\ORM\Query\Exec;
+
+use Doctrine\DBAL\LockMode;
+use Doctrine\ORM\Query;
+use Doctrine\ORM\Query\QueryException;
+use Doctrine\ORM\Utility\LockSqlHelper;
+
+/**
+ * SingleSelectSqlFinalizer finalizes a given SQL query by applying
+ * the query's firstResult/maxResult values as well as extra read lock/write lock
+ * statements, both through the platform-specific methods.
+ *
+ * The resulting, "finalized" SQL is passed to a FinalizedSelectExecutor.
+ */
+class SingleSelectSqlFinalizer implements SqlFinalizer
+{
+    use LockSqlHelper;
+
+    public function __construct(private string $sql)
+    {
+    }
+
+    /**
+     * This method exists temporarily to support old SqlWalker interfaces.
+     *
+     * @internal
+     *
+     * @psalm-internal Doctrine\ORM
+     */
+    public function finalizeSql(Query $query): string
+    {
+        $platform = $query->getEntityManager()->getConnection()->getDatabasePlatform();
+
+        $sql = $platform->modifyLimitQuery($this->sql, $query->getMaxResults(), $query->getFirstResult());
+
+        $lockMode = $query->getHint(Query::HINT_LOCK_MODE) ?: LockMode::NONE;
+
+        if ($lockMode !== LockMode::NONE && $lockMode !== LockMode::OPTIMISTIC && $lockMode !== LockMode::PESSIMISTIC_READ && $lockMode !== LockMode::PESSIMISTIC_WRITE) {
+            throw QueryException::invalidLockMode();
+        }
+
+        if ($lockMode === LockMode::PESSIMISTIC_READ) {
+            $sql .= ' ' . $this->getReadLockSQL($platform);
+        } elseif ($lockMode === LockMode::PESSIMISTIC_WRITE) {
+            $sql .= ' ' . $this->getWriteLockSQL($platform);
+        }
+
+        return $sql;
+    }
+
+    /** @return FinalizedSelectExecutor */
+    public function createExecutor(Query $query): AbstractSqlExecutor
+    {
+        return new FinalizedSelectExecutor($this->finalizeSql($query));
+    }
+}

src/Query/Exec/SingleTableDeleteUpdateExecutor.php

@@ -14,8 +14,6 @@
  * that are mapped to a single table.
  *
  * @link        www.doctrine-project.org
- *
- * @todo This is exactly the same as SingleSelectExecutor. Unify in SingleStatementExecutor.
  */
 class SingleTableDeleteUpdateExecutor extends AbstractSqlExecutor
 {

src/Query/Exec/SqlFinalizer.php

@@ -0,0 +1,26 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Doctrine\ORM\Query\Exec;
+
+use Doctrine\ORM\Query;
+
+/**
+ * SqlFinalizers are created by OutputWalkers that traversed the DQL AST.
+ * The SqlFinalizer instance can be kept in the query cache and re-used
+ * at a later time.
+ *
+ * Once the SqlFinalizer has been created or retrieved from the query cache,
+ * it receives the Query object again in order to yield the AbstractSqlExecutor
+ * that will then be used to execute the query.
+ *
+ * The SqlFinalizer may assume that the DQL that was used to build the AST
+ * and run the OutputWalker (which created the SqlFinalizer) is equivalent to
+ * the query that will be passed to the createExecutor() method. Potential differences
+ * are the parameter values or firstResult/maxResult settings.
+ */
+interface SqlFinalizer
+{
+    public function createExecutor(Query $query): AbstractSqlExecutor;
+}

src/Query/OutputWalker.php

@@ -0,0 +1,27 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Doctrine\ORM\Query;
+
+use Doctrine\ORM\Query\Exec\SqlFinalizer;
+
+/**
+ * Interface for output walkers
+ *
+ * Output walkers, like tree walkers, can traverse the DQL AST to perform
+ * their purpose.
+ *
+ * The goal of an OutputWalker is to ultimately provide the SqlFinalizer
+ * which produces the final, executable SQL statement in a "finalization" phase.
+ *
+ * It must be possible to use the same SqlFinalizer for Queries with different
+ * firstResult/maxResult values. In other words, SQL produced by the
+ * output walker should not depend on those values, and any SQL generation/modification
+ * specific to them should happen in the finalizer's `\Doctrine\ORM\Query\Exec\SqlFinalizer::createExecutor()`
+ * method instead.
+ */
+interface OutputWalker
+{
+    public function getFinalizer(AST\DeleteStatement|AST\UpdateStatement|AST\SelectStatement $AST): SqlFinalizer;
+}