Skip to content

Commit 93c9a95

Browse files
sgrifsgrif
committed
Revert "Extract bind_param and bind_attribute into ActiveRecord::TestCase"
This reverts commit b6ad405. This is not something that the majority of Active Record should be testing or care about. We should look at having fewer places rely on these details, not make it easier to rely on them.
1 parent 84eb498 commit 93c9a95

13 files changed

+56
-46
lines changed

‎activerecord/test/cases/adapter_test.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -242,7 +242,7 @@ def test_select_all_always_return_activerecord_result
242242
def test_select_all_with_legacy_binds
243243
post = Post.create!(title: "foo", body: "bar")
244244
expected = @connection.select_all("SELECT * FROM posts WHERE id = #{post.id}")
245-
result = @connection.select_all("SELECT * FROM posts WHERE id = #{bind_param.to_sql}", nil, [[nil, post.id]])
245+
result = @connection.select_all("SELECT * FROM posts WHERE id = #{Arel::Nodes::BindParam.new.to_sql}", nil, [[nil, post.id]])
246246
assert_equal expected.to_hash, result.to_hash
247247
end
248248
end

‎activerecord/test/cases/adapters/postgresql/connection_test.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -133,8 +133,8 @@ def test_schema_names_logs_name
133133

134134
if ActiveRecord::Base.connection.prepared_statements
135135
def test_statement_key_is_logged
136-
binds = [bind_attribute(nil, 1)]
137-
@connection.exec_query("SELECT $1::integer", "SQL", binds, prepare: true)
136+
bind = Relation::QueryAttribute.new(nil, 1, Type::Value.new)
137+
@connection.exec_query("SELECT $1::integer", "SQL", [bind], prepare: true)
138138
name = @subscriber.payloads.last[:statement_name]
139139
assert name
140140
res = @connection.exec_query("EXPLAIN (FORMAT JSON) EXECUTE #{name}(1)")

‎activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -202,8 +202,8 @@ def test_exec_with_binds
202202
string = @connection.quote("foo")
203203
@connection.exec_query("INSERT INTO ex (id, data) VALUES (1, #{string})")
204204

205-
binds = [bind_attribute("id", 1)]
206-
result = @connection.exec_query("SELECT id, data FROM ex WHERE id = $1", nil, binds)
205+
bind = Relation::QueryAttribute.new("id", 1, Type::Value.new)
206+
result = @connection.exec_query("SELECT id, data FROM ex WHERE id = $1", nil, [bind])
207207

208208
assert_equal 1, result.rows.length
209209
assert_equal 2, result.columns.length
@@ -217,8 +217,8 @@ def test_exec_typecasts_bind_vals
217217
string = @connection.quote("foo")
218218
@connection.exec_query("INSERT INTO ex (id, data) VALUES (1, #{string})")
219219

220-
binds = [bind_attribute("id", "1-fuu", Type::Integer.new)]
221-
result = @connection.exec_query("SELECT id, data FROM ex WHERE id = $1", nil, binds)
220+
bind = Relation::QueryAttribute.new("id", "1-fuu", Type::Integer.new)
221+
result = @connection.exec_query("SELECT id, data FROM ex WHERE id = $1", nil, [bind])
222222

223223
assert_equal 1, result.rows.length
224224
assert_equal 2, result.columns.length

‎activerecord/test/cases/adapters/postgresql/schema_authorization_test.rb

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ def test_auth_with_bind
6868
USERS.each do |u|
6969
@connection.clear_cache!
7070
set_session_auth u
71-
assert_equal u, @connection.select_value("SELECT name FROM #{TABLE_NAME} WHERE id = $1", "SQL", [bind_attribute("id", 1)])
71+
assert_equal u, @connection.select_value("SELECT name FROM #{TABLE_NAME} WHERE id = $1", "SQL", [bind_param(1)])
7272
set_session_auth
7373
end
7474
end
@@ -101,4 +101,8 @@ def test_tables_in_current_schemas
101101
def set_session_auth(auth = nil)
102102
@connection.session_auth = auth || "default"
103103
end
104+
105+
def bind_param(value)
106+
ActiveRecord::Relation::QueryAttribute.new(nil, value, ActiveRecord::Type::Value.new)
107+
end
104108
end

