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 1a246f7161.

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 be48ed3071.
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.
This commit is contained in:
Vladimir Kochnev 2018-09-25 14:54:03 +04:00
parent 4f3eb86ffe
commit 41b92914f8
10 changed files with 4 additions and 27 deletions

View File

@ -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 &&

View File

@ -37,7 +37,6 @@ module Arel # :nodoc: all
On
Ordering
RollUp
Top
}.each do |name|
const_set(name, Class.new(Unary))
end

View File

@ -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

View File

@ -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)

View File

@ -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

View File

@ -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"

View File

@ -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

View File

@ -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

View File

@ -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|

View File

@ -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