make sure order quote symbol arguments

[timfjord]

Current order implementation contains symbol parsing and converting it to Arel::Nodes::Ordering
But this code will be never executed, because we preprocess symbol params and convert it to Arel::Nodes::Ascending.
But when we convert it to Arel::Nodes::Ascending it has issues with some values(for example :key, see here #2601)

So i removed symbol preprocessing and leave all calculation on build_order method

Also this pull fixes limited_ids_for method for postgresql adapter, which is passing order_values(raw values that we pass to order) to columns_for_distinct. So for symbol(after i removed preprocessing) and hash arguments it will raise NoMethodError, because there is no to_sql method

[rafaelfranca]

Why the test was removed?

[timfjord]

Because they were very specific to implementation.
After we remove preprocessing, order_values will contain raw data, so there are no ascending?, expr.name and expr.relation.name.
I tried swap node = relation.order_values.first with node = relation.arel.orders.first, but it raises error.
So i decided remove those specs for now.
Please let me know if i need bring them back and adapt to current implementation?

[timfjord]

I can also add specs here to cover hash arguments

[rafaelfranca]

I see. Yes, we will need some integration tests to make sure the behavior was ne regressed.

[timfjord]

Ok. I will work on tests.

[timfjord]

@rafaelfranca i added tests

[senny]

@Timsly is this PR still required? I tried to reproduce the error case with :key but had no success:

  def test_order_by_key
    Post.create(key: "b")
    Post.create(key: "c")
    Post.create(key: "a")
    assert_equal ["a", "b", "c"], Post.order(:key).to_a.map(&:key)
  end

I create a gist of the complete example.

@Timsly Let me know if I'm missing something or if we can close this PR.

[senny]

It's been a month without feedback. I'm going to close this PR. If the requested change is still necessary please report back so we can reopen.