Rails 6.1: Fix "ActiveRecord::StatementInvalid: A column has been specified more than once in the order by list"

[mgrunberg]

This commit (rails/rails@55ef531) introduced an additional order(:id) to assert_includes_and_joins_equal.
After that, some tests produce queries like ORDER BY authors.id, [authors].[id] ASC.
Other adapters are fine with this but SQL Server raises an error.

This PR doesn't try to deduplicate orders. That could be a complex task. Instead, it coerces the tests to ensure we don't duplicate order columns.
It also adds a test showing the current behavior.

[wpolicarpo]

Just curious if you had a chance to investigate how difficult would be for the adapter be more proactive if we tried to remove duplication instead?

[mgrunberg]

I thought about adding proactivity but I dropped the initiative without investigating how to implement it.

What I thought is that to cover all cases all orders need to be normalized (authors.id, [authors].id, etc are equal to [authors].[id] ASC), dealing with different structures (text vs arel nodes, etc). We need to deal with [authors].[id] ASC, [authors].[id] DESC. There are edge-cases I'm not sure yet how should be resolved: authors.id, authors.name, [authors].[id] DESC

With all that in mind, I chose to do the easiest thing to make tests pass 😅.

[wpolicarpo]

Ok, thanks for the clarification. This is an already existing problem so let's move on and maybe we can come back to it later.