-
Notifications
You must be signed in to change notification settings - Fork 18
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
Support primary keys that aren't named id
#154
Comments
:id
id
id
id
Yeah, I have the same issue. |
Hello @Aesthetikx. This is an issue that popped up a couple of times already, and not an uncommon use case either. I tried to introduce support for custom primary keys before but hit a roadblock when I had to change the tests, which are unfortunately not very maintainable. Feel free to open a new PR and to take a look at my attempt from last year here but keep in mind that it may require some major refactoring of the tests 🙁 |
@Aesthetikx maybe you already found, but there is a "fork" that has what we want: https://github.com/healthie/activerecord_cursor_paginate |
I understand that when giving a
order_by:
column other than the default:id
, extra care is required to ensure that unique results are accounted for because we cannot ensure that the user specified column is unique, and hence adding:id
is required. However, I am thinking that the 'spirit' of this behavior is really not about:id
specifically but about if we are sorting by the table's primary key or not. I have a few cases where unfortunately the table's primary key is not called:id
, and in fact also does not even have a column named:id
, and hence the 'more complex' sorting case is triggered and fails because there is no:id
column.I may or may not take a swing at this change if it seems worthwhile, but at a glance it seems that in
Paginator
something likeorder_by ||= :id
could be replaced withorder_by ||= relation.primary_key.to_sym
, and then a few more spots also would have to be updated where we are relying on the behavior that the primary key is:id
.The text was updated successfully, but these errors were encountered: