Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Collections] Criteria::orderBy() ignores all but first ordering in multi-column sort #437

Open
tautis154 opened this issue Dec 10, 2024 · 1 comment

Comments

@tautis154
Copy link

Bug Report

Q A
Version 2.1.2

Summary

The Criteria::orderBy() method in Doctrine Collections only applies the first ordering pair from the provided array, completely ignoring subsequent orderings. This prevents proper multi-column sorting in Collection filtering.

Current behavior

When using Criteria::orderBy() with multiple sort criteria, only the first key/value pair in the array is applied to the sorting. Any additional orderings are ignored, and changing their direction (ASC/DESC) has no effect on the results.
For example, with two records having the same validFrom date:

When using Criteria::orderBy() with multiple sort criteria, only the first key/value pair in the array is applied to the sorting. Any additional orderings are ignored, and changing their direction (ASC/DESC) has no effect on the results.

For example, with two records having the same validFrom date:

// Records:
// id=5, validFrom='2024-12-10 16:04:11'
// id=6, validFrom='2024-12-10 16:04:11'

$collection->matching(
    Criteria::create()
        ->orderBy([
            'validFrom' => Criteria::DESC,
            'id' => Criteria::DESC  // This is ignored
        ])
);

// Returns id=5 first, despite having same validFrom and lower id

Even changing 'id' => Criteria::ASC has no effect on the results, demonstrating that the second ordering is completely ignored.

Expected behavior

All orderings in the array should be applied in sequence, matching SQL ORDER BY behavior:

  • Primary sorting should be by the first criteria
  • When values are equal for the first ordering, subsequent orderings should be used as tiebreakers
  • In the example above, id=6 should come first when sorting by validFrom DESC, id DESC

How to reproduce

use Doctrine\Common\Collections\Criteria;
use Doctrine\Common\Collections\ArrayCollection;

class PriceList
{
    private int $id;
    private \DateTimeImmutable $validFrom;
    
    public function __construct(int $id, \DateTimeImmutable $validFrom) 
    {
        $this->id = $id;
        $this->validFrom = $validFrom;
    }

    public function getId(): int
    {
        return $this->id;
    }

    public function getValidFrom(): \DateTimeImmutable
    {
        return $this->validFrom;
    }
}

// Test setup
$list1 = new PriceList(5, new \DateTimeImmutable('2024-12-10 16:04:11'));
$list2 = new PriceList(6, new \DateTimeImmutable('2024-12-10 16:04:11'));

$collection = new ArrayCollection([$list1, $list2]);

$result = $collection->matching(
    Criteria::create()
        ->orderBy([
            'validFrom' => Criteria::DESC,
            'id' => Criteria::DESC
        ])
        ->setMaxResults(1)
);

// Expected: First result has id=6
// Actual: First result has id=5

// Changing to ASC has no effect, proving second ordering is ignored
$result = $collection->matching(
    Criteria::create()
        ->orderBy([
            'validFrom' => Criteria::DESC,
            'id' => Criteria::ASC
        ])
        ->setMaxResults(1)
);

// Still returns id=5 first
@greg0ire
Copy link
Member

greg0ire commented Dec 10, 2024

I reproduce the issue with dates, but not with integers. The following test passes.

class MyTestCase
{
    public function testBug(): void
    {
        $list1 = new PriceList(5, 4);
        $list2 = new PriceList(6, 4);

        $collection = new ArrayCollection([$list1, $list2]);

        $result = $collection->matching(
            Criteria::create()
                ->orderBy([
                    'validFrom' => Order::Descending,
                    'id' => Order::Descending,
                ])
                ->setMaxResults(1),
        );

        self::assertSame(6, $result->first()->getId());

        $result = $collection->matching(
            Criteria::create()
                ->orderBy([
                    'validFrom' => Order::Descending,
                    'id' => Order::Ascending,
                ])
                ->setMaxResults(1),
        );
        self::assertSame(5, $result->first()->getId());
    }
}


class PriceList
{
    public function __construct(private int $id, private int $validFrom)
    {
    }

    public function getId(): int
    {
        return $this->id;
    }

    public function getValidFrom(): int
    {
        return $this->validFrom;
    }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants