Fix DbalCollection::getQueryBuilder()

composer.json

@@ -24,7 +24,7 @@
 		"ext-ctype": "*",
 		"nette/caching": "~3.2 || ~3.1.3",
 		"nette/utils": "~3.0 || ~4.0",
-		"nextras/dbal": "^5.0.3",
+		"nextras/dbal": "dev-js/duplicated-joins",
 		"phpstan/phpdoc-parser": "^1.33.0 || ^2.0.0"
 	},
 	"require-dev": {

src/Bridges/NetteDI/OrmExtension.php

@@ -177,6 +177,7 @@ protected function setupMetadataStorage(array $entityClassMap): void
 
 
 	/**
+	 * @param class-string<IModel> $modelClass
 	 * @param array{
 	 *     array<class-string<IRepository<IEntity>>, true>,
 	 *     array<string, class-string<IRepository<IEntity>>>,

src/Bridges/NetteDI/PhpDocRepositoryFinder.php

@@ -117,6 +117,9 @@ protected function setupMapperService(string $repositoryName, string $repository
 	}
 
 
+	/**
+	 * @param class-string<IRepository<IEntity>> $repositoryClass
+	 */
 	protected function setupRepositoryService(string $repositoryName, string $repositoryClass): void
 	{
 		$serviceName = $this->extension->prefix('repositories.' . $repositoryName);

src/Collection/DbalCollection.php

@@ -38,14 +38,14 @@ class DbalCollection implements ICollection
 	/** @var Iterator<E>|null */
 	protected Iterator|null $fetchIterator = null;
 
-	/** @var array<mixed> FindBy expressions for deferred processing */
-	protected array $filtering = [];
+	/** @var array<string, Fqn> GROUP BY columns contributed by findBy() filtering. */
+	private array $filterGroupByColumns = [];
 
-	/** @var array<array{DbalExpressionResult, string}> OrderBy expression result & sorting direction */
-	protected array $ordering = [];
+	/** @var array<string, Fqn> GROUP BY columns contributed by orderBy() expressions. */
+	private array $orderGroupByColumns = [];
 
-	/** @var array{int, int|null}|null */
-	protected array|null $limitBy = null;
+	/** @var array<string, Fqn> Columns referenced by orderBy() expressions; required in GROUP BY once grouping is active. */
+	private array $orderColumns = [];
 
 	protected DbalQueryBuilderHelper|null $helper = null;
 
@@ -54,7 +54,8 @@ class DbalCollection implements ICollection
 	protected ?int $resultCount = null;
 	protected bool $entityFetchEventTriggered = false;
 
-	protected QueryBuilder|null $queryBuilderCache = null;
+	/** @var array<string, true> */
+	private array $mysqlGroupByWorkaroundApplied = [];
 
 
 	/**
@@ -100,30 +101,41 @@ public function getByIdChecked($id): IEntity
 	public function findBy(array $conds): ICollection
 	{
 		$collection = clone $this;
-		$collection->filtering[] = $conds;
+		$expression = $collection->getHelper()->processExpression(
+			builder: $collection->queryBuilder,
+			expression: $conds,
+			aggregator: null,
+		);
+		$finalContext = $expression->havingExpression === null
+			? ExpressionContext::FilterAnd
+			: ExpressionContext::FilterAndWithHavingClause;
+		$expression = $expression->collect($finalContext);
+
+		$collection->applyExpressionJoins($expression);
+		if ($expression->expression !== null && $expression->args !== []) {
+			$collection->queryBuilder->andWhere($expression->expression, ...$expression->args);
+		}
+		if ($expression->havingExpression !== null && $expression->havingArgs !== []) {
+			$collection->queryBuilder->andHaving($expression->havingExpression, ...$expression->havingArgs);
+		}
+		$collection->filterGroupByColumns = $collection->mergeColumns($collection->filterGroupByColumns, $expression->groupBy);
+		$collection->rebuildGroupBy();
 		return $collection;
 	}
 
 
 	public function orderBy($expression, string $direction = ICollection::ASC): ICollection
 	{
 		$collection = clone $this;
-		$helper = $collection->getHelper();
 		if (is_array($expression) && !isset($expression[0])) {
 			/** @var array<string, string> $expression */
 			$expression = $expression; // no-op for PHPStan
 
 			foreach ($expression as $subExpression => $subDirection) {
-				$collection->ordering[] = [
-					$helper->processExpression($collection->queryBuilder, $subExpression, null),
-					$subDirection,
-				];
+				$collection->addOrderByExpression($subExpression, $subDirection);
 			}
 		} else {
-			$collection->ordering[] = [
-				$helper->processExpression($collection->queryBuilder, $expression, null),
-				$direction,
-			];
+			$collection->addOrderByExpression($expression, $direction);
 		}
 		return $collection;
 	}
