kotovalexarian-likes-github/rails--rails
1
0
Fork 0
mirror of https://github.com/rails/rails.git synced 2022-11-09 12:12:34 -05:00
Code Releases Activity

Add a value field Nodes::BindParam

Browse source 
This is part of a greater refactoring to have the `BindParam` nodes hold
onto their values. We want to generally keep the AST decoupled from what
you're actually doing with those values, but ultimately the usage of
`BindParam` is almost identical to how you'd use `Casted` or `Quoted`.
Forcing consumers of Arel's API to maintain the bind values separately
from the AST makes manipulating the AST essentially impossible, as you
would need to perform a full walk of the AST to determine whether a
given node contains bind parameters, and which value it maps to.

By storing the value on the bind parameter directly, we can collect them
in another AST pass (realistically it'll be part of the same pass that
performs SQL construction for performance reasons). This will
dramatically simplify AST manipulation for Rails or any other consumers
that work with bind params.

As part of this change I've removed the `BindVisitor`, which appears to
be dead code, and had tests break from this change.
This commit is contained in:
Sean Griffin 2017-07-21 08:33:09 -04:00
parent f031a3b9aa
commit db1bb4e9a7
14 changed files with 37 additions and 125 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
arel.gemspec
View file

@ -17,7 +17,7 @@ Gem::Specification.new do |s|
s.rdoc_options = ["--main", "README.md"]
s.extra_rdoc_files = ["History.txt", "MIT-LICENSE.txt", "README.md"]
s.files = ["History.txt","MIT-LICENSE.txt","README.md","lib/arel.rb","lib/arel/alias_predication.rb","lib/arel/attributes.rb","lib/arel/attributes/attribute.rb","lib/arel/collectors/substitute_binds.rb","lib/arel/collectors/plain_string.rb","lib/arel/collectors/sql_string.rb","lib/arel/compatibility/wheres.rb","lib/arel/crud.rb","lib/arel/delete_manager.rb","lib/arel/errors.rb","lib/arel/expressions.rb","lib/arel/factory_methods.rb","lib/arel/insert_manager.rb","lib/arel/math.rb","lib/arel/nodes.rb","lib/arel/nodes/and.rb","lib/arel/nodes/ascending.rb","lib/arel/nodes/binary.rb","lib/arel/nodes/bind_param.rb","lib/arel/nodes/case.rb","lib/arel/nodes/casted.rb","lib/arel/nodes/count.rb","lib/arel/nodes/delete_statement.rb","lib/arel/nodes/descending.rb","lib/arel/nodes/equality.rb","lib/arel/nodes/extract.rb","lib/arel/nodes/false.rb","lib/arel/nodes/full_outer_join.rb","lib/arel/nodes/function.rb","lib/arel/nodes/grouping.rb","lib/arel/nodes/in.rb","lib/arel/nodes/infix_operation.rb","lib/arel/nodes/inner_join.rb","lib/arel/nodes/insert_statement.rb","lib/arel/nodes/join_source.rb","lib/arel/nodes/matches.rb","lib/arel/nodes/named_function.rb","lib/arel/nodes/node.rb","lib/arel/nodes/outer_join.rb","lib/arel/nodes/over.rb","lib/arel/nodes/regexp.rb","lib/arel/nodes/right_outer_join.rb","lib/arel/nodes/select_core.rb","lib/arel/nodes/select_statement.rb","lib/arel/nodes/sql_literal.rb","lib/arel/nodes/string_join.rb","lib/arel/nodes/table_alias.rb","lib/arel/nodes/terminal.rb","lib/arel/nodes/true.rb","lib/arel/nodes/unary.rb","lib/arel/nodes/unary_operation.rb","lib/arel/nodes/unqualified_column.rb","lib/arel/nodes/update_statement.rb","lib/arel/nodes/values.rb","lib/arel/nodes/window.rb","lib/arel/nodes/with.rb","lib/arel/order_predications.rb","lib/arel/predications.rb","lib/arel/select_manager.rb","lib/arel/table.rb","lib/arel/tree_manager.rb","lib/arel/update_manager.rb","lib/arel/visitors.rb","lib/arel/visitors/bind_visitor.rb","lib/arel/visitors/depth_first.rb","lib/arel/visitors/dot.rb","lib/arel/visitors/ibm_db.rb","lib/arel/visitors/informix.rb","lib/arel/visitors/mssql.rb","lib/arel/visitors/mysql.rb","lib/arel/visitors/oracle.rb","lib/arel/visitors/oracle12.rb","lib/arel/visitors/postgresql.rb","lib/arel/visitors/reduce.rb","lib/arel/visitors/sqlite.rb","lib/arel/visitors/to_sql.rb","lib/arel/visitors/visitor.rb","lib/arel/visitors/where_sql.rb","lib/arel/window_predications.rb"]
s.files = ["History.txt","MIT-LICENSE.txt","README.md","lib/arel.rb","lib/arel/alias_predication.rb","lib/arel/attributes.rb","lib/arel/attributes/attribute.rb","lib/arel/collectors/substitute_binds.rb","lib/arel/collectors/plain_string.rb","lib/arel/collectors/sql_string.rb","lib/arel/compatibility/wheres.rb","lib/arel/crud.rb","lib/arel/delete_manager.rb","lib/arel/errors.rb","lib/arel/expressions.rb","lib/arel/factory_methods.rb","lib/arel/insert_manager.rb","lib/arel/math.rb","lib/arel/nodes.rb","lib/arel/nodes/and.rb","lib/arel/nodes/ascending.rb","lib/arel/nodes/binary.rb","lib/arel/nodes/bind_param.rb","lib/arel/nodes/case.rb","lib/arel/nodes/casted.rb","lib/arel/nodes/count.rb","lib/arel/nodes/delete_statement.rb","lib/arel/nodes/descending.rb","lib/arel/nodes/equality.rb","lib/arel/nodes/extract.rb","lib/arel/nodes/false.rb","lib/arel/nodes/full_outer_join.rb","lib/arel/nodes/function.rb","lib/arel/nodes/grouping.rb","lib/arel/nodes/in.rb","lib/arel/nodes/infix_operation.rb","lib/arel/nodes/inner_join.rb","lib/arel/nodes/insert_statement.rb","lib/arel/nodes/join_source.rb","lib/arel/nodes/matches.rb","lib/arel/nodes/named_function.rb","lib/arel/nodes/node.rb","lib/arel/nodes/outer_join.rb","lib/arel/nodes/over.rb","lib/arel/nodes/regexp.rb","lib/arel/nodes/right_outer_join.rb","lib/arel/nodes/select_core.rb","lib/arel/nodes/select_statement.rb","lib/arel/nodes/sql_literal.rb","lib/arel/nodes/string_join.rb","lib/arel/nodes/table_alias.rb","lib/arel/nodes/terminal.rb","lib/arel/nodes/true.rb","lib/arel/nodes/unary.rb","lib/arel/nodes/unary_operation.rb","lib/arel/nodes/unqualified_column.rb","lib/arel/nodes/update_statement.rb","lib/arel/nodes/values.rb","lib/arel/nodes/window.rb","lib/arel/nodes/with.rb","lib/arel/order_predications.rb","lib/arel/predications.rb","lib/arel/select_manager.rb","lib/arel/table.rb","lib/arel/tree_manager.rb","lib/arel/update_manager.rb","lib/arel/visitors.rb","lib/arel/visitors/depth_first.rb","lib/arel/visitors/dot.rb","lib/arel/visitors/ibm_db.rb","lib/arel/visitors/informix.rb","lib/arel/visitors/mssql.rb","lib/arel/visitors/mysql.rb","lib/arel/visitors/oracle.rb","lib/arel/visitors/oracle12.rb","lib/arel/visitors/postgresql.rb","lib/arel/visitors/reduce.rb","lib/arel/visitors/sqlite.rb","lib/arel/visitors/to_sql.rb","lib/arel/visitors/visitor.rb","lib/arel/visitors/where_sql.rb","lib/arel/window_predications.rb"]
s.require_paths = ["lib"]
s.add_development_dependency('minitest', '~> 5.4')

