Extend ordering with null sorting

spec/query_builder/merge_spec.cr

@@ -19,9 +19,9 @@ describe "Avram::QueryBuilder#merge" do
 
   it "merges orders" do
     query_1 = new_query
-      .order_by(:id, :asc)
+      .order_by(Avram::OrderBy.new(:id, :asc))
     query_2 = new_query
-      .order_by(:name, :desc)
+      .order_by(Avram::OrderBy.new(:name, :desc))
 
     query_1.merge(query_2)
     query_1.statement.should contain "ORDER BY id ASC, name DESC"

spec/query_builder_spec.cr

@@ -9,8 +9,8 @@ describe Avram::QueryBuilder do
       .raw_where(Avram::Where::Raw.new("name = ?", "Mikias"))
       .join(Avram::Join::Inner.new(:users, :posts))
       .join(Avram::Join::Inner.new(:users, :posts))
-      .order_by(:my_column, :asc)
-      .order_by(:my_column, :asc)
+      .order_by(Avram::OrderBy.new(:my_column, :asc))
+      .order_by(Avram::OrderBy.new(:my_column, :asc))
 
     query.wheres.size.should eq(1)
     query.raw_wheres.size.should eq(1)
@@ -20,7 +20,7 @@ describe Avram::QueryBuilder do
   end
 
   it "can reset order" do
-    query = new_query.order_by(:my_column, :asc)
+    query = new_query.order_by(Avram::OrderBy.new(:my_column, :asc))
     query.statement.should eq "SELECT * FROM users ORDER BY my_column ASC"
 
     query.reset_order
@@ -78,14 +78,14 @@ describe Avram::QueryBuilder do
 
   it "can be ordered" do
     query = new_query
-      .order_by(:name, :asc)
-      .order_by(:birthday, :asc)
-      .order_by(:email, :desc)
+      .order_by(Avram::OrderBy.new(:name, :asc))
+      .order_by(Avram::OrderBy.new(:birthday, :asc))
+      .order_by(Avram::OrderBy.new(:email, :desc))
     query.statement.should eq "SELECT * FROM users ORDER BY name ASC, birthday ASC, email DESC"
     query.args.should eq [] of String
 
     query = new_query
-      .order_by(:name, :asc)
+      .order_by(Avram::OrderBy.new(:name, :asc))
     query.statement.should eq "SELECT * FROM users ORDER BY name ASC"
   end
 
@@ -196,7 +196,7 @@ describe Avram::QueryBuilder do
     it "reverses the order of the query" do
       query = Avram::QueryBuilder
         .new(table: :users)
-        .order_by(:id, :asc)
+        .order_by(Avram::OrderBy.new(:id, :asc))
         .reverse_order
 
       query.statement.should eq "SELECT * FROM users ORDER BY id DESC"
@@ -205,8 +205,8 @@ describe Avram::QueryBuilder do
     it "reverses both directions" do
       query = Avram::QueryBuilder
         .new(table: :users)
-        .order_by(:id, :asc)
-        .order_by(:name, :desc)
+        .order_by(Avram::OrderBy.new(:id, :asc))
+        .order_by(Avram::OrderBy.new(:name, :desc))
         .reverse_order
 
       query.statement.should eq "SELECT * FROM users ORDER BY name ASC, id DESC"
@@ -229,7 +229,7 @@ describe Avram::QueryBuilder do
     it "returns true if the query is ordered" do
       query = Avram::QueryBuilder
         .new(table: :users)
-        .order_by(:id, :asc)
+        .order_by(Avram::OrderBy.new(:id, :asc))
 
       query.ordered?.should eq true
     end
@@ -248,7 +248,7 @@ describe Avram::QueryBuilder do
         .select([:name, :age])
         .join(Avram::Join::Inner.new(:users, :posts))
         .where(Avram::Where::Equal.new(:name, "Paul"))
-        .order_by(:id, :asc)
+        .order_by(Avram::OrderBy.new(:id, :asc))
         .limit(1)
         .offset(2)
       cloned_query = new_query

spec/query_spec.cr

@@ -625,6 +625,24 @@ describe Avram::Query do
       query = Post::BaseQuery.new.where_comments(Comment::BaseQuery.new.created_at.asc_order)
       query.to_sql[0].should contain "ORDER BY comments.created_at ASC"
     end
+
+    it "orders nulls first" do
+      query = Post::BaseQuery.new.published_at.asc_order(:nulls_first)
+
+      query.to_sql[0].should contain "ORDER BY posts.published_at ASC NULLS FIRST"
+    end
+
+    it "orders nulls last" do
+      query = Post::BaseQuery.new.published_at.asc_order(:nulls_last)
+
+      query.to_sql[0].should contain "ORDER BY posts.published_at ASC NULLS LAST"
+    end
+
+    it "returns a nice error when trying to order by a weird direction" do
+      expect_raises(Exception, /Accepted values are: :asc, :desc/) do
+        Post::BaseQuery.new.order_by(:published_at, :sideways)
+      end
+    end
   end
 
   describe "cloning queries" do

