Skip to content

Commit

Permalink
Support NullsFirst for all databases.
Browse files Browse the repository at this point in the history
Most databases order tables with the `NULL` value first, having it before
all other data values. Postgres has `NULLS` last.

Fortunately, ANSI SQL has an option to allow the database to specify where NULLS
come out in this sort order

    ORDER BY column ASC NULLS FIRST

MS SQL, SQLite, Oracle, and Postgres all follow this syntax. Unfortunately, MySql
does not.

Before:

PostgreSQL: both `.nulls_first()` and `.nulls_last()` work as designed.
Others: both raise a runtime error.

After:

MySQL: `.nulls_first()` works as designed.
MySQL: `.nulls_last()` raises a runtime error
Others: both work as designed
  • Loading branch information
kbrock committed May 18, 2021
1 parent a34d64d commit b775f0c
Show file tree
Hide file tree
Showing 7 changed files with 57 additions and 40 deletions.
4 changes: 4 additions & 0 deletions 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*
Expand Down
5 changes: 5 additions & 0 deletions activerecord/lib/arel/visitors/mysql.rb
Expand Up @@ -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.
Expand Down
10 changes: 0 additions & 10 deletions activerecord/lib/arel/visitors/postgresql.rb
Expand Up @@ -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

Expand Down
11 changes: 11 additions & 0 deletions activerecord/lib/arel/visitors/to_sql.rb
Expand Up @@ -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
Expand Down
9 changes: 9 additions & 0 deletions activerecord/test/cases/arel/visitors/mysql_test.rb
Expand Up @@ -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
30 changes: 0 additions & 30 deletions activerecord/test/cases/arel/visitors/postgres_test.rb
Expand Up @@ -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"}')
Expand Down
28 changes: 28 additions & 0 deletions activerecord/test/cases/arel/visitors/to_sql_test.rb
Expand Up @@ -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
Expand Down

0 comments on commit b775f0c

Please sign in to comment.