2
lib/arel/collectors/plain_string.rb
View file

@ -3,7 +3,7 @@ module Arel
module Collectors
class PlainString
def initialize
@str = ''.dup
@str = String.new
end
def value

10
lib/arel/nodes/bind_param.rb
View file

@ -2,8 +2,16 @@
module Arel
module Nodes
class BindParam < Node
attr_reader :value
def initialize(value)
@value = value
super()
end
def ==(other)
other.is_a?(BindParam)
other.is_a?(BindParam) &&
value == other.value
end
end
end

40
lib/arel/visitors/bind_visitor.rb
View file

@ -1,40 +0,0 @@
# frozen_string_literal: true
module Arel
module Visitors
module BindVisitor
def initialize target
@block = nil
super
end
def accept node, collector, &block
@block = block if block_given?
super
end
private
def visit_Arel_Nodes_Assignment o, collector
if o.right.is_a? Arel::Nodes::BindParam
collector = visit o.left, collector
collector << " = "
visit o.right, collector
else
super
end
end
def visit_Arel_Nodes_BindParam o, collector
if @block
val = @block.call
if String === val
collector << val
end
else
super
end
end
end
end
end

2
test/collectors/test_sql_string.rb
View file

@ -27,7 +27,7 @@ module Arel
end
def test_compile
bv = Nodes::BindParam.new
bv = Nodes::BindParam.new(nil)
collector = collect ast_with_binds bv
sql = collector.compile ["hello", "world"]