src/avram/criteria.cr

@@ -6,13 +6,13 @@ class Avram::Criteria(T, V)
     @negate_next_criteria = false
   end
 
-  def desc_order : T
-    rows.query.order_by(column, :desc)
+  def desc_order(null_sorting : Avram::OrderBy::NullSorting = :default) : T
+    rows.query.order_by(Avram::OrderBy.new(column, :desc, null_sorting))
     rows
   end
 
-  def asc_order : T
-    rows.query.order_by(column, :asc)
+  def asc_order(null_sorting : Avram::OrderBy::NullSorting = :default) : T
+    rows.query.order_by(Avram::OrderBy.new(column, :asc, null_sorting))
     rows
   end
 

src/avram/order_by.cr

@@ -0,0 +1,37 @@
+module Avram
+  class OrderBy
+    enum NullSorting
+      DEFAULT
+      NULLS_FIRST
+      NULLS_LAST
+
+      def to_s
+        super.gsub("_", " ")
+      end
+    end
+
+    enum Direction
+      ASC
+      DESC
+    end
+
+    getter column
+    getter direction
+    getter nulls
+
+    def initialize(@column : String | Symbol, @direction : Direction, @nulls : NullSorting = :default)
+    end
+
+    def reversed
+      @direction = @direction.asc? ? Direction::DESC : Direction::ASC
+      self
+    end
+
+    def prepare
+      String.build do |str|
+        str << "#{column} #{direction}"
+        str << " #{nulls}" unless nulls.default?
+      end
+    end
+  end
+end

src/avram/query_builder.cr

@@ -6,18 +6,13 @@ class Avram::QueryBuilder
   @wheres = [] of Avram::Where::SqlClause
   @raw_wheres = [] of Avram::Where::Raw
   @joins = [] of Avram::Join::SqlClause
-  @orders = {
-    asc:  [] of Symbol | String,
-    desc: [] of Symbol | String,
-  }
+  @orders = [] of Avram::OrderBy
   @selections : String = "*"
   @prepared_statement_placeholder = 0
   @distinct : Bool = false
   @delete : Bool = false
   @distinct_on : String | Symbol | Nil = nil
 
-  VALID_DIRECTIONS = [:asc, :desc]
-
   def initialize(@table : Symbol)
   end
 
@@ -39,10 +34,8 @@ class Avram::QueryBuilder
       join(join)
     end
 
-    query_to_merge.orders.each do |direction, order_bys|
-      order_bys.each do |order|
-        order_by(order, direction)
-      end
+    query_to_merge.orders.each do |order|
+      order_by(order)
     end
   end
 
@@ -135,38 +128,28 @@ class Avram::QueryBuilder
     self
   end
 
-  def order_by(column, direction : Symbol)
-    raise "Direction must be :asc or :desc, got #{direction}" unless VALID_DIRECTIONS.includes?(direction)
-    @orders[direction] << column
+  def order_by(order : OrderBy)
+    @orders << order
     self
   end
 
   def reset_order
-    @orders.values.each(&.clear)
+    @orders.clear
   end
 
   def reverse_order
-    @orders = {
-      asc:  @orders[:desc],
-      desc: @orders[:asc],
-    }
+    @orders = @orders.map(&.reversed).reverse
     self
   end
 
   def order_sql
     if ordered?
-      "ORDER BY " + orders.map do |direction, columns|
-        next if columns.empty?
-        "#{columns.join(" #{direction.to_s.upcase}, ")} #{direction.to_s.upcase}"
-      end.reject(&.nil?).join(", ")
+      "ORDER BY " + orders.map(&.prepare).join(", ")
     end
   end
 
   def orders
-    {
-      asc:  @orders[:asc].uniq,
-      desc: @orders[:desc].uniq,
-    }
+    @orders.uniq!(&.column)
   end
 
   def select_count
@@ -226,9 +209,7 @@ class Avram::QueryBuilder
   end
 
   def ordered?
-    @orders.values.any? do |columns|
-      columns.any?
-    end
+    @orders.any?
   end
 
   private def select_sql

src/avram/queryable.cr

@@ -99,8 +99,11 @@ module Avram::Queryable(T)
   end
 
   def order_by(column, direction) : self
-    query.order_by(column, direction)
+    direction = Avram::OrderBy::Direction.parse(direction.to_s)
+    query.order_by(Avram::OrderBy.new(column, direction))
     self
+  rescue e : ArgumentError
+    raise "#{e.message}. Accepted values are: :asc, :desc"
   end
 
   def none : self