‎activerecord/test/cases/adapters/postgresql/schema_test.rb

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -169,17 +169,17 @@ def test_drop_schema_with_nonexisting_schema
169169

170170
def test_raise_wrapped_exception_on_bad_prepare
171171
assert_raises(ActiveRecord::StatementInvalid) do
172-
@connection.exec_query "select * from developers where id = ?", "sql", [bind_attribute("id", 1)]
172+
@connection.exec_query "select * from developers where id = ?", "sql", [bind_param(1)]
173173
end
174174
end
175175

176176
if ActiveRecord::Base.connection.prepared_statements
177177
def test_schema_change_with_prepared_stmt
178178
altered = false
179-
@connection.exec_query "select * from developers where id = $1", "sql", [bind_attribute("id", 1)]
179+
@connection.exec_query "select * from developers where id = $1", "sql", [bind_param(1)]
180180
@connection.exec_query "alter table developers add column zomg int", "sql", []
181181
altered = true
182-
@connection.exec_query "select * from developers where id = $1", "sql", [bind_attribute("id", 1)]
182+
@connection.exec_query "select * from developers where id = $1", "sql", [bind_param(1)]
183183
ensure
184184
# We are not using DROP COLUMN IF EXISTS because that syntax is only
185185
# supported by pg 9.X
@@ -467,6 +467,10 @@ def do_dump_index_assertions_for_one_index(this_index, this_index_name, this_ind
467467
assert_equal this_index_column, this_index.columns[0]
468468
assert_equal this_index_name, this_index.name
469469
end
470+
471+
def bind_param(value)
472+
ActiveRecord::Relation::QueryAttribute.new(nil, value, ActiveRecord::Type::Value.new)
473+
end
470474
end
471475

472476
class SchemaForeignKeyTest < ActiveRecord::PostgreSQLTestCase

‎activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -66,11 +66,11 @@ def test_column_types
6666

6767
def test_exec_insert
6868
with_example_table do
69-
binds = [bind_attribute("number", 10)]
70-
@conn.exec_insert("insert into ex (number) VALUES (?)", "SQL", binds)
69+
vals = [Relation::QueryAttribute.new("number", 10, Type::Value.new)]
70+
@conn.exec_insert("insert into ex (number) VALUES (?)", "SQL", vals)
7171

7272
result = @conn.exec_query(
73-
"select number from ex where number = ?", "SQL", binds)
73+
"select number from ex where number = ?", "SQL", vals)
7474

7575
assert_equal 1, result.rows.length
7676
assert_equal 10, result.rows.first.first
@@ -134,7 +134,7 @@ def test_exec_query_with_binds
134134
with_example_table "id int, data string" do
135135
@conn.exec_query('INSERT INTO ex (id, data) VALUES (1, "foo")')
136136
result = @conn.exec_query(
137-
"SELECT id, data FROM ex WHERE id = ?", nil, [bind_attribute("id", 1)])
137+
"SELECT id, data FROM ex WHERE id = ?", nil, [Relation::QueryAttribute.new(nil, 1, Type::Value.new)])
138138

139139
assert_equal 1, result.rows.length
140140
assert_equal 2, result.columns.length
@@ -148,7 +148,7 @@ def test_exec_query_typecasts_bind_vals
148148
@conn.exec_query('INSERT INTO ex (id, data) VALUES (1, "foo")')
149149

150150
result = @conn.exec_query(
151-
"SELECT id, data FROM ex WHERE id = ?", nil, [bind_attribute("id", "1-fuu", Type::Integer.new)])
151+
"SELECT id, data FROM ex WHERE id = ?", nil, [Relation::QueryAttribute.new("id", "1-fuu", Type::Integer.new)])
152152

153153
assert_equal 1, result.rows.length
154154
assert_equal 2, result.columns.length

‎activerecord/test/cases/bind_parameter_test.rb

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,9 @@ def test_bind_from_join_in_subquery
3939
end
4040

4141
def test_binds_are_logged
42-
binds = [bind_attribute("id", 1)]
43-
sql = "select * from topics where id = #{bind_param.to_sql}"
42+
sub = Arel::Nodes::BindParam.new
43+
binds = [Relation::QueryAttribute.new("id", 1, Type::Value.new)]
44+
sql = "select * from topics where id = #{sub.to_sql}"
4445

4546
@connection.exec_query(sql, "SQL", binds)
4647

