Fix `ActiveRecord::QueryMethods#in_order_of` to work with nils

[fatkodima]

When passing nil to in_order_of, the invalid SQL query is generated (because NULL != NULL):

Book.in_order_of(:format, ["hardcover", "ebook", nil]).to_sql

SELECT "books".* FROM "books" WHERE "books"."format" IN ('hardcover', 'ebook', NULL)
ORDER BY CASE "books"."format" WHEN 'hardcover' THEN 1
WHEN 'ebook' THEN 2 WHEN NULL THEN 3 ELSE 4 END ASC

This PR also removes the special order field generation (ORDER BY FIELD) for MYSQL, because it is unable to work with NULLs and the default ORDER BY CASE works for it (tested on MariaDB and MySQL 5.7+).

cc @kddnewton (as a creator of this feature)

Reference: https://github.com/rails/rails/pull/42061

[skipkayhil]

Some previous discussion at #44814

[fatkodima]

Oh, thanks for the reference. I forgot to check if something similar was reported before 😄

I see people suggest to raise when nil is passed, but I do not understand why, because it is a valid value to order by. And I do not get it why ORDER BY FIELD is used for MySQL (except that it makes a query a little simpler).

[kddnewton]

I think this is fine, but I'd really like to understand the implications of removing the FIELD usage here. If it's got no impact on indexed or non-indexed columns it's probably fine, but that feels like a separate change.

I would prefer to use @jonathanhefner's suggestion here: https://github.com/rails/rails/pull/45670#discussion_r931200929 and then if it's desirable to remove the FIELD usage open that change in a different PR.

[simi]

:information_source: I have asked at MariaDB mail-list and MySQL community Slack about the performance difference of ORDER BY FIELD and ORDER BY CASE WHEN. I'll report back once I'll get some insight. :pray:

[simi]

I got nice explanation by Gordan Bobić at MySQL community Slack.

There is no meaningful difference between the two (ORDER BY FIELD, ORDER BY CASE WHEN) because both cases are not indexable for the purpose of ordering. ... because IN () resolves as a range scan and there isn't much helpful you can do beyond that...

I did quick tests again at our production MariaDB and I can confirm I see same plans for various combinations of column types and indexed/non-indexed columns.

Also I would expect ActiveRecord to manage those edge cases (like in_order_of using nil). Personally I would prefer solution mentioned at https://github.com/rails/rails/pull/45670#discussion_r931200929. Keep the query using as elegant syntax at SQL side as possible, until edge case is not needed to be handled. :pray:

[fatkodima]

Thank you for testing. If there is no performance difference, I think it is a good idea to just use the single approach across all the database adapters?

Keep the query using as elegant syntax at SQL side as possible

If I understood you correctly, you recently mentioned that we should not care much about the SQL elegancy - https://github.com/rails/rails/pull/45695#issuecomment-1203016107? 😄

[simi]

Thank you for testing. If there is no performance difference, I think it is a good idea to just use the single approach across all the database adapters?

Keep the query using as elegant syntax at SQL side as possible

If I understood you correctly, you recently mentioned that we should not care much about the SQL elegancy - #45695 (comment)? smile

That linked comment is about Ruby ordering vs SQL ordering. This discussion in here is about 2 potential SQL queries. Here I do prefer to make query simple when possible. In the linked comment I would prefer "complex" SQL over ordering in Ruby and unnecessarily limiting of functionality.

[kddnewton]

I'm 👍 on either approach at this point (keeping the behavior around when there's no nil and using super or just rolling with one method). I think either is fine. Glad to see this get fixed.

[fatkodima]

@jonathanhefner Thank you. Updated with the suggestions.

[jonathanhefner]

Thank you, @fatkodima! (And @simi, and @kddnewton!) :bow:

[iatanas0v]

Is this released? We're running rails 7.0.6 and it's not part of it :thinking:

[fatkodima]

It was not backported into the 7-0-stable branch. I created a PR - https://github.com/rails/rails/pull/48886. It will be available in the next rails 7.0 release.

[alinazulfiqar]

Is there any timeline of when the next rails 7.0 release will be? Do we need to be running 7.0.6 in order to access this feature?

[fatkodima]

No timeline. You can point your rails to 7-0-stable branch in the Gemfile until then, something like gem "rails", github: "rails/rails", branch: "7-0-stable".

[basex]

I'm doing some tests with 10k rows with the ORDER BY FIELD vs ORDER BY CASE (with in_order_of) in rails 7.1 with mysql 8, and ORDER BY FIELD is significantly faster if all 10k rows have a custom ORDER BY.

some example code:

product_ids = Product.limit(10000).ids.reverse

Benchmark.measure { Product.in_order_of(:id, product_ids).all }
# <Benchmark::Tms:0x0000ffff66d684e8 @cstime=0.0, @cutime=0.0, @label="", @real=0.07039317605085671, @stime=0.0, @total=0.07037899999999908, @utime=0.07037899999999908>

Benchmark.measure { Product.where(id: product_ids).order(Arel.sql(Product.sanitize_sql_for_assignment(['field(products.id, ?)', product_ids]))).all }
# <Benchmark::Tms:0x0000ffff6a376a08 @cstime=0.0, @cutime=0.0, @label="", @real=0.02547149290330708, @stime=0.0, @total=0.025472000000000605, @utime=0.025472000000000605>

Let me know if there is something wrong with this benchmark. I run a couple of time each one before taking a measure.