@@ -132,18 +144,18 @@ public function orderBy($expression, string $direction = ICollection::ASC): ICol
 	public function resetOrderBy(): ICollection
 	{
 		$collection = clone $this;
-		$collection->ordering = [];
-		// reset default ordering from mapper
-		$collection->queryBuilder = clone $collection->queryBuilder;
+		$collection->orderGroupByColumns = [];
+		$collection->orderColumns = [];
 		$collection->queryBuilder->orderBy(null);
+		$collection->rebuildGroupBy();
 		return $collection;
 	}
 
 
 	public function limitBy(int $limit, int|null $offset = null): ICollection
 	{
 		$collection = clone $this;
-		$collection->limitBy = [$limit, $offset];
+		$collection->queryBuilder->limitBy($limit, $offset);
 		return $collection;
 	}
 
@@ -281,7 +293,7 @@ public function subscribeOnEntityFetch(callable $callback): void
 
 	public function __clone()
 	{
-		$this->queryBuilderCache = null;
+		$this->queryBuilder = clone $this->queryBuilder;
 		$this->result = null;
 		$this->resultCount = null;
 		$this->fetchIterator = null;
@@ -290,76 +302,19 @@ public function __clone()
 
 
 	/**
-	 * @internal
+	 * Returns the live, mutable query builder backing this collection.
+	 *
+	 * Callers may further mutate the returned builder, therefore any already fetched result is discarded so that the
+	 * next fetch reflects the changes. Internal callers intentionally read {@see $queryBuilder} directly to avoid
+	 * invalidating an in-progress iteration.
 	 */
 	public function getQueryBuilder(): QueryBuilder
 	{
-		if ($this->queryBuilderCache !== null) {
-			return $this->queryBuilderCache;
-		}
-
-		$joins = [];
-		$groupBy = [];
-		$queryBuilder = clone $this->queryBuilder;
-		$helper = $this->getHelper();
-
-		$filtering = $this->filtering;
-		if (count($filtering) > 0) {
-			array_unshift($filtering, ICollection::AND);
-			$expression = $helper->processExpression(
-				builder: $queryBuilder,
-				expression: $filtering,
-				aggregator: null,
-			);
-			$finalContext = $expression->havingExpression === null
-				? ExpressionContext::FilterAnd
-				: ExpressionContext::FilterAndWithHavingClause;
-			$expression = $expression->collect($finalContext);
-			$joins = $expression->joins;
-			$groupBy = $expression->groupBy;
-			if ($expression->expression !== null && $expression->args !== []) {
-				$queryBuilder->andWhere($expression->expression, ...$expression->args);
-			}
-			if ($expression->havingExpression !== null && $expression->havingArgs !== []) {
-				$queryBuilder->andHaving($expression->havingExpression, ...$expression->havingArgs);
-			}
-			if ($this->mapper->getDatabasePlatform()->getName() === MySqlPlatform::NAME) {
-				$this->applyGroupByWithSameNamedColumnsWorkaround($queryBuilder, $groupBy);
-			}
-		}
-
-		foreach ($this->ordering as [$expression, $direction]) {
-			$joins = array_merge($joins, $expression->joins);
-			$groupBy = array_merge($groupBy, $expression->groupBy);
-			$orderingExpression = $helper->processOrderDirection($expression, $direction);
-			$queryBuilder->addOrderBy('%ex', $orderingExpression);
-		}
-
-		if ($this->limitBy !== null) {
-			$queryBuilder->limitBy($this->limitBy[0], $this->limitBy[1]);
-		}
-
-		$mergedJoins = $helper->mergeJoins('%and', $joins);
-		foreach ($mergedJoins as $join) {
-			$join->applyJoin($queryBuilder);
-		}
-
-		if (count($groupBy) > 0) {
-			foreach ($this->ordering as [$expression]) {
-				$groupBy = array_merge($groupBy, $expression->columns);
-			}
-		}
-
-		if (count($groupBy) > 0) {
-			$unique = [];
-			foreach ($groupBy as $groupByFqn) {
-				$unique[$groupByFqn->getUnescaped()] = $groupByFqn;
-			}
-			$queryBuilder->groupBy('%column[]', array_values($unique));
-		}
-
-		$this->queryBuilderCache = $queryBuilder;
-		return $queryBuilder;
+		$this->result = null;
+		$this->resultCount = null;
+		$this->fetchIterator = null;
+		$this->entityFetchEventTriggered = false;
+		return $this->queryBuilder;
 	}
 
 
@@ -369,7 +324,7 @@ protected function getIteratorCount(): int
 			return $this->resultCount;
 		}
 