@@ -55,7 +56,7 @@ def test_find_one_uses_binds
5556
end
5657

5758
def test_logs_binds_after_type_cast
58-
binds = [bind_attribute("id", "10", Type::Integer.new)]
59+
binds = [Relation::QueryAttribute.new("id", "10", Type::Integer.new)]
5960
assert_logs_binds(binds)
6061
end
6162

‎activerecord/test/cases/explain_test.rb

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ def test_exec_explain_with_no_binds
4747

4848
def test_exec_explain_with_binds
4949
sqls = %w(foo bar)
50-
binds = [[bind_attribute("wadus", 1)], [bind_attribute("chaflan", 2)]]
50+
binds = [[bind_param("wadus", 1)], [bind_param("chaflan", 2)]]
5151
queries = sqls.zip(binds)
5252

5353
stub_explain_for_query_plans(["query plan foo\n", "query plan bar\n"]) do
@@ -79,5 +79,9 @@ def stub_explain_for_query_plans(query_plans = ["query plan foo", "query plan ba
7979
yield
8080
end
8181
end
82+
83+
def bind_param(name, value)
84+
ActiveRecord::Relation::QueryAttribute.new(name, value, ActiveRecord::Type::Value.new)
85+
end
8286
end
8387
end

‎activerecord/test/cases/inheritance_test.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -418,6 +418,7 @@ def test_alt_eager_loading
418418

419419
def test_eager_load_belongs_to_primary_key_quoting
420420
con = Account.connection
421+
bind_param = Arel::Nodes::BindParam.new
421422
assert_sql(/#{con.quote_table_name('companies')}\.#{con.quote_column_name('id')} = (?:#{Regexp.quote(bind_param.to_sql)}|1)/) do
422423
Account.all.merge!(includes: :firm).find(1)
423424
end

‎activerecord/test/cases/relation/where_chain_test.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ def test_not_with_nil
2525
end
2626

2727
def test_association_not_eq
28-
expected = Arel::Nodes::Grouping.new(Comment.arel_table[@name].not_eq(bind_param))
28+
expected = Arel::Nodes::Grouping.new(Comment.arel_table[@name].not_eq(Arel::Nodes::BindParam.new))
2929
relation = Post.joins(:comments).where.not(comments: { title: "hello" })
3030
assert_equal(expected.to_sql, relation.where_clause.ast.to_sql)
3131
end

‎activerecord/test/cases/relation/where_clause_test.rb

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -47,15 +47,15 @@ class WhereClauseTest < ActiveRecord::TestCase
4747
test "merge removes bind parameters matching overlapping equality clauses" do
4848
a = WhereClause.new(
4949
[table["id"].eq(bind_param), table["name"].eq(bind_param)],
50-
[bind_attribute("id", 1), bind_attribute("name", "Sean")],
50+
[attribute("id", 1), attribute("name", "Sean")],
5151
)
5252
b = WhereClause.new(
5353
[table["name"].eq(bind_param)],
54-
[bind_attribute("name", "Jim")]
54+
[attribute("name", "Jim")]
5555
)
5656
expected = WhereClause.new(
5757
[table["id"].eq(bind_param), table["name"].eq(bind_param)],
58-
[bind_attribute("id", 1), bind_attribute("name", "Jim")],
58+
[attribute("id", 1), attribute("name", "Jim")],
5959
)
6060

6161
assert_equal expected, a.merge(b)
@@ -103,17 +103,17 @@ class WhereClauseTest < ActiveRecord::TestCase
103103
table["name"].eq(bind_param),
104104
table["age"].gteq(bind_param),
105105
], [
106-
bind_attribute("name", "Sean"),
107-
bind_attribute("age", 30),
106+
attribute("name", "Sean"),
107+
attribute("age", 30),
108108
])
109-
expected = WhereClause.new([table["age"].gteq(bind_param)], [bind_attribute("age", 30)])
109+
expected = WhereClause.new([table["age"].gteq(bind_param)], [attribute("age", 30)])
110110

111111
assert_equal expected, where_clause.except("id", "name")
112112
end
113113

114114
test "except jumps over unhandled binds (like with OR) correctly" do
115115
wcs = (0..9).map do |i|
116-
WhereClause.new([table["id#{i}"].eq(bind_param)], [bind_attribute("id#{i}", i)])
116+
WhereClause.new([table["id#{i}"].eq(bind_param)], [attribute("id#{i}", i)])
117117
end
118118

119119
wc = wcs[0] + wcs[1] + wcs[2].or(wcs[3]) + wcs[4] + wcs[5] + wcs[6].or(wcs[7]) + wcs[8] + wcs[9]
@@ -162,8 +162,8 @@ class WhereClauseTest < ActiveRecord::TestCase
162162
end
163163

164164
test "or joins the two clauses using OR" do
165-
where_clause = WhereClause.new([table["id"].eq(bind_param)], [bind_attribute("id", 1)])
166-
other_clause = WhereClause.new([table["name"].eq(bind_param)], [bind_attribute("name", "Sean")])
165+
where_clause = WhereClause.new([table["id"].eq(bind_param)], [attribute("id", 1)])
166+
other_clause = WhereClause.new([table["name"].eq(bind_param)], [attribute("name", "Sean")])
167167
expected_ast =
168168
Arel::Nodes::Grouping.new(
169169
Arel::Nodes::Or.new(table["id"].eq(bind_param), table["name"].eq(bind_param))
@@ -175,7 +175,7 @@ class WhereClauseTest < ActiveRecord::TestCase
175175
end
176176

177177
test "or returns an empty where clause when either side is empty" do
178-
where_clause = WhereClause.new([table["id"].eq(bind_param)], [bind_attribute("id", 1)])
178+
where_clause = WhereClause.new([table["id"].eq(bind_param)], [attribute("id", 1)])
179179

180180
assert_equal WhereClause.empty, where_clause.or(WhereClause.empty)
181181
assert_equal WhereClause.empty, WhereClause.empty.or(where_clause)
@@ -186,5 +186,13 @@ class WhereClauseTest < ActiveRecord::TestCase
186186
def table
187187
Arel::Table.new("table")
188188
end
189+
190+
def bind_param
191+
Arel::Nodes::BindParam.new
192+
end
193+
194+
def attribute(name, value)
195+
ActiveRecord::Attribute.with_cast_value(name, value, ActiveRecord::Type::Value.new)
196+
end
189197
end
190198
end

‎activerecord/test/cases/relations_test.rb

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1853,8 +1853,7 @@ def test_unscope_specific_where_value
18531853
def test_unscope_removes_binds
18541854
left = Post.where(id: 20)
18551855

1856-
binds = [bind_attribute("id", 20, Post.type_for_attribute("id"))]
1857-
assert_equal binds, left.bound_attributes
1856+
assert_equal 1, left.bound_attributes.length
18581857

18591858
relation = left.unscope(where: :id)
18601859
assert_equal [], relation.bound_attributes
@@ -1864,21 +1863,18 @@ def test_merging_removes_rhs_binds
18641863
left = Post.where(id: 20)
18651864
right = Post.where(id: [1, 2, 3, 4])
18661865

1867-
binds = [bind_attribute("id", 20, Post.type_for_attribute("id"))]
1868-
assert_equal binds, left.bound_attributes
1866+
assert_equal 1, left.bound_attributes.length
18691867

18701868
merged = left.merge(right)
18711869
assert_equal [], merged.bound_attributes
18721870
end
18731871

18741872
def test_merging_keeps_lhs_binds
1875-
binds = [bind_attribute("id", 20, Post.type_for_attribute("id"))]
1876-
18771873
right = Post.where(id: 20)
18781874
left = Post.where(id: 10)
18791875

18801876
merged = left.merge(right)
1881-
assert_equal binds, merged.bound_attributes
1877+
assert_equal [20], merged.bound_attributes.map(&:value)
18821878
end
18831879

18841880
def test_locked_should_not_build_arel

‎activerecord/test/cases/test_case.rb

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -75,14 +75,6 @@ def has_column?(model, column_name)
7575
model.reset_column_information
7676
model.column_names.include?(column_name.to_s)
7777
end
78-
79-
def bind_param
80-
Arel::Nodes::BindParam.new
81-
end
82-
83-
def bind_attribute(name, value, type = ActiveRecord::Type.default_value)
84-
ActiveRecord::Relation::QueryAttribute.new(name, value, type)
85-
end
8678
end
8779

8880
class PostgreSQLTestCase < TestCase

0 commit comments

Comments
 (0)