From 0ef19b91d1f9f74d5cd8f884d65f396d701410e6 Mon Sep 17 00:00:00 2001 From: nio-dtp Date: Thu, 2 Jan 2025 22:04:06 +0100 Subject: [PATCH] query builders aggregator --- src/Connection.php | 10 ++ src/Query/QueryBuilder.php | 28 ++--- src/Query/QueryBuildersAggregator.php | 120 ++++++++++++++++++++ tests/Functional/Query/QueryBuilderTest.php | 41 ++++++- tests/Query/QueryBuilderTest.php | 6 + 5 files changed, 188 insertions(+), 17 deletions(-) create mode 100644 src/Query/QueryBuildersAggregator.php diff --git a/src/Connection.php b/src/Connection.php index e85d7d647f4..e8304d32a60 100644 --- a/src/Connection.php +++ b/src/Connection.php @@ -26,6 +26,7 @@ use Doctrine\DBAL\Platforms\AbstractPlatform; use Doctrine\DBAL\Query\Expression\ExpressionBuilder; use Doctrine\DBAL\Query\QueryBuilder; +use Doctrine\DBAL\Query\QueryBuildersAggregator; use Doctrine\DBAL\Schema\AbstractSchemaManager; use Doctrine\DBAL\Schema\DefaultSchemaManagerFactory; use Doctrine\DBAL\Schema\SchemaManagerFactory; @@ -104,6 +105,8 @@ class Connection implements ServerVersionProvider private SchemaManagerFactory $schemaManagerFactory; + private ?QueryBuildersAggregator $queryBuildersAggregator = null; + /** * Initializes a new instance of the Connection class. * @@ -126,6 +129,13 @@ public function __construct( $this->schemaManagerFactory = $this->_config->getSchemaManagerFactory() ?? new DefaultSchemaManagerFactory(); + + $this->queryBuildersAggregator = QueryBuildersAggregator::create(); + } + + public function getQueryBuildersAggregator(): QueryBuildersAggregator + { + return $this->queryBuildersAggregator; } /** diff --git a/src/Query/QueryBuilder.php b/src/Query/QueryBuilder.php index 3cbde0f035d..a8f867bc778 100644 --- a/src/Query/QueryBuilder.php +++ b/src/Query/QueryBuilder.php @@ -75,13 +75,6 @@ class QueryBuilder */ private ?int $maxResults = null; - /** - * The counter of bound parameters used with {@see bindValue). - * - * @var int<0, max> - */ - private int $boundCounter = 0; - /** * The SELECT parts of the query. * @@ -172,6 +165,7 @@ class QueryBuilder */ public function __construct(private readonly Connection $connection) { + $this->connection->getQueryBuildersAggregator()->register($this); } /** @@ -302,12 +296,18 @@ public function fetchFirstColumn(): array */ public function executeQuery(): Result { - return $this->connection->executeQuery( + $queryBuildersAggregate = $this->connection->getQueryBuildersAggregator(); + [$params, $types] = $queryBuildersAggregate->buildParametersAndTypes(); + + $result = $this->connection->executeQuery( $this->getSQL(), - $this->params, - $this->types, + $params, + $types, $this->resultCacheProfile, ); + $queryBuildersAggregate->toReset(); + + return $result; } /** @@ -1430,8 +1430,8 @@ public function createNamedParameter( ?string $placeHolder = null, ): string { if ($placeHolder === null) { - $this->boundCounter++; - $placeHolder = ':dcValue' . $this->boundCounter; + $this->connection->getQueryBuildersAggregator()->incrementBoundCounter(); + $placeHolder = ':dcValue' . $this->connection->getQueryBuildersAggregator()->getBoundCounter(); } $this->setParameter(substr($placeHolder, 1), $value, $type); @@ -1460,8 +1460,8 @@ public function createPositionalParameter( mixed $value, string|ParameterType|Type|ArrayParameterType $type = ParameterType::STRING, ): string { - $this->setParameter($this->boundCounter, $value, $type); - $this->boundCounter++; + $this->setParameter($this->connection->getQueryBuildersAggregator()->getBoundCounter(), $value, $type); + $this->connection->getQueryBuildersAggregator()->incrementBoundCounter(); return '?'; } diff --git a/src/Query/QueryBuildersAggregator.php b/src/Query/QueryBuildersAggregator.php new file mode 100644 index 00000000000..f3613c36198 --- /dev/null +++ b/src/Query/QueryBuildersAggregator.php @@ -0,0 +1,120 @@ + + */ + private int $boundCounter = 0; + + public static function create(): self + { + if (self::$instance === null) { + self::$instance = new QueryBuildersAggregator(); + } + + return self::$instance; + } + + private function __construct() + { + } + + public function toReset(): void + { + $this->toReset = true; + } + + public function register(QueryBuilder $queryBuilder): void + { + if ($this->toReset) { + $this->queryBuilders = []; + $this->boundCounter = 0; + $this->toReset = false; + } + + $this->queryBuilders[] = $queryBuilder; + } + + /** + * @return list|array + * + * @throws QueryException + */ + public function buildParametersAndTypes(): array + { + $parameters = []; + $types = []; + foreach ($this->queryBuilders as $queryBuilder) { + $this->rejectDuplicatedParameterNames($parameters, $queryBuilder->getParameters()); + $parameters = array_merge($parameters, $queryBuilder->getParameters()); + $types = array_merge($types, $queryBuilder->getParameterTypes()); + } + + return [$parameters, $types]; + } + + /** + * Guards against duplicated parameter names. + * + * @param list|array $params + * @param list|array $paramsToMerge + * + * @throws QueryException + */ + private function rejectDuplicatedParameterNames(array $params, array $paramsToMerge): void + { + if (count($params) === 0 || count($paramsToMerge) === 0) { + return; + } + + $paramKeys = array_filter(array_keys($params), 'is_string'); + $cteParamKeys = array_filter(array_keys($paramsToMerge), 'is_string'); + $duplicated = array_intersect($paramKeys, $cteParamKeys); + if (count($duplicated) > 0) { + throw new QueryException(sprintf( + 'Found duplicated parameter in query. The duplicated parameter names are: "%s".', + implode(', ', $duplicated), + )); + } + } + + public function getBoundCounter(): int + { + return $this->boundCounter; + } + + public function incrementBoundCounter(): void + { + $this->boundCounter++; + } +} diff --git a/tests/Functional/Query/QueryBuilderTest.php b/tests/Functional/Query/QueryBuilderTest.php index b54a9376804..9a412065265 100644 --- a/tests/Functional/Query/QueryBuilderTest.php +++ b/tests/Functional/Query/QueryBuilderTest.php @@ -256,17 +256,26 @@ public function testUnionAndAddUnionWithNamedParameterOnOuterInstanceAndOrderByD $subQueryBuilder1 = $this->connection->createQueryBuilder(); $subQueryBuilder1->select('id') ->from('for_update') - ->where($qb->expr()->eq('id', $qb->createNamedParameter(1, ParameterType::INTEGER))); + ->where($subQueryBuilder1->expr()->eq( + 'id', + $subQueryBuilder1->createNamedParameter(1, ParameterType::INTEGER), + )); $subQueryBuilder2 = $this->connection->createQueryBuilder(); $subQueryBuilder2->select('id') ->from('for_update') - ->where($qb->expr()->eq('id', $qb->createNamedParameter(2, ParameterType::INTEGER))); + ->where($subQueryBuilder2->expr()->eq( + 'id', + $subQueryBuilder2->createNamedParameter(2, ParameterType::INTEGER), + )); $subQueryBuilder3 = $this->connection->createQueryBuilder(); $subQueryBuilder3->select('id') ->from('for_update') - ->where($qb->expr()->eq('id', $qb->createNamedParameter(1, ParameterType::INTEGER))); + ->where($subQueryBuilder3->expr()->eq( + 'id', + $subQueryBuilder3->createNamedParameter(1, ParameterType::INTEGER), + )); $qb->union($subQueryBuilder1) ->addUnion($subQueryBuilder2, UnionType::DISTINCT) @@ -332,6 +341,32 @@ public function testUnionAndAddUnionWorksWithQueryBuilderPartsAndReturnsExpected self::assertSame($expectedRows, $qb->executeQuery()->fetchAllAssociative()); } + public function testSelectWithParametersBindOnDifferentQueryBuilders(): void + { + $expectedRows = $this->prepareExpectedRows([['id' => 1], ['id' => 2]]); + $qb = $this->connection->createQueryBuilder(); + + $queryBuilder1 = $this->connection->createQueryBuilder(); + $queryBuilder1->select('id') + ->from('for_update') + ->where('id = :id1') + ->setParameter('id1', 1); + + $queryBuilder2 = $this->connection->createQueryBuilder(); + $queryBuilder2->select('id') + ->from('for_update') + ->where('id = :id2') + ->setParameter('id2', 2); + + $qb->select('id') + ->from('for_update') + ->where($qb->expr()->in('id', $queryBuilder1->getSQL())) + ->orWhere($qb->expr()->in('id', $queryBuilder2->getSQL())) + ->orderBy('id', 'ASC'); + + self::assertSame($expectedRows, $qb->executeQuery()->fetchAllAssociative()); + } + /** * @param array> $rows * diff --git a/tests/Query/QueryBuilderTest.php b/tests/Query/QueryBuilderTest.php index 5cfa4cb6b01..724f9f02d74 100644 --- a/tests/Query/QueryBuilderTest.php +++ b/tests/Query/QueryBuilderTest.php @@ -11,6 +11,7 @@ use Doctrine\DBAL\Platforms\AbstractPlatform; use Doctrine\DBAL\Query\Expression\ExpressionBuilder; use Doctrine\DBAL\Query\QueryBuilder; +use Doctrine\DBAL\Query\QueryBuildersAggregator; use Doctrine\DBAL\Query\QueryException; use Doctrine\DBAL\Query\UnionType; use Doctrine\DBAL\Result; @@ -51,6 +52,11 @@ protected function setUp(): void $this->conn->method('getDatabasePlatform') ->willReturn($platform); + + $queryBuildersAggregate = QueryBuildersAggregator::create(); + $this->conn->method('getQueryBuildersAggregator') + ->willReturn($queryBuildersAggregate); + $queryBuildersAggregate->toReset(); } public function testSimpleSelectWithoutFrom(): void