do not ignore parameters from query builder union parts

doctrine · Jan 5, 2025 · 9e6c631 · 9e6c631
1 parent 4d8badd
commit 9e6c631
Show file tree

Hide file tree

Showing 3 changed files with 190 additions and 7 deletions.
diff --git a/docs/en/reference/query-builder.rst b/docs/en/reference/query-builder.rst
@@ -324,6 +324,9 @@ or QueryBuilder instances to one of the following methods:
 * ``union(string|QueryBuilder $part)``
 * ``addUnion(string|QueryBuilder $part, UnionType $type = UnionType::DISTINCT)``
 
+If you pass a QueryBuilder instance, you can set parameters on it.
+But you cannot use same parameter names in different QueryBuilder instances.
+
 .. code-block:: php
 
     <?php
@@ -358,10 +361,14 @@ or QueryBuilder instances to one of the following methods:
 
     $subQueryBuilder1
         ->select('id AS field')
-        ->from('a_table');
+        ->from('a_table')
+        ->where('id > :minId')
+        ->setParameter('minId', 12);
     $subQueryBuilder2
         ->select('id AS field')
-        ->from('a_table');
+        ->from('a_table')
+        ->where('id < :maxId')
+        ->setParameter('maxId', 133);
     $queryBuilder
         ->union($subQueryBuilder1)
         ->addUnion($subQueryBuilder2,UnionType::ALL)

diff --git a/src/Query/QueryBuilder.php b/src/Query/QueryBuilder.php
@@ -312,14 +312,77 @@ public function fetchFirstColumn(): array
      */
     public function executeQuery(): Result
     {
+        [$params, $types] = $this->buildParametersAndTypes();
+
         return $this->connection->executeQuery(
             $this->getSQL(),
-            $this->params,
-            $this->types,
+            $params,
+            $types,
             $this->resultCacheProfile,
         );
     }
 