10
test/collectors/test_bind_collector.rb → test/collectors/test_substitute_bind_collector.rb
View file

@ -4,7 +4,7 @@ require 'arel/collectors/substitute_binds'
module Arel
module Collectors
class TestBindCollector < Arel::Test
class TestSubstituteBindCollector < Arel::Test
def setup
@conn = FakeRecord::Base.new
@visitor = Visitors::ToSql.new @conn.connection
@ -28,14 +28,14 @@ module Arel
end
def test_leaves_binds
node = Nodes::BindParam.new
node = Nodes::BindParam.new(nil)
list = compile node
assert_equal node, list.first
assert_equal node.class, list.first.class
end
def test_adds_strings
bv = Nodes::BindParam.new
bv = Nodes::BindParam.new(nil)
list = compile ast_with_binds bv
assert_operator list.length, :>, 0
assert_equal bv, list.grep(Nodes::BindParam).first
@ -43,7 +43,7 @@ module Arel
end
def test_substitute_binds
bv = Nodes::BindParam.new
bv = Nodes::BindParam.new(nil)
collector = collect ast_with_binds bv
values = collector.value
@ -60,7 +60,7 @@ module Arel
end
def test_compile
bv = Nodes::BindParam.new
bv = Nodes::BindParam.new(nil)
collector = collect ast_with_binds bv
sql = collector.compile ["hello", "world"]

11
test/nodes/test_bind_param.rb
View file

@ -4,12 +4,17 @@ require 'helper'
module Arel
module Nodes
describe 'BindParam' do
it 'is equal to other bind params' do
BindParam.new.must_equal(BindParam.new)
it 'is equal to other bind params with the same value' do
BindParam.new(1).must_equal(BindParam.new(1))
BindParam.new("foo").must_equal(BindParam.new("foo"))
end
it 'is not equal to other nodes' do
BindParam.new.wont_equal(Node.new)
BindParam.new(nil).wont_equal(Node.new)
end
it 'is not equal to bind params with different values' do
BindParam.new(1).wont_equal(BindParam.new(2))
end
end
end

2
test/test_update_manager.rb
View file

@ -13,7 +13,7 @@ module Arel
table = Table.new(:users)
um = Arel::UpdateManager.new
um.table table
um.set [[table[:name], Arel::Nodes::BindParam.new]]
um.set [[table[:name], Arel::Nodes::BindParam.new(nil)]]
um.to_sql.must_be_like %{ UPDATE "users" SET "name" = ? }
end

61
test/visitors/test_bind_visitor.rb
View file