-		$builder = clone $this->getQueryBuilder();
+		$builder = clone $this->queryBuilder;
 
 		if ($this->connection->getPlatform()->getName() === SqlServerPlatform::NAME) {
 			if (!$builder->hasLimitOffsetClause()) {
@@ -397,7 +352,7 @@ protected function getIteratorCount(): int
 
 	protected function execute(): void
 	{
-		$result = $this->connection->queryByQueryBuilder($this->getQueryBuilder());
+		$result = $this->connection->queryByQueryBuilder($this->queryBuilder);
 
 		$this->result = [];
 		while (($data = $result->fetch()) !== null) {
@@ -419,6 +374,75 @@ protected function getHelper(): DbalQueryBuilderHelper
 	}
 
 
+	/**
+	 * @param array<mixed>|string $expression
+	 */
+	private function addOrderByExpression(string|array $expression, string $direction): void
+	{
+		$expressionResult = $this->getHelper()->processExpression($this->queryBuilder, $expression, null);
+		$this->applyExpressionJoins($expressionResult);
+
+		$this->orderGroupByColumns = $this->mergeColumns($this->orderGroupByColumns, $expressionResult->groupBy);
+		$this->orderColumns = $this->mergeColumns($this->orderColumns, $expressionResult->columns);
+		$this->rebuildGroupBy();
+
+		$orderByExpression = $this->getHelper()->processOrderDirection($expressionResult, $direction);
+		$this->queryBuilder->addOrderBy('%ex', $orderByExpression);
+	}
+
+
+	/**
+	 * Merges the given columns into a keyed map (deduplicated by their unescaped Fqn), preserving order.
+	 *
+	 * @param array<string, Fqn> $target
+	 * @param list<Fqn> $columns
+	 * @return array<string, Fqn>
+	 */
+	private function mergeColumns(array $target, array $columns): array
+	{
+		foreach ($columns as $fqn) {
+			$target[$fqn->getUnescaped()] ??= $fqn;
+		}
+		return $target;
+	}
+
+
+	/**
+	 * Rebuilds the whole GROUP BY clause from the tracked filtering and ordering columns.
+	 *
+	 * The clause is recomputed instead of mutated incrementally so that it does not depend on the order in which
+	 * findBy()/orderBy() were called, and so that resetOrderBy() can drop the ordering-induced columns again.
+	 * Order-by columns are added only when grouping is otherwise active, matching the SQL grouping requirements.
+	 */
+	private function rebuildGroupBy(): void
+	{
+		$groupBy = $this->filterGroupByColumns + $this->orderGroupByColumns;
+		if (count($groupBy) > 0) {
+			$groupBy += $this->orderColumns;
+		}
+
+		if (count($groupBy) === 0) {
+			$this->queryBuilder->groupBy(null);
+			return;
+		}
+
+		$this->queryBuilder->groupBy('%column[]', array_values($groupBy));
+
+		if ($this->mapper->getDatabasePlatform()->getName() === MySqlPlatform::NAME) {
+			$this->applyGroupByWithSameNamedColumnsWorkaround($this->queryBuilder, array_values($groupBy));
+		}
+	}
+
+
+	private function applyExpressionJoins(DbalExpressionResult $expression): void
+	{
+		$mergedJoins = $this->getHelper()->mergeJoins('%and', $expression->joins);
+		foreach ($mergedJoins as $join) {
+			$join->applyJoin($this->queryBuilder);
+		}
+	}
+
+
 	/**
 	 * Apply workaround for MySQL that is not able to properly resolve columns when there are more same-named
 	 * columns in the GROUP BY clause, even though they are properly referenced to their tables. Orm workarounds
@@ -430,17 +454,18 @@ private function applyGroupByWithSameNamedColumnsWorkaround(QueryBuilder $queryB
 	{
 		$map = [];
 		foreach ($groupBy as $fqn) {
-			if (!isset($map[$fqn->name])) {
-				$map[$fqn->name] = [$fqn];
-			} else {
-				$map[$fqn->name][] = $fqn;
-			}
+			$map[$fqn->name][$fqn->getUnescaped()] = $fqn;
 		}
-		$i = 0;
+
 		foreach ($map as $fqns) {
 			if (count($fqns) > 1) {
-				foreach ($fqns as $fqn) {
-					$queryBuilder->addSelect("%column AS __nextras_fix_" . $i++, $fqn); // @phpstan-ignore-line
+				foreach ($fqns as $key => $fqn) {
+					if (isset($this->mysqlGroupByWorkaroundApplied[$key])) {
+						continue;
+					}
+
+					$queryBuilder->addSelect("%column AS __nextras_fix_" . count($this->mysqlGroupByWorkaroundApplied), $fqn); // @phpstan-ignore-line
+					$this->mysqlGroupByWorkaroundApplied[$key] = true;
 				}
 			}
 		}

src/Collection/Functions/Result/DbalTableJoin.php

@@ -5,6 +5,8 @@
 
 use Nextras\Dbal\Platforms\Data\Fqn;
 use Nextras\Dbal\QueryBuilder\QueryBuilder;
+use function md5;
+use function serialize;
 
 
 /**
@@ -41,11 +43,15 @@ public function __construct(
 
 	public function applyJoin(QueryBuilder $queryBuilder): void
 	{
-		$queryBuilder->joinLeft(
+		$queryBuilder->joinOnce(
+			'LEFT',
 			"$this->toExpression AS [$this->toAlias]",
 			$this->onExpression,
-			...$this->toArgs,
-			...$this->onArgs,
+			[
+				...$this->toArgs,
+				...$this->onArgs,
+			],
+			hashSuffix: md5(serialize([$this->toArgs, $this->onArgs])),
 		);
 	}
 }

src/Mapper/Dbal/RelationshipMapperManyHasMany.php

@@ -123,7 +123,7 @@ private function fetchByTwoPassStrategy(QueryBuilder $builder, array $values): M
 		$hasOrderBy = $builder->getClause('order')[0] !== null;
 
 		$builder = clone $builder;
-		$builder->joinLeft(
+		$builder->addLeftJoin(
 			"%table AS %table",
 			'%column = %column',
 			// args
@@ -232,7 +232,7 @@ private function fetchCounts(QueryBuilder $builder, array $values): array
 		$targetTable = DbalQueryBuilderHelper::getAlias($this->joinTable);
 
 		$builder = clone $builder;
-		$builder->joinLeft(
+		$builder->addLeftJoin(
 			'%table AS %table',
 			'%column = %column',
 			// args