Fix first and last when records are not loaded in a sorted relationship

[GermanDZ]

Summary

Fix last and first methods on a sorted relationship with elements with the same order criteria.

A test was added in 9863eae and is failing with current implementation, for all databases tests, i.e.:

The underlying problem is that first and last when the relationship is not loaded in the parent record uses an optimization based on the database to retrieve the items. The optimization applies correctly the order criteria and also a LIMIT clause to minimize the retrieved data to only the amount of records requested (1 by default, but can be used for .last(4)).

The solution proposed is to add the default ordering criteria (based on implicit_order_column and primary_key) to disambiguate the sorting on the database.

Related issues

This PR adds a second column for implicit_order_column => #37626? Am I duplicating the solution and it's already solved? @pawurb?

Probably solving this issue #36475?

No, after reproducing the case with the fix applies is still failing.

[p8]

Hi @GermanDZ, Thanks for creating this PR. There also is a newer related ticket: #39525

[jonathanhefner]

I agree with @p8 that #39525 would be a better approach to solve this, though it wouldn't be an "automatic" solution.

One problem with this PR is that it doesn't address the order of first, nor any of the other ordinal finders (e.g. owner.pets.second).

A larger problem, though, is that it affects last unconditionally. For example, Part.order(:serial_number).last will now always use ORDER BY "parts"."serial_number" DESC, "parts"."id" DESC, which is both unexpected and a potential performance issue.

[rails-bot]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
Thank you for your contributions.

[GermanDZ]

@p8 / @jonathanhefner I've just added my failing test case (just a cherry-pick of my commit) to the #39525 PR and is still failing.

We should consider mix both solutions in a single PR? Maybe we could wait until the other PR is merged and then adapt this PR to fix "my" problem on top of the previous (better) solution.

WDYT?

[rails-bot]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
Thank you for your contributions.

[GermanDZ]

Waiting for merging of #39525 to rebase and rework this solution

[rails-bot]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
Thank you for your contributions.

[GermanDZ]

Still waiting for #39525 to rebase and rework this solution

[rails-bot]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
Thank you for your contributions.

[GermanDZ]

Still waiting for #39525 to rebase and rework this solution

[rails-bot]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
Thank you for your contributions.

[GermanDZ]

Still waiting for #39525 to rebase and rework this solution

[rails-bot]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
Thank you for your contributions.

[GermanDZ]

Still waiting for #39525 to rebase and rework this solution

[rails-bot]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
Thank you for your contributions.

[GermanDZ]

Still waiting for #39525 to rebase and rework this solution