+    /**
+     * Build then return parameters and types for the query.
+     *
+     * @return array{
+     *     list<mixed>|array<string, mixed>,
+     *     WrapperParameterTypeArray,
+     * } The parameters and types for the query.
+     */
+    private function buildParametersAndTypes(): array
+    {
+        $partParams = $partParamTypes = [];
+
+        foreach ($this->unionParts as $part) {
+            if (! $part->query instanceof self || count($part->query->params) === 0) {
+                continue;
+            }
+
+            $this->guardDuplicatedParameterNames($partParams, $part->query->params);
+
+            $partParams     = array_merge($partParams, $part->query->params);
+            $partParamTypes = array_merge($partParamTypes, $part->query->types);
+        }
+
+        if (count($partParams) === 0) {
+            return [$this->params, $this->types];
+        }
+
+        $this->guardDuplicatedParameterNames($partParams, $this->params);
+
+        return [
+            array_merge($partParams, $this->params),
+            array_merge($partParamTypes, $this->types),
+        ];
+    }
+
+    /**
+     * Guards against duplicated parameter names.
+     *
+     * @param list<mixed>|array<string, mixed> $params
+     * @param list<mixed>|array<string, mixed> $paramsToMerge
+     *
+     * @throws QueryException
+     */
+    private function guardDuplicatedParameterNames(array $params, array $paramsToMerge): void
+    {
+        if (count($params) === 0 || count($paramsToMerge) === 0) {
+            return;
+        }
+
+        $paramsKeys        = array_filter(array_keys($params), 'is_string');
+        $paramsToMergeKeys = array_filter(array_keys($paramsToMerge), 'is_string');
+
+        $duplicates = array_intersect($paramsKeys, $paramsToMergeKeys);
+        if (count($duplicates) > 0) {
+            throw new QueryException(sprintf(
+                'Found duplicated parameter in query. The duplicated parameter names are: "%s".',
+                implode(', ', $duplicates),
+            ));
+        }
+    }
+
     /**
      * Executes an SQL statement and returns the number of affected rows.
      *

diff --git a/tests/Functional/Query/QueryBuilderTest.php b/tests/Functional/Query/QueryBuilderTest.php
@@ -215,9 +215,6 @@ public function testUnionWithLimitAndOffsetClauseReturnsExpectedResult(): void
     {
         $expectedRows = $this->prepareExpectedRows([['field_one' => 2]]);
         $platform     = $this->connection->getDatabasePlatform();
-        $plainSelect1 = $platform->getDummySelectSQL('1 as field_one');
-        $plainSelect2 = $platform->getDummySelectSQL('2 as field_one');
-        $plainSelect3 = $platform->getDummySelectSQL('1 as field_one');
         $qb           = $this->connection->createQueryBuilder();
         $qb->union($platform->getDummySelectSQL('1 as field_one'))
             ->addUnion($platform->getDummySelectSQL('2 as field_one'), UnionType::DISTINCT)
@@ -535,6 +532,122 @@ public function testPlatformDoesNotSupportCTE(): void
         $qb->executeQuery();
     }
 
+    public function testUnionAndAddUnionWorksWithBindingNamedParametersToQueryBuilderParts(): void
+    {
+        $expectedRows = $this->prepareExpectedRows([['id' => 2], ['id' => 1], ['id' => 1]]);
+        $qb           = $this->connection->createQueryBuilder();
+
+        $subQueryBuilder1 = $this->connection->createQueryBuilder();
+        $subQueryBuilder1->select('id')
+            ->from('for_update')
+            ->where('id = :id1')
+            ->setParameter('id1', 1, ParameterType::INTEGER);
+
+        $subQueryBuilder2 = $this->connection->createQueryBuilder();
+        $subQueryBuilder2->select('id')
+            ->from('for_update')
+            ->where('id = :id2')
+            ->setParameter('id2', 2, ParameterType::INTEGER);
+
+        $subQueryBuilder3 = $this->connection->createQueryBuilder();
+        $subQueryBuilder3->select('id')
+            ->from('for_update')
+            ->where('id = :id3')
+            ->setParameter('id3', 1, ParameterType::INTEGER);
+
+        $qb->union($subQueryBuilder1)
+            ->addUnion($subQueryBuilder2)
+            ->addUnion($subQueryBuilder3, UnionType::ALL)
+            ->orderBy('id', 'DESC');
+
+        self::assertSame($expectedRows, $qb->executeQuery()->fetchAllAssociative());
+    }
+
+    public function testUnionAndAddUnionWorksWithBindingPositionalParametersToQueryBuilderParts(): void
+    {
+        $expectedRows = $this->prepareExpectedRows([['id' => 1], ['id' => 1], ['id' => 2]]);
+        $qb           = $this->connection->createQueryBuilder();
+
+        $subQueryBuilder1 = $this->connection->createQueryBuilder();
+        $subQueryBuilder1->select('id')
+            ->from('for_update')
+            ->where('id = ?')
+            ->setParameter(0, 1, ParameterType::INTEGER);
+
+        $subQueryBuilder2 = $this->connection->createQueryBuilder();
+        $subQueryBuilder2->select('id')
+            ->from('for_update')
+            ->where($subQueryBuilder2->expr()->eq(
+                'id',
+                $subQueryBuilder2->createPositionalParameter(2, ParameterType::INTEGER),
+            ));
+
+        $subQueryBuilder3 = $this->connection->createQueryBuilder();
+        $subQueryBuilder3->select('id')
+            ->from('for_update')
+            ->where('id = ?')
+            ->setParameter(0, 1, ParameterType::INTEGER);
+
+        $qb->union($subQueryBuilder1)
+            ->addUnion($subQueryBuilder2)
+            ->addUnion($subQueryBuilder3, UnionType::ALL)
+            ->orderBy('id', 'ASC');
+
+        self::assertSame($expectedRows, $qb->executeQuery()->fetchAllAssociative());
+    }
+
+    public function testUnionAndAddUnionThrowsExceptionWithDuplicatedParametersNames(): void
+    {
+        $qb = $this->connection->createQueryBuilder();
+
+        $subQueryBuilder1 = $this->connection->createQueryBuilder();
+        $subQueryBuilder1->select('id')
+            ->from('for_update')
+            ->where('id = :id')
+            ->setParameter('id', 1, ParameterType::INTEGER);
+
+        $subQueryBuilder2 = $this->connection->createQueryBuilder();
+        $subQueryBuilder2->select('id')
+            ->from('for_update')
+            ->where('id = :id')
+            ->setParameter('id', 2, ParameterType::INTEGER);
+
+        $qb->union($subQueryBuilder1)
+            ->addUnion($subQueryBuilder2);
+
+        self::expectExceptionMessage('Found duplicated parameter in query. The duplicated parameter names are: "id".');
+        $qb->executeQuery();
+    }
+
+    public function testUnionAndAddUnionThrowsExceptionWithDuplicatedCreatedParametersNames(): void
+    {
+        $qb = $this->connection->createQueryBuilder();
+
+        $subQueryBuilder1 = $this->connection->createQueryBuilder();
+        $subQueryBuilder1->select('id')
+            ->from('for_update')
+            ->where($subQueryBuilder1->expr()->eq(
+                'id',
+                $subQueryBuilder1->createNamedParameter(1, ParameterType::INTEGER),
+            ));
+
+        $subQueryBuilder2 = $this->connection->createQueryBuilder();
+        $subQueryBuilder2->select('id')
+            ->from('for_update')
+            ->where($subQueryBuilder2->expr()->eq(
+                'id',
+                $subQueryBuilder2->createNamedParameter(2, ParameterType::INTEGER),
+            ));
+
+        $qb->union($subQueryBuilder1)
+            ->addUnion($subQueryBuilder2);
+
+        self::expectExceptionMessage(
+            'Found duplicated parameter in query. The duplicated parameter names are: "dcValue1".',
+        );
+        $qb->executeQuery();
+    }
+
     /**
      * @param array<array<string, int>> $rows
      *