Abandon TOP support.

Initially, `TOP` was introduced to support `limit` for MSSQL database.
Unlike PostgreSQL/MySQL/SQLite, MSSQL does not have native `LIMIT`/`OFFSET` support.
The commit adding `TOP` is 1a246f71616cf246a75ef6cbdb56032e43d4e643.

However, it figured out that `TOP` implementation was weak and it's not sufficient
to also support `OFFSET`, then `TOP` was substituted with
`ROW_NUMBER()` subquery in be48ed3071fd6524d0145c4ad3faeb4aafe3eda3.
This is a well known trick in MSSQL -
https://stackoverflow.com/questions/2135418/equivalent-of-limit-and-offset-for-sql-server.

So now we don't need this `visit_Arel_Nodes_Top` at all.
It does nothing useful but also adds an extra space after `SELECT` when `LIMIT` is being
used for **any** database.

10 files changed, 4 insertions, 27 deletions

diff --git a/activerecord/lib/arel/nodes/select_core.rb b/activerecord/lib/arel/nodes/select_core.rb
index 2defe61974..73461ff683 100644
--- a/activerecord/lib/arel/nodes/select_core.rb
+++ b/activerecord/lib/arel/nodes/select_core.rb
@@ -3,13 +3,12 @@
 module Arel # :nodoc: all
   module Nodes
     class SelectCore < Arel::Nodes::Node
-      attr_accessor :top, :projections, :wheres, :groups, :windows
+      attr_accessor :projections, :wheres, :groups, :windows
       attr_accessor :havings, :source, :set_quantifier
 
       def initialize
         super()
         @source         = JoinSource.new nil
-        @top            = nil
 
         # https://ronsavage.github.io/SQL/sql-92.bnf.html#set%20quantifier
         @set_quantifier = nil
@@ -43,7 +42,7 @@ module Arel # :nodoc: all
 
       def hash
         [
-          @source, @top, @set_quantifier, @projections,
+          @source, @set_quantifier, @projections,
           @wheres, @groups, @havings, @windows
         ].hash
       end
@@ -51,7 +50,6 @@ module Arel # :nodoc: all
       def eql?(other)
         self.class == other.class &&
           self.source == other.source &&
-          self.top == other.top &&
           self.set_quantifier == other.set_quantifier &&
           self.projections == other.projections &&
           self.wheres == other.wheres &&
diff --git a/activerecord/lib/arel/nodes/unary.rb b/activerecord/lib/arel/nodes/unary.rb
index a3c0045897..00639304e4 100644
--- a/activerecord/lib/arel/nodes/unary.rb
+++ b/activerecord/lib/arel/nodes/unary.rb
@@ -37,7 +37,6 @@ module Arel # :nodoc: all
       On
       Ordering
       RollUp
-      Top
     }.each do |name|
       const_set(name, Class.new(Unary))
     end
diff --git a/activerecord/lib/arel/select_manager.rb b/activerecord/lib/arel/select_manager.rb
index 733176ad9e..a2b2838a3d 100644
--- a/activerecord/lib/arel/select_manager.rb
+++ b/activerecord/lib/arel/select_manager.rb
@@ -222,10 +222,8 @@ module Arel # :nodoc: all
     def take(limit)
       if limit
         @ast.limit = Nodes::Limit.new(limit)
-        @ctx.top   = Nodes::Top.new(limit)
       else
         @ast.limit = nil
-        @ctx.top   = nil
       end
       self
     end
diff --git a/activerecord/lib/arel/visitors/depth_first.rb b/activerecord/lib/arel/visitors/depth_first.rb
index bcf8f8f980..5948622aea 100644
--- a/activerecord/lib/arel/visitors/depth_first.rb
+++ b/activerecord/lib/arel/visitors/depth_first.rb
@@ -34,7 +34,6 @@ module Arel # :nodoc: all
         alias :visit_Arel_Nodes_Ordering          :unary
         alias :visit_Arel_Nodes_Ascending         :unary
         alias :visit_Arel_Nodes_Descending        :unary
-        alias :visit_Arel_Nodes_Top               :unary
         alias :visit_Arel_Nodes_UnqualifiedColumn :unary
 
         def function(o)
diff --git a/activerecord/lib/arel/visitors/dot.rb b/activerecord/lib/arel/visitors/dot.rb
index d352b81914..76830412d4 100644
--- a/activerecord/lib/arel/visitors/dot.rb
+++ b/activerecord/lib/arel/visitors/dot.rb
@@ -81,7 +81,6 @@ module Arel # :nodoc: all
         alias :visit_Arel_Nodes_Not               :unary
         alias :visit_Arel_Nodes_Offset            :unary
         alias :visit_Arel_Nodes_On                :unary
