mirror of
https://github.com/doctrine/orm.git
synced 2026-03-23 22:42:18 +01:00
PersistentCollection::matching() merges orderings incorrectly when association has default sorting #6315
Reference in New Issue
Block a user
Delete Branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Originally created by @paxuclus on GitHub (Sep 27, 2019).
Originally assigned to: @paxuclus on GitHub.
Bug Report
When PersistentCollection::matching() is called with orderings, they are not correctly merged together with the default sortings of an association.
Summary
When a relationship has a default sorting configured using
@ORM\OrderBy({"lastName" = "ASC", "firstName" = "ASC"}), the orderings are not correctly merged together with the orderings of the$criteriain PersistentCollection::matching().Current behavior
The orderings will be sorted like the array given to the
@ORM\OrderByannotation, leading to unexpected results when passing another orderBy to the$criteriaof PersistentCollection::matching().How to reproduce
@ORM\OrderBy({"lastName" = "ASC", "firstName" = "ASC"})(sort by at least 2 properties)$relationship->matching(Criteria::create()->orderBy(['firstName' => 'DESC']))on the PersistentCollection of that relationshipExpected behavior
The collection should be ordered by
firstName DESC, lastName ASC, but instead it will be ordered bylastName ASC, firstName DESC.Origin of the bug
Original Issue: #7767
The bug was introduced in this PR: https://github.com/doctrine/orm/pull/7766/files#diff-108586f774fc8acb02163ed709e05e86R675
Using
$criteria->orderBy(array_merge($criteria->getOrderings(), $this->association['orderBy'] ?? [], $criteria->getOrderings()));there would fix the ordering.@SenseException commented on GitHub (Sep 29, 2019):
Thank you for reporting this issue. If you like, you can create a PR with a fix and test(s) to this issue.
As far as I understand you describe a regression between two versions/commits for the orderBy behavior. But I'm not sure if there's an intended or defined behavior for a use case like this.
When I read
$relationship->matching(Criteria::create()->orderBy(['firstName' => 'DESC']))shouldn't it override theOrderByannotation instead of mixing it? I would expect thatfirstName DESCwill be the only order in here.If you want to create a PR, you should aim to restore the previous known behavior you described, but I wonder if we should change it for the next major release.
@paxuclus commented on GitHub (Oct 1, 2019):
Hey @SenseException,
thinking about this again I agree with you that "orderBy()" should override the default sorting.
Previously (before #7766 was merged), the "->matching()" would completely ignore the
OrderByannotation, even if there were no orderings in the$criteria.I think the PersistentCollection should only use the
OrderByannotation if the$criteriadoes not contain any orderings.I will try to create a PR this week. 👍
@lcobucci commented on GitHub (Oct 2, 2019):
Handled by #7850