DateTime sortable

[LaurentMarquet]

Hi,
according to @clytemnestra in KnpLabs/knp-components#129, it seems that it's NOT possible to sort based on a DateTime ?

Because, I am trying to paginate with a DateTime object but I receive the following error Warning: strtolower() expects parameter 1 to be string, object given, full log is CRITICAL - Uncaught PHP Exception Symfony\Component\Debug\Exception\ContextErrorException: "Warning: strtolower() expects parameter 1 to be string, object given" at /.../vendor/knplabs/knp-components/src/Knp/Component/Pager/Event/Subscriber/Sortable/ArraySubscriber.php line 61

I give a.dateTransaction to the query which is the name of the field in the entity.

So, is it really impossible to sort on a DateTime ?

[ricohumme]

👍 I also have this issue

[ricohumme]

Found out that is only occurs when you pass an already hydrated array with objects to the sortable.
If you pass the querybuilder to the paginate function, the sorting will already happen on the the database instead of the array with objects.

[LaurentMarquet]

Ah, thanks for your answer. Effectively, I give an hydrated array via the following :

public function localeViewTransactionsAction(Request $request)
{
        $em = $this->getDoctrine()->getManager();
        $repository = $em->getRepository('AppBundle:Transaction');
        $transactions = $repository->findByUserId($this->getUser()->getId(), array('creation' => 'desc'));

        $paginator  = $this->get('knp_paginator');
        $pagination = $paginator->paginate(
            $transactions,
            $request->query->getInt('p', 1),
            25
        );

        return $this->render('pages/transactions.html.twig', array(
            'transactions' => $pagination,
            ));
}

So, for me, giving the querybuilder means re-build what I can have by using the repository, which is, for me, and if I understand well, NOT what should be done as "good practice".

[ricohumme]

Well that depends on how you define the return type in your repository function.

public function findByUserId($id, array $orderBy = array(), $hydrate = false)
{
	$qb = $this->createQueryBuilder('t')
		->join('t.user', 'u', 'WITH', 'u.id = :id')->setParameter('id', $id);
	if ($orderBy) 
		$qb->addOrderBy($orderBy);
	
	if ($hydrate)
		return $qb->getQuery()->getResult();
	
	return $qb;
}

Would be an approach without giving up on repository functionality

[LaurentMarquet]

Thanks for your code!
In fact, I try to not write queries, except for specific things (search criterias, etc.), but try to use what's "built-in" with "defaults" functions provided by Doctrine. That's what I mean by re-building (overidding) the query.
In this case the query should (must) be re-written to be able to sort on datetime, which is, for me, not a "good" idea, as I re-do what is available by default.
I don't know if i explain it well, if so, what do you think of it?

[ricohumme]

I understand your view of code.
The issue in this case is that the \DateTime object doesn't implement __toString. This is the source of the current error you are experiencing.
So in my point of view there are a few paths one can take to address this situation:

1. Use the implementation I earlier provided.
2. Implement your own \DateTime functionality where you do implement a __toString method. Don't know if and how Doctrine will cope with this, but you will have to modify the getters of your class as well to achieve this. It won't make your code any cleaner.
3. Rewrite the code of the paginator to take \DateTimeInterface object into account and ommit the strtolower option. (I tried this and ran into the own ArraySubscriber of Knp after mine was done experiencing the issue again). On a second note, if the \DateTimeInterface is taken into account, what will happen if objects of other types are treated differently. It could end up in an object racism.

As I see it, option 1 has the least impact.
How would you approach this?

[assada]

Same problem with sorting by DateTime field type.

PHP Version:
7.1.4 -1+deb.sury.org~xenial+1

Symfony version:
3.3.0-DEV af4703f

KnpPaginatorBundle version:
2.5.4

$users = $this->getDoctrine()->getRepository('DashboardBundle:User')->findAll();

        $paginator  = $this->get('knp_paginator');
        $pagination = $paginator->paginate(
            $users,
            $request->query->getInt('page', 1),
            10
        );

REQUEST:

GET /app_dev.php/dashboard/system/users?sort=u.lastLogin&direction=asc&page=1

RESPONSE:

Warning: strtolower() expects parameter 1 to be string, object given

[akerouanton]

Inddeed, we can't sort on \DateTime. I think this PR on knp-components should solve your issue. I ping you when it's ready.

[ricohumme]

Still, why would you want to hydrate everything from the database and then put it in the paginator. It might work for small tables but those can get large too you know.

The only reason is when you would use a js datatable of some sort for this setup to be efficient. In other cases, let the paginator build your query and apply the offset and size. Then, datetime sorting won't be an issue anymore aswell.

[akerouanton]

Still, why would you want to hydrate everything from the database and then put it in the paginator. It might work for small tables but those can get large too you know.

Because the paginator paginates everything, wherever it comes from a database or not. Thus, I don't understand this point: I (and other maintainers) never recommended to paginates only array. And I can find at least another example where array paginator is useful: paginating items coming from a SOAP response (you can forget about iterators and generators).

Also you noted yourself that sorting on \DateTime isn't a problem when the pagination is done on the query builder. Thus I'm talking only about ArraySubscriber because the issue is only confirmed for it...

[ricohumme]

I'm not saying it shouldn't be supported. Only when working with large datasets, hydrating everything (mostly into entity objects) is not cost efficient nor time efficient for your application and heavy on the database. But I think this more of a "how do you implement your code" topic :)

[LaurentMarquet]

@ricohumme As I said, I agree with you, but it forces us to re-write what's "built-in", but I think you're right and I should work in this direction.
With your last comment, another question came into my mind, that maybe you can give an answer, by using

metadata_cache_driver: apcu
query_cache_driver: apcu
result_cache_driver: apcu

It should lowerizes the load on the server, isn't it?

[ricohumme]

It should. However, I'm not to keen myself on this topic but maybe this could help a little https://stackoverflow.com/questions/38093196/symfony-3-apcu-php7

[akerouanton]

@Laurent3170 In your case I agree with @ricohumme you should rely on doctrine paginator rather than paginating the whole transaction set. It will be more efficient. Beside this point, could you update knp-components just to confirm it works now the aforementioned PR has been merged?

[LaurentMarquet]

@NiR- Yes I confirm that updating files in KnpLabs/knp-components#159 makes the array sortable on date!
@ricohumme and @NiR- Thanks for having made me understand that I should use the query instead of the result. It's written in the README.md, but it's easier, when beginning, to use the result rather than building the query...

Concerning this issue, should it be closed? As till the PR is not present in a superior version of knp-components, the sortable array won't work.

[akerouanton]

@Laurent3170 We'll release a new version of knp-components soon. So I consider this PR solved, since it wasn't really related to this repo, the encountered bug as been solved and you choose the most efficient way of sorting for your use case :)

[LaurentMarquet]

And thanks for your work for having made the array sortable on a date!