From 059caa206519687345eddf545134e42dad21b0f1 Mon Sep 17 00:00:00 2001
From: tttffff <tristanfellows@icloud.com>
Date: Wed, 15 Nov 2023 21:06:25 +0000
Subject: [PATCH] Arel/MySQL ordering `nulls_first` and `nulls_last`

Fixes #50079 where there is unexpected behaviour for MySQL

Previously:
 - Calling nulls_first on asc worked as expected
 - Calling nulls_first on desc ordered desc nulls last
 - Calling nuls_last on asc raised error
 - Calling nulls_last on desc raised error

Now:
 - Calling nulls_first on asc works as expected
 - Calling nulls_first on desc works as expected
 - Calling nulls_last on asc works as expected
 - Calling nulls_last on desc works as expected
---
 activerecord/CHANGELOG.md                     |  4 +++
 activerecord/lib/arel/visitors/mysql.rb       |  9 +++++--
 .../test/cases/arel/visitors/mysql_test.rb    | 25 +++++++++++++++++--
 3 files changed, 34 insertions(+), 4 deletions(-)

diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 285d93e0b73e0..6d0d101a1816f 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -84,4 +84,8 @@
 
     *Jason Meller*
 
+*   Add `nulls_last` and working `desc.nulls_first` for MySQL.
+
+    *Tristan Fellows*
+
 Please check [7-1-stable](https://github.com/rails/rails/blob/7-1-stable/activerecord/CHANGELOG.md) for previous changes.
diff --git a/activerecord/lib/arel/visitors/mysql.rb b/activerecord/lib/arel/visitors/mysql.rb
index 8b10eb18250c7..495c78ce3d047 100644
--- a/activerecord/lib/arel/visitors/mysql.rb
+++ b/activerecord/lib/arel/visitors/mysql.rb
@@ -59,9 +59,14 @@ def visit_Arel_Nodes_NotRegexp(o, collector)
           infix_value o, collector, " NOT REGEXP "
         end
 
-        # no-op
         def visit_Arel_Nodes_NullsFirst(o, collector)
-          visit o.expr, collector
+          visit(o.expr.expr, collector) << " IS NOT NULL, "
+          visit(o.expr, collector)
+        end
+
+        def visit_Arel_Nodes_NullsLast(o, collector)
+          visit(o.expr.expr, collector) << " IS NULL, "
+          visit(o.expr, collector)
         end
 
         def visit_Arel_Nodes_Cte(o, collector)
diff --git a/activerecord/test/cases/arel/visitors/mysql_test.rb b/activerecord/test/cases/arel/visitors/mysql_test.rb
index c34a11476bc15..c2ce5a4a98ef4 100644
--- a/activerecord/test/cases/arel/visitors/mysql_test.rb
+++ b/activerecord/test/cases/arel/visitors/mysql_test.rb
@@ -154,10 +154,31 @@ def compile(node)
       end
 
       describe "Nodes::Ordering" do
-        it "should no-op ascending nulls first" do
+        it "should handle nulls first" do
           test = Table.new(:users)[:first_name].asc.nulls_first
           _(compile(test)).must_be_like %{
-            "users"."first_name" ASC
+            "users"."first_name" IS NOT NULL, "users"."first_name" ASC
+          }
+        end
+
+        it "should handle nulls last" do
+          test = Table.new(:users)[:first_name].asc.nulls_last
+          _(compile(test)).must_be_like %{
+            "users"."first_name" IS NULL, "users"."first_name" ASC
+          }
+        end
+
+        it "should handle nulls first reversed" do
+          test = Table.new(:users)[:first_name].asc.nulls_first.reverse
+          _(compile(test)).must_be_like %{
+            "users"."first_name" IS NULL, "users"."first_name" DESC
+          }
+        end
+
+        it "should handle nulls last reversed" do
+          test = Table.new(:users)[:first_name].asc.nulls_last.reverse
+          _(compile(test)).must_be_like %{
+            "users"."first_name" IS NOT NULL, "users"."first_name" DESC
           }
         end
       end