Merge pull request #42245 from kbrock/nulls_last

Support NullsFirst for all databases.

activerecord/CHANGELOG.md

@@ -1,3 +1,7 @@
+*   Add `.asc.nulls_first` for all databases. Unfortunately MySQL still doesn't like `nulls_last`.
+
+    *Keenan Brock*
+
 *   Improve performance of `one?` and `many?` by limiting the generated count query to 2 results.
 
     *Gonzalo Riestra*

activerecord/lib/arel/visitors/mysql.rb

@@ -58,6 +58,11 @@ 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
+        end
+
         # In the simple case, MySQL allows us to place JOINs directly into the UPDATE
         # query. However, this does not allow for LIMIT, OFFSET and ORDER. To support
         # these, we must use a subquery.

activerecord/lib/arel/visitors/postgresql.rb

@@ -78,16 +78,6 @@ def visit_Arel_Nodes_IsDistinctFrom(o, collector)
           visit o.right, collector
         end
 
-        def visit_Arel_Nodes_NullsFirst(o, collector)
-          visit o.expr, collector
-          collector << " NULLS FIRST"
-        end
-
-        def visit_Arel_Nodes_NullsLast(o, collector)
-          visit o.expr, collector
-          collector << " NULLS LAST"
-        end
-
         BIND_BLOCK = proc { |i| "$#{i}" }
         private_constant :BIND_BLOCK
 

activerecord/lib/arel/visitors/to_sql.rb

@@ -359,6 +359,17 @@ def visit_Arel_Nodes_Descending(o, collector)
           visit(o.expr, collector) << " DESC"
         end
 
+        # NullsFirst is available on all but MySQL, where it is redefined.
+        def visit_Arel_Nodes_NullsFirst(o, collector)
+          visit o.expr, collector
+          collector << " NULLS FIRST"
+        end
+
+        def visit_Arel_Nodes_NullsLast(o, collector)
+          visit o.expr, collector
+          collector << " NULLS LAST"
+        end
+
         def visit_Arel_Nodes_Group(o, collector)
           visit o.expr, collector
         end

activerecord/test/cases/arel/visitors/mysql_test.rb

@@ -152,6 +152,15 @@ def compile(node)
           }
         end
       end
+
+      describe "Nodes::Ordering" do
+        it "should no-op ascending nulls first" do
+          test = Table.new(:users)[:first_name].asc.nulls_first
+          _(compile(test)).must_be_like %{
+            "users"."first_name" ASC
+          }
+        end
+      end
     end
   end
 end

activerecord/test/cases/arel/visitors/postgres_test.rb

@@ -316,36 +316,6 @@ def compile(node)
         end
       end
 
-      describe "Nodes::Ordering" do
-        it "should handle nulls first" do
-          test = Table.new(:users)[:first_name].desc.nulls_first
-          _(compile(test)).must_be_like %{
-            "users"."first_name" DESC NULLS FIRST
-          }
-        end
-
-        it "should handle nulls last" do
-          test = Table.new(:users)[:first_name].desc.nulls_last
-          _(compile(test)).must_be_like %{
-            "users"."first_name" DESC NULLS LAST
-          }
-        end
-
-        it "should handle nulls first reversed" do
-          test = Table.new(:users)[:first_name].desc.nulls_first.reverse
-          _(compile(test)).must_be_like %{
-            "users"."first_name" ASC NULLS LAST
-          }
-        end
-
-        it "should handle nulls last reversed" do
-          test = Table.new(:users)[:first_name].desc.nulls_last.reverse
-          _(compile(test)).must_be_like %{
-            "users"."first_name" ASC NULLS FIRST
-          }
-        end
-      end
-
       describe "Nodes::InfixOperation" do
         it "should handle Contains" do
           inner = Nodes.build_quoted('{"foo":"bar"}')

activerecord/test/cases/arel/visitors/to_sql_test.rb

@@ -382,6 +382,34 @@ def dispatch
             "users"."id" DESC
           }
         end
+
+        it "should handle nulls first" do
+          node = @attr.desc.nulls_first
+          _(compile(node)).must_be_like %{
+            "users"."id" DESC NULLS FIRST
+          }
+        end
+
+        it "should handle nulls last" do
+          node = @attr.desc.nulls_last
+          _(compile(node)).must_be_like %{
+            "users"."id" DESC NULLS LAST
+          }
+        end
+
+        it "should handle nulls first reversed" do
+          node = @attr.desc.nulls_first.reverse
+          _(compile(node)).must_be_like %{
+            "users"."id" ASC NULLS LAST
+          }
+        end
+
+        it "should handle nulls last reversed" do
+          node = @attr.desc.nulls_last.reverse
+          _(compile(node)).must_be_like %{
+            "users"."id" ASC NULLS FIRST
+          }
+        end
       end
 
       describe "Nodes::In" do