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

Cursor Pagination in conjunction with subqueries in order by #95

Open
MKodde opened this issue Nov 28, 2024 · 1 comment
Open

Cursor Pagination in conjunction with subqueries in order by #95

MKodde opened this issue Nov 28, 2024 · 1 comment
Labels
bug Something isn't working

Comments

@MKodde
Copy link

MKodde commented Nov 28, 2024

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?

@abbasudo
Copy link
Owner

abbasudo commented Nov 28, 2024

@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!

@abbasudo abbasudo added the bug Something isn't working label Nov 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants