Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stop creating SQL literals for aliases. #187

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions lib/arel/alias_predication.rb
@@ -1,7 +1,7 @@
module Arel
module AliasPredication
def as other
Nodes::As.new self, Nodes::SqlLiteral.new(other)
Nodes::As.new self, other
end
end
end
end
4 changes: 2 additions & 2 deletions lib/arel/nodes/extract.rb
Expand Up @@ -11,11 +11,11 @@ class Extract < Arel::Nodes::Unary
def initialize expr, field, aliaz = nil
super(expr)
@field = field
@alias = aliaz && SqlLiteral.new(aliaz)
@alias = aliaz
end

def as aliaz
self.alias = SqlLiteral.new(aliaz)
self.alias = aliaz
self
end

Expand Down
4 changes: 2 additions & 2 deletions lib/arel/nodes/function.rb
Expand Up @@ -9,12 +9,12 @@ class Function < Arel::Nodes::Node
def initialize expr, aliaz = nil
super()
@expressions = expr
@alias = aliaz && SqlLiteral.new(aliaz)
@alias = aliaz
@distinct = false
end

def as aliaz
self.alias = SqlLiteral.new(aliaz)
self.alias = aliaz
self
end

Expand Down
2 changes: 1 addition & 1 deletion lib/arel/select_manager.rb
Expand Up @@ -44,7 +44,7 @@ def exists
end

def as other
create_table_alias grouping(@ast), Nodes::SqlLiteral.new(other)
create_table_alias grouping(@ast), other
end

def where_clauses
Expand Down
49 changes: 49 additions & 0 deletions lib/arel/visitors/postgresql.rb
Expand Up @@ -14,6 +14,55 @@ def visit_Arel_Nodes_DoesNotMatch o, a
def visit_Arel_Nodes_DistinctOn o, a
"DISTINCT ON ( #{visit o.expr, a} )"
end

def visit_Arel_Nodes_As o, a
"#{visit o.left, a} AS #{o.right}"
end

def visit_Arel_Nodes_TableAlias o, a
"#{visit o.relation, a} #{o.name}"
end

def visit_Arel_Nodes_NamedFunction o, a
"#{o.name}(#{o.distinct ? 'DISTINCT ' : ''}#{o.expressions.map { |x|
visit x, a
}.join(', ')})#{o.alias ? " AS #{o.alias}" : ''}"
end

def visit_Arel_Nodes_Extract o, a
"EXTRACT(#{o.field.to_s.upcase} FROM #{visit o.expr, a})#{o.alias ? " AS #{o.alias}" : ''}"
end

def visit_Arel_Nodes_Count o, a
"COUNT(#{o.distinct ? 'DISTINCT ' : ''}#{o.expressions.map { |x|
visit x, a
}.join(', ')})#{o.alias ? " AS #{o.alias}" : ''}"
end

def visit_Arel_Nodes_Sum o, a
"SUM(#{o.distinct ? 'DISTINCT ' : ''}#{o.expressions.map { |x|
visit x, a }.join(', ')})#{o.alias ? " AS #{o.alias}" : ''}"
end

def visit_Arel_Nodes_Exists o, a
"EXISTS (#{visit o.expressions, a})#{
o.alias ? " AS #{o.alias}" : ''}"
end

def visit_Arel_Nodes_Max o, a
"MAX(#{o.distinct ? 'DISTINCT ' : ''}#{o.expressions.map { |x|
visit x, a }.join(', ')})#{o.alias ? " AS #{o.alias}" : ''}"
end

def visit_Arel_Nodes_Min o, a
"MIN(#{o.distinct ? 'DISTINCT ' : ''}#{o.expressions.map { |x|
visit x, a }.join(', ')})#{o.alias ? " AS #{o.alias}" : ''}"
end

def visit_Arel_Nodes_Avg o, a
"AVG(#{o.distinct ? 'DISTINCT ' : ''}#{o.expressions.map { |x|
visit x, a }.join(', ')})#{o.alias ? " AS #{o.alias}" : ''}"
end
end
end
end
6 changes: 0 additions & 6 deletions test/nodes/test_as.rb
Expand Up @@ -10,12 +10,6 @@ module Nodes
assert_equal attr, as.left
assert_equal 'foo', as.right
end

it 'converts right to SqlLiteral if a string' do
attr = Table.new(:users)[:id]
as = attr.as('foo')
assert_kind_of Arel::Nodes::SqlLiteral, as.right
end
end

describe 'equality' do
Expand Down
2 changes: 1 addition & 1 deletion test/nodes/test_count.rb
Expand Up @@ -11,7 +11,7 @@
it 'should alias the count' do
table = Arel::Table.new :users
table[:id].count.as('foo').to_sql.must_be_like %{
COUNT("users"."id") AS foo
COUNT("users"."id") AS 'foo'
}
end
end
Expand Down
2 changes: 1 addition & 1 deletion test/nodes/test_extract.rb
Expand Up @@ -12,7 +12,7 @@
it 'should alias the extract' do
table = Arel::Table.new :users
table[:timestamp].extract('date').as('foo').to_sql.must_be_like %{
EXTRACT(DATE FROM "users"."timestamp") AS foo
EXTRACT(DATE FROM "users"."timestamp") AS 'foo'
}
end
end
Expand Down
2 changes: 0 additions & 2 deletions test/nodes/test_named_function.rb
Expand Up @@ -14,15 +14,13 @@ def test_function_alias
function = function.as('wth')
assert_equal 'omg', function.name
assert_equal 'zomg', function.expressions
assert_kind_of SqlLiteral, function.alias
assert_equal 'wth', function.alias
end

