aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorSean Griffin <sean@thoughtbot.com>2014-11-26 13:45:31 -0700
committerSean Griffin <sean@thoughtbot.com>2014-11-29 16:08:51 -0700
commit7508284800f67b4611c767bff9eae7045674b66f (patch)
tree9f16f258662c4b0165f1ee255eddb3f07d6aeeb6
parentec083687a96af1f35f9fb9e75611cc4bf4f5bf81 (diff)
downloadrails-7508284800f67b4611c767bff9eae7045674b66f.tar.gz
rails-7508284800f67b4611c767bff9eae7045674b66f.tar.bz2
rails-7508284800f67b4611c767bff9eae7045674b66f.zip
Remove engine from the constructor arguments `Arel::Table`
It is never used outside of convenience methods which are only used in tests. In practice, it just made constructing tables more complicated on the rails side. This is the minimum possible change to remove the constructor argument, but continue to have the tests passing. I'm not sure if we have a reason to keep `project` and friends, and the solution might actually just be to remove the engine from `SelectManager` and friends. As such I've held off on deleting those methods. We need to figure out what to do with `Table#from`. It's old invocation, which read `table.from(table)` was certainly nonsensical.
-rw-r--r--lib/arel/table.rb61
-rw-r--r--test/nodes/test_table_alias.rb13
-rw-r--r--test/test_select_manager.rb32
-rw-r--r--test/test_table.rb46
4 files changed, 49 insertions, 103 deletions
diff --git a/lib/arel/table.rb b/lib/arel/table.rb
index fb86c03404..d534c44fa1 100644
--- a/lib/arel/table.rb
+++ b/lib/arel/table.rb
@@ -11,20 +11,18 @@ module Arel
# TableAlias and Table both have a #table_name which is the name of the underlying table
alias :table_name :name
- def initialize name, engine = Table.engine
+ def initialize name, options = {}
@name = name.to_s
- @engine = engine
@columns = nil
@aliases = []
- @table_alias = nil
-
- if Hash === engine
- @engine = engine[:engine] || Table.engine
-
- # Sometime AR sends an :as parameter to table, to let the table know
- # that it is an Alias. We may want to override new, and return a
- # TableAlias node?
- @table_alias = engine[:as] unless engine[:as].to_s == @name
+ @engine = Table.engine
+
+ # Sometime AR sends an :as parameter to table, to let the table know
+ # that it is an Alias. We may want to override new, and return a
+ # TableAlias node?
+ @table_alias = options[:as]
+ if @table_alias.to_s == @name
+ @table_alias = nil
end
end
@@ -34,12 +32,12 @@ module Arel
end
end
- def from table
- SelectManager.new(@engine, table)
+ def from engine = Table.engine
+ SelectManager.new(engine, self)
end
def join relation, klass = Nodes::InnerJoin
- return from(self) unless relation
+ return from unless relation
case relation
when String, Nodes::SqlLiteral
@@ -47,7 +45,7 @@ module Arel
klass = Nodes::StringJoin
end
- from(self).join(relation, klass)
+ from.join(relation, klass)
end
def outer_join relation
@@ -55,55 +53,39 @@ module Arel
end
def group *columns
- from(self).group(*columns)
+ from.group(*columns)
end
def order *expr
- from(self).order(*expr)
+ from.order(*expr)
end
def where condition
- from(self).where condition
+ from.where condition
end
def project *things
- from(self).project(*things)
+ from.project(*things)
end
def take amount
- from(self).take amount
+ from.take amount
end
def skip amount
- from(self).skip amount
+ from.skip amount
end
def having expr
- from(self).having expr
+ from.having expr
end
def [] name
::Arel::Attribute.new self, name
end
- def select_manager
- SelectManager.new(@engine)
- end
-
- def insert_manager
- InsertManager.new(@engine)
- end
-
- def update_manager
- UpdateManager.new(@engine)
- end
-
- def delete_manager
- DeleteManager.new(@engine)
- end
-
def hash
- # Perf note: aliases, table alias and engine is excluded from the hash
+ # Perf note: aliases and table alias is excluded from the hash
# aliases can have a loop back to this table breaking hashes in parent
# relations, for the vast majority of cases @name is unique to a query
@name.hash
@@ -112,7 +94,6 @@ module Arel
def eql? other
self.class == other.class &&
self.name == other.name &&
- self.engine == other.engine &&
self.aliases == other.aliases &&
self.table_alias == other.table_alias
end
diff --git a/test/nodes/test_table_alias.rb b/test/nodes/test_table_alias.rb
index 4aafd12b79..e30f97b748 100644
--- a/test/nodes/test_table_alias.rb
+++ b/test/nodes/test_table_alias.rb
@@ -5,27 +5,26 @@ module Arel
module Nodes
describe 'table alias' do
it 'has an #engine which delegates to the relation' do
- engine = 'vroom'
- relation = Table.new(:users, engine)
+ relation = OpenStruct.new(engine: 'vroom')
node = TableAlias.new relation, :foo
- node.engine.must_equal engine
+ node.engine.must_equal 'vroom'
end
describe 'equality' do
it 'is equal with equal ivars' do
- relation1 = Table.new(:users, 'vroom')
+ relation1 = Table.new(:users)
node1 = TableAlias.new relation1, :foo
- relation2 = Table.new(:users, 'vroom')
+ relation2 = Table.new(:users)
node2 = TableAlias.new relation2, :foo
array = [node1, node2]
assert_equal 1, array.uniq.size
end
it 'is not equal with different ivars' do
- relation1 = Table.new(:users, 'vroom')
+ relation1 = Table.new(:users)
node1 = TableAlias.new relation1, :foo
- relation2 = Table.new(:users, 'vroom')
+ relation2 = Table.new(:users)
node2 = TableAlias.new relation2, :bar
array = [node1, node2]
assert_equal 2, array.uniq.size
diff --git a/test/test_select_manager.rb b/test/test_select_manager.rb
index 78b3a25928..f55b3877eb 100644
--- a/test/test_select_manager.rb
+++ b/test/test_select_manager.rb
@@ -110,14 +110,14 @@ module Arel
describe 'having' do
it 'converts strings to SQLLiterals' do
table = Table.new :users
- mgr = table.from table
+ mgr = table.from
mgr.having 'foo'
mgr.to_sql.must_be_like %{ SELECT FROM "users" HAVING foo }
end
it 'can have multiple items specified separately' do
table = Table.new :users
- mgr = table.from table
+ mgr = table.from
mgr.having 'foo'
mgr.having 'bar'
mgr.to_sql.must_be_like %{ SELECT FROM "users" HAVING foo AND bar }
@@ -125,7 +125,7 @@ module Arel
it 'can have multiple items specified together' do
table = Table.new :users
- mgr = table.from table
+ mgr = table.from
mgr.having 'foo', 'bar'
mgr.to_sql.must_be_like %{ SELECT FROM "users" HAVING foo AND bar }
end
@@ -135,7 +135,7 @@ module Arel
it 'converts to sqlliterals' do
table = Table.new :users
right = table.alias
- mgr = table.from table
+ mgr = table.from
mgr.join(right).on("omg")
mgr.to_sql.must_be_like %{ SELECT FROM "users" INNER JOIN "users" "users_2" ON omg }
end
@@ -143,7 +143,7 @@ module Arel
it 'converts to sqlliterals with multiple items' do
table = Table.new :users
right = table.alias
- mgr = table.from table
+ 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 }
end
@@ -153,7 +153,7 @@ module Arel
describe 'clone' do
it 'creates new cores' do
table = Table.new :users, :as => 'foo'
- mgr = table.from table
+ mgr = table.from
m2 = mgr.clone
m2.project "foo"
mgr.to_sql.wont_equal m2.to_sql
@@ -161,7 +161,7 @@ module Arel
it 'makes updates to the correct copy' do
table = Table.new :users, :as => 'foo'
- mgr = table.from table
+ mgr = table.from
m2 = mgr.clone
m3 = m2.clone
m2.project "foo"
@@ -173,7 +173,7 @@ module Arel
describe 'initialize' do
it 'uses alias in sql' do
table = Table.new :users, :as => 'foo'
- mgr = table.from table
+ mgr = table.from
mgr.skip 10
mgr.to_sql.must_be_like %{ SELECT FROM "users" "foo" OFFSET 10 }
end
@@ -182,14 +182,14 @@ module Arel
describe 'skip' do
it 'should add an offset' do
table = Table.new :users
- mgr = table.from table
+ mgr = table.from
mgr.skip 10
mgr.to_sql.must_be_like %{ SELECT FROM "users" OFFSET 10 }
end
it 'should chain' do
table = Table.new :users
- mgr = table.from table
+ mgr = table.from
mgr.skip(10).to_sql.must_be_like %{ SELECT FROM "users" OFFSET 10 }
end
end
@@ -197,14 +197,14 @@ module Arel
describe 'offset' do
it 'should add an offset' do
table = Table.new :users
- mgr = table.from table
+ mgr = table.from
mgr.offset = 10
mgr.to_sql.must_be_like %{ SELECT FROM "users" OFFSET 10 }
end
it 'should remove an offset' do
table = Table.new :users
- mgr = table.from table
+ mgr = table.from
mgr.offset = 10
mgr.to_sql.must_be_like %{ SELECT FROM "users" OFFSET 10 }
@@ -214,7 +214,7 @@ module Arel
it 'should return the offset' do
table = Table.new :users
- mgr = table.from table
+ mgr = table.from
mgr.offset = 10
assert_equal 10, mgr.offset
end
@@ -379,13 +379,13 @@ module Arel
describe 'ast' do
it 'should return the ast' do
table = Table.new :users
- mgr = table.from table
+ mgr = table.from
assert mgr.ast
end
it 'should allow orders to work when the ast is grepped' do
table = Table.new :users
- mgr = table.from table
+ mgr = table.from
mgr.project Arel.sql '*'
mgr.from table
mgr.orders << Arel::Nodes::Ascending.new(Arel.sql('foo'))
@@ -406,7 +406,7 @@ module Arel
# This should fail on other databases
it 'adds a lock node' do
table = Table.new :users
- mgr = table.from table
+ mgr = table.from
mgr.lock.to_sql.must_be_like %{ SELECT FROM "users" FOR UPDATE }
end
end
diff --git a/test/test_table.rb b/test/test_table.rb
index 14256475ec..394a755727 100644
--- a/test/test_table.rb
+++ b/test/test_table.rb
@@ -47,12 +47,6 @@ module Arel
assert_equal "INSERT INTO \"users\" VALUES(NULL)", im.to_sql
end
- it 'should return IM from insert_manager' do
- im = @relation.insert_manager
- assert_kind_of Arel::InsertManager, im
- assert_equal im.engine, @relation.engine
- end
-
describe 'skip' do
it 'should add an offset' do
sm = @relation.skip 2
@@ -60,29 +54,6 @@ module Arel
end
end
- describe 'select_manager' do
- it 'should return an empty select manager' do
- sm = @relation.select_manager
- sm.to_sql.must_be_like 'SELECT'
- end
- end
-
- describe 'update_manager' do
- it 'should return an update manager' do
- um = @relation.update_manager
- assert_kind_of Arel::UpdateManager, um
- assert_equal um.engine, @relation.engine
- end
- end
-
- describe 'delete_manager' do
- it 'should return a delete manager' do
- dm = @relation.delete_manager
- assert_kind_of Arel::DeleteManager, dm
- assert_equal dm.engine, @relation.engine
- end
- end
-
describe 'having' do
it 'adds a having clause' do
mgr = @relation.having @relation[:id].eq(10)
@@ -149,14 +120,9 @@ module Arel
end
describe 'new' do
- it 'should accept an engine' do
- rel = Table.new :users, 'foo'
- rel.engine.must_equal 'foo'
- end
-
it 'should accept a hash' do
- rel = Table.new :users, :engine => 'foo'
- rel.engine.must_equal 'foo'
+ rel = Table.new :users, :as => 'foo'
+ rel.table_alias.must_equal 'foo'
end
it 'ignores as if it equals name' do
@@ -228,10 +194,10 @@ module Arel
describe 'equality' do
it 'is equal with equal ivars' do
- relation1 = Table.new(:users, 'vroom')
+ relation1 = Table.new(:users)
relation1.aliases = %w[a b c]
relation1.table_alias = 'zomg'
- relation2 = Table.new(:users, 'vroom')
+ relation2 = Table.new(:users)
relation2.aliases = %w[a b c]
relation2.table_alias = 'zomg'
array = [relation1, relation2]
@@ -239,10 +205,10 @@ module Arel
end
it 'is not equal with different ivars' do
- relation1 = Table.new(:users, 'vroom')
+ relation1 = Table.new(:users)
relation1.aliases = %w[a b c]
relation1.table_alias = 'zomg'
- relation2 = Table.new(:users, 'vroom')
+ relation2 = Table.new(:users)
relation2.aliases = %w[x y z]
relation2.table_alias = 'zomg'
array = [relation1, relation2]