-
Notifications
You must be signed in to change notification settings - Fork 11k
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
[10.x] Fix cursor paginate with union and column alias #50882
[10.x] Fix cursor paginate with union and column alias #50882
Conversation
Is there anything I can do to move this forward? |
@thijsvdanker not really. Just await a review by @taylorotwell. |
@thijsvdanker Do we still need this if #50884 PR get merged? |
@crynobone Yes we do. I've just ran the tests in this PR against the code in #50884 and they still fail for the same reason. |
@thijsvdanker I feel like we should prioritize integration tests for this instead of just mocking everything. |
@crynobone I agree. |
@crynobone I've added an integration test. |
@thijsvdanker would you remind resolving the conflicts? Does the integration test you added fail without your code changes? |
5593882
to
abdb064
Compare
@taylorotwell I've resolved the conflicts. |
When using cursorPaginate on a query with a union the where clause on the union uses the wrong column name.
Table Post:
Table News:
Should sort Post by the start_time column and News by the created_at column.
The current implementation sorts both Post and News by the start_time column.
This is similar to #50809 but with a dedicated test to confirm the issue.