def test_construct_with_alias
function = NamedFunction.new 'omg', 'zomg', 'wth'
assert_equal 'omg', function.name
assert_equal 'zomg', function.expressions
assert_kind_of SqlLiteral, function.alias
assert_equal 'wth', function.alias
end

Expand Down
2 changes: 1 addition & 1 deletion test/nodes/test_over.rb
Expand Up @@ -5,7 +5,7 @@
it 'should alias the expression' do
table = Arel::Table.new :users
table[:id].count.over.as('foo').to_sql.must_be_like %{
COUNT("users"."id") OVER () AS foo
COUNT("users"."id") OVER () AS 'foo'
}
end
end
Expand Down
2 changes: 1 addition & 1 deletion test/nodes/test_sum.rb
Expand Up @@ -5,7 +5,7 @@
it 'should alias the sum' do
table = Arel::Table.new :users
table[:id].sum.as('foo').to_sql.must_be_like %{
SUM("users"."id") AS foo
SUM("users"."id") AS 'foo'
}
end
end
Expand Down
10 changes: 2 additions & 8 deletions test/test_select_manager.rb
Expand Up @@ -104,12 +104,6 @@ def test_join_sources
assert_equal 'foo', as.right
end

it 'converts right to SqlLiteral if a string' do
manager = Arel::SelectManager.new Table.engine
as = manager.as('foo')
assert_kind_of Arel::Nodes::SqlLiteral, as.right
end

it 'can make a subselect' do
manager = Arel::SelectManager.new Table.engine
manager.project Arel.star
Expand Down Expand Up @@ -281,7 +275,7 @@ def test_join_sources
manager.project SqlLiteral.new '*'
m2 = Arel::SelectManager.new(manager.engine)
m2.project manager.exists.as('foo')
m2.to_sql.must_be_like %{ SELECT EXISTS (#{manager.to_sql}) AS foo }
m2.to_sql.must_be_like %{ SELECT EXISTS (#{manager.to_sql}) AS 'foo' }
end
end

Expand Down Expand Up @@ -667,7 +661,7 @@ def test_join_sources

joins = users.join(counts).on(counts[:user_id].eq(10))
joins.to_sql.must_be_like %{
SELECT FROM "users" INNER JOIN (SELECT "comments"."user_id" AS user_id, COUNT("comments"."user_id") AS count FROM "comments" GROUP BY "comments"."user_id") counts ON counts."user_id" = 10
SELECT FROM "users" INNER JOIN (SELECT "comments"."user_id" AS 'user_id', COUNT("comments"."user_id") AS 'count' FROM "comments" GROUP BY "comments"."user_id") "counts" ON "counts"."user_id" = 10
}
end

Expand Down
51 changes: 51 additions & 0 deletions test/visitors/test_postgres.rb
Expand Up @@ -43,6 +43,57 @@ module Visitors
core.set_quantifier = Arel::Nodes::Distinct.new
assert_equal 'SELECT DISTINCT', @visitor.accept(core)
end

it 'should not quote aliases' do
aliaz = Arel::Nodes::As.new('zomg', 'wth')
assert_equal "'zomg' AS wth", @visitor.accept(aliaz)
end

it 'should not quote table aliases' do
aliaz = Arel::Nodes::TableAlias.new('zomg', 'wth')
assert_equal "'zomg' wth", @visitor.accept(aliaz)
end

it 'should not quote named function aliases' do
func = Arel::Nodes::NamedFunction.new(:max, ['zomg'], 'wth')
assert_equal "max('zomg') AS wth", @visitor.accept(func)
end

it 'should not quote extract aliases' do
extract = Arel::Nodes::Extract.new('zomg', 'hi2u', 'wth')
assert_equal "EXTRACT(HI2U FROM 'zomg') AS wth",
@visitor.accept(extract)
end

it 'should not quote count aliases' do
count = Arel::Nodes::Count.new(['zomg'], false, 'wth')
assert_equal "COUNT('zomg') AS wth", @visitor.accept(count)
end

it 'should not quote sum aliases' do
sum = Arel::Nodes::Sum.new(['zomg'], 'wth')
assert_equal "SUM('zomg') AS wth", @visitor.accept(sum)
end

it 'should not quote exists aliases' do
exists = Arel::Nodes::Exists.new(['zomg'], 'wth')
assert_equal "EXISTS ('zomg') AS wth", @visitor.accept(exists)
end

it 'should not quote max aliases' do
max = Arel::Nodes::Max.new(['zomg'], 'wth')
assert_equal "MAX('zomg') AS wth", @visitor.accept(max)
end

it 'should not quote min aliases' do
min = Arel::Nodes::Min.new(['zomg'], 'wth')
assert_equal "MIN('zomg') AS wth", @visitor.accept(min)
end

it 'should not quote avg aliases' do
avg = Arel::Nodes::Avg.new(['zomg'], 'wth')
assert_equal "AVG('zomg') AS wth", @visitor.accept(avg)
end
end
end
end