-        alias :visit_Arel_Nodes_Top               :unary
         alias :visit_Arel_Nodes_UnqualifiedColumn :unary
         alias :visit_Arel_Nodes_Preceding         :unary
         alias :visit_Arel_Nodes_Following         :unary
diff --git a/activerecord/lib/arel/visitors/mssql.rb b/activerecord/lib/arel/visitors/mssql.rb
index 9aedc51d15..d564e19089 100644
--- a/activerecord/lib/arel/visitors/mssql.rb
+++ b/activerecord/lib/arel/visitors/mssql.rb
@@ -12,13 +12,6 @@ module Arel # :nodoc: all
 
       private
 
-        # `top` wouldn't really work here. I.e. User.select("distinct first_name").limit(10) would generate
-        # "select top 10 distinct first_name from users", which is invalid query! it should be
-        # "select distinct top 10 first_name from users"
-        def visit_Arel_Nodes_Top(o)
-          ""
-        end
-
         def visit_Arel_Visitors_MSSQL_RowNumber(o, collector)
           collector << "ROW_NUMBER() OVER (ORDER BY "
           inject_join(o.children, collector, ", ") << ") as _row_num"
diff --git a/activerecord/lib/arel/visitors/to_sql.rb b/activerecord/lib/arel/visitors/to_sql.rb
index 0682c066fb..81ca63f261 100644
--- a/activerecord/lib/arel/visitors/to_sql.rb
+++ b/activerecord/lib/arel/visitors/to_sql.rb
@@ -237,8 +237,6 @@ module Arel # :nodoc: all
         def visit_Arel_Nodes_SelectCore(o, collector)
           collector << "SELECT"
 
-          collector = maybe_visit o.top, collector
-
           collector = maybe_visit o.set_quantifier, collector
 
           collect_nodes_for o.projections, collector, SPACE
@@ -405,11 +403,6 @@ module Arel # :nodoc: all
           visit o.expr, collector
         end
 
-        # FIXME: this does nothing on most databases, but does on MSSQL
-        def visit_Arel_Nodes_Top(o, collector)
-          collector
-        end
-
         def visit_Arel_Nodes_Lock(o, collector)
           visit o.expr, collector
         end
diff --git a/activerecord/test/cases/arel/select_manager_test.rb b/activerecord/test/cases/arel/select_manager_test.rb
index c17487ae88..5220950905 100644
--- a/activerecord/test/cases/arel/select_manager_test.rb
+++ b/activerecord/test/cases/arel/select_manager_test.rb
@@ -131,7 +131,7 @@ module Arel
           right = table.alias
           mgr   = table.from
           mgr.join(right).on("omg")
-          mgr.to_sql.must_be_like %{ SELECT  FROM "users" INNER JOIN "users" "users_2" ON omg }
+          mgr.to_sql.must_be_like %{ SELECT FROM "users" INNER JOIN "users" "users_2" ON omg }
         end
 
         it "converts to sqlliterals with multiple items" do
@@ -139,7 +139,7 @@ module Arel
           right = table.alias
           mgr   = table.from
           mgr.join(right).on("omg", "123")
-          mgr.to_sql.must_be_like %{ SELECT  FROM "users" INNER JOIN "users" "users_2" ON omg AND 123 }
+          mgr.to_sql.must_be_like %{ SELECT FROM "users" INNER JOIN "users" "users_2" ON omg AND 123 }
         end
       end
     end
diff --git a/activerecord/test/cases/arel/visitors/depth_first_test.rb b/activerecord/test/cases/arel/visitors/depth_first_test.rb
index 3baccc7316..f94ad521d7 100644
--- a/activerecord/test/cases/arel/visitors/depth_first_test.rb
+++ b/activerecord/test/cases/arel/visitors/depth_first_test.rb
@@ -33,7 +33,6 @@ module Arel
         Arel::Nodes::Ordering,
         Arel::Nodes::StringJoin,
         Arel::Nodes::UnqualifiedColumn,
-        Arel::Nodes::Top,
         Arel::Nodes::Limit,
         Arel::Nodes::Else,
       ].each do |klass|
diff --git a/activerecord/test/cases/arel/visitors/dot_test.rb b/activerecord/test/cases/arel/visitors/dot_test.rb
index 98f3bab620..6b3c132f83 100644
--- a/activerecord/test/cases/arel/visitors/dot_test.rb
+++ b/activerecord/test/cases/arel/visitors/dot_test.rb
@@ -37,7 +37,6 @@ module Arel
         Arel::Nodes::Offset,
         Arel::Nodes::Ordering,
         Arel::Nodes::UnqualifiedColumn,
-        Arel::Nodes::Top,
         Arel::Nodes::Limit,
       ].each do |klass|
         define_method("test_#{klass.name.gsub('::', '_')}") do