@ -1,61 +0,0 @@
# frozen_string_literal: true
require 'helper'
require 'arel/visitors/bind_visitor'
require 'support/fake_record'
module Arel
module Visitors
class TestBindVisitor < Arel::Test
attr_reader :collector
def setup
@collector = Collectors::SQLString.new
super
end
##
# Tests visit_Arel_Nodes_Assignment correctly
# substitutes binds with values from block
def test_assignment_binds_are_substituted
table = Table.new(:users)
um = Arel::UpdateManager.new
bp = Nodes::BindParam.new
um.set [[table[:name], bp]]
visitor = Class.new(Arel::Visitors::ToSql) {
include Arel::Visitors::BindVisitor
}.new Table.engine.connection
assignment = um.ast.values[0]
actual = visitor.accept(assignment, collector) {
"replace"
}
assert actual
value = actual.value
assert_like "\"name\" = replace", value
end
def test_visitor_yields_on_binds
visitor = Class.new(Arel::Visitors::ToSql) {
include Arel::Visitors::BindVisitor
}.new nil
bp = Nodes::BindParam.new
called = false
visitor.accept(bp, collector) { called = true }
assert called
end
def test_visitor_only_yields_on_binds
visitor = Class.new(Arel::Visitors::ToSql) {
include Arel::Visitors::BindVisitor
}.new(nil)
bp = Arel.sql 'omg'
called = false
visitor.accept(bp, collector) { called = true }
refute called
end
end
end
end

2
test/visitors/test_dot.rb
View file

@ -74,7 +74,7 @@ module Arel
end
def test_Arel_Nodes_BindParam
node = Arel::Nodes::BindParam.new
node = Arel::Nodes::BindParam.new(nil)
collector = Collectors::PlainString.new
assert_match '[label="<f0>Arel::Nodes::BindParam"]', @visitor.accept(node, collector).value
end

8
test/visitors/test_oracle.rb
View file

@ -127,8 +127,8 @@ module Arel
it 'creates a subquery when there is limit and offset with BindParams' do
stmt = Nodes::SelectStatement.new
stmt.limit = Nodes::Limit.new(Nodes::BindParam.new)
stmt.offset = Nodes::Offset.new(Nodes::BindParam.new)
stmt.limit = Nodes::Limit.new(Nodes::BindParam.new(nil))
stmt.offset = Nodes::Offset.new(Nodes::BindParam.new(nil))
sql = compile stmt
sql.must_be_like %{
SELECT * FROM (
@ -184,8 +184,8 @@ module Arel
describe "Nodes::BindParam" do
it "increments each bind param" do
query = @table[:name].eq(Arel::Nodes::BindParam.new)
.and(@table[:id].eq(Arel::Nodes::BindParam.new))
query = @table[:name].eq(Arel::Nodes::BindParam.new(nil))
.and(@table[:id].eq(Arel::Nodes::BindParam.new(nil)))
compile(query).must_be_like %{
"users"."name" = :a1 AND "users"."id" = :a2
}

4
test/visitors/test_oracle12.rb
View file

@ -48,8 +48,8 @@ module Arel
describe "Nodes::BindParam" do
it "increments each bind param" do
query = @table[:name].eq(Arel::Nodes::BindParam.new)
.and(@table[:id].eq(Arel::Nodes::BindParam.new))
query = @table[:name].eq(Arel::Nodes::BindParam.new(nil))
.and(@table[:id].eq(Arel::Nodes::BindParam.new(nil)))
compile(query).must_be_like %{
"users"."name" = :a1 AND "users"."id" = :a2
}

4
test/visitors/test_postgres.rb
View file

@ -176,8 +176,8 @@ module Arel
describe "Nodes::BindParam" do
it "increments each bind param" do
query = @table[:name].eq(Arel::Nodes::BindParam.new)
.and(@table[:id].eq(Arel::Nodes::BindParam.new))
query = @table[:name].eq(Arel::Nodes::BindParam.new(nil))
.and(@table[:id].eq(Arel::Nodes::BindParam.new(nil)))
compile(query).must_be_like %{
"users"."name" = $1 AND "users"."id" = $2
}

4
test/visitors/test_to_sql.rb
View file

@ -17,13 +17,13 @@ module Arel
end
it 'works with BindParams' do
node = Nodes::BindParam.new
node = Nodes::BindParam.new(nil)
sql = compile node
sql.must_be_like '?'
end
it 'does not quote BindParams used as part of a Values' do
bp = Nodes::BindParam.new
bp = Nodes::BindParam.new(nil)
values = Nodes::Values.new([bp])
sql = compile values
sql.must_be_like 'VALUES (?)'