You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Let me start by saying: thank you for this great library. I've been using it for the last couple of weeks and it prevented me from writing sort and filter logic for my project for the nth time!
The issue
I did however run into some complications while sorting on a join column. The Purity sorting component quite nicely created a sub query in the order by clause. Causing the dataset to be correctly sorted on the name field of a related table.
However, when going to the next page of my data set (utilizing the cursor pagination implementation shipped with illuminate). The resulting data set was not compatible with the cursor pagination, and a 500 error is thrown when the pagination UI components are created. Specifically, the call to the flip method errors, because the registered order by parts of the query builder contain non string arguments. (It contains the sub query expression).
To illustrate why the framework is not able to apply the cursor pagination: The exact issue that I ran into was already posted as an issue at the Laravel boards: laravel/framework#38447. They are not going to support this, as cursor pagination needs a result set of unique values, and including a sub query does not ensure that.
Proposed solution
The solution (in my opinion) would be to change the way the Sort component creates his query parts. I would suggest to create join's to the related tables instead of using a sub query.
Before I start putting a PR together, can you elaborate if this would be OK, and if this would be at all possible?
The text was updated successfully, but these errors were encountered:
@MKodde Thank you for your detailed feedback and for using Purity! We are happy to hear that it has been helpful for your projects. Your description of the issue and the proposed solution is well thought out. Generally, in SQL, sorting by relation is a pain. Purity didn't even have this function until version 2. Improving this functionality (while maintaining usage simplicity) is definitely our goal.
At the beginning of Purity, we tended to make use of Eloquent queries to take advantage of Eloquent's full compatibility and support any kind of queries for all databases. However, in #71, we had to switch to a more manual approach. I think this is the case for this issue too.
Regarding your proposed solution: I agree that replacing subqueries with joins in the Sort component could address the cursor pagination compatibility issue that you brought up. Using joins might also make the query more efficient. However, I'm a little concerned about maintaining compatibility for all kinds of queries. we should consider a solution for the cases where the model and its relation have the same column name and avoid SQL ambiguous errors.
You're absolutely welcome to make a PR for this! I'll be happy to review it and discuss any necessary adjustments. Thanks again for contributing to Purity and improving its functionality!
Thanks
Let me start by saying: thank you for this great library. I've been using it for the last couple of weeks and it prevented me from writing sort and filter logic for my project for the nth time!
The issue
I did however run into some complications while sorting on a join column. The Purity sorting component quite nicely created a sub query in the
order by
clause. Causing the dataset to be correctly sorted on the name field of a related table.However, when going to the next page of my data set (utilizing the cursor pagination implementation shipped with illuminate). The resulting data set was not compatible with the cursor pagination, and a 500 error is thrown when the pagination UI components are created. Specifically, the call to the
flip
method errors, because the registered order by parts of the query builder contain non string arguments. (It contains the sub query expression).To illustrate why the framework is not able to apply the cursor pagination: The exact issue that I ran into was already posted as an issue at the Laravel boards: laravel/framework#38447. They are not going to support this, as cursor pagination needs a result set of unique values, and including a sub query does not ensure that.
Proposed solution
The solution (in my opinion) would be to change the way the Sort component creates his query parts. I would suggest to create join's to the related tables instead of using a sub query.
Before I start putting a PR together, can you elaborate if this would be OK, and if this would be at all possible?
The text was updated successfully, but these errors were encountered: