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

Deprecate passing a column to type_cast #39106

Merged
merged 1 commit into from
May 2, 2020
Merged
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: 4 additions & 0 deletions activerecord/CHANGELOG.md
@@ -1,3 +1,7 @@
* Deprecate passing a column to `type_cast`.

*Ryuta Kamizono*

* Deprecate `in_clause_length` and `allowed_index_name_length` in `DatabaseLimits`.

*Ryuta Kamizono*
Expand Down
Expand Up @@ -21,7 +21,11 @@ def type_cast(value, column = nil)
value = id_value_for_database(value) if value.is_a?(Base)

if column
value = type_cast_from_column(column, value)
ActiveSupport::Deprecation.warn(<<~MSG.squish)
Passing a column to `type_cast` is deprecated and will be removed in Rails 6.2.
MSG
type = lookup_cast_type_from_column(column)
value = type.serialize(value)
end

_type_cast(value)
Expand All @@ -39,16 +43,6 @@ def type_cast(value, column = nil)
# represent the type doesn't sufficiently reflect the differences
# (varchar vs binary) for example. The type used to get this primitive
# should have been provided before reaching the connection adapter.
def type_cast_from_column(column, value) # :nodoc:
if column
type = lookup_cast_type_from_column(column)
type.serialize(value)
else
value
end
end

# See docs for #type_cast_from_column
def lookup_cast_type_from_column(column) # :nodoc:
lookup_cast_type(column.sql_type)
end
Expand Down Expand Up @@ -193,10 +187,13 @@ def column_name_with_order_matcher # :nodoc:

private
def type_casted_binds(binds)
if binds.first.is_a?(Array)
case binds.first
when ActiveModel::Attribute
binds.map { |attr| type_cast(attr.value_for_database) }
when Array
binds.map { |column, value| type_cast(value, column) }
else
binds.map { |attr| type_cast(attr.value_for_database) }
binds.map { |value| type_cast(value) }
end
end

Expand Down
Expand Up @@ -67,8 +67,8 @@ def quote_default_expression(value, column) # :nodoc:
elsif column.type == :uuid && value.is_a?(String) && /\(\)/.match?(value)
value # Does not quote function default values for UUID columns
elsif column.respond_to?(:array?)
value = type_cast_from_column(column, value)
quote(value)
type = lookup_cast_type_from_column(column)
quote(type.serialize(value))
else
super
end
Expand Down
13 changes: 9 additions & 4 deletions activerecord/lib/active_record/log_subscriber.rb
Expand Up @@ -55,13 +55,18 @@ def type_casted_binds(casted_binds)
end

def render_bind(attr, value)
if attr.is_a?(Array)
case attr
when ActiveModel::Attribute
if attr.type.binary? && attr.value
value = "<#{attr.value_for_database.to_s.bytesize} bytes of binary data>"
end
when Array
attr = attr.first
elsif attr.type.binary? && attr.value
value = "<#{attr.value_for_database.to_s.bytesize} bytes of binary data>"
else
attr = nil
end

[attr && attr.name, value]
[attr&.name, value]
end

def colorize_payload_name(name, payload_name)
Expand Down
57 changes: 44 additions & 13 deletions activerecord/test/cases/adapter_test.rb
Expand Up @@ -312,41 +312,72 @@ def test_select_all_always_return_activerecord_result
end

if ActiveRecord::Base.connection.prepared_statements
def test_select_all_with_legacy_binds
post = Post.create!(title: "foo", body: "bar")
expected = @connection.select_all("SELECT * FROM posts WHERE id = #{post.id}")
result = @connection.select_all("SELECT * FROM posts WHERE id = #{Arel::Nodes::BindParam.new(nil).to_sql}", nil, [[nil, post.id]])
assert_equal expected.to_a, result.to_a
def test_select_all_insert_update_delete_with_legacy_binds
binds = [[Event.column_for_attribute("id"), 1]]
bind_param = Arel::Nodes::BindParam.new(nil)

assert_deprecated do
id = @connection.insert("INSERT INTO events(id) VALUES (#{bind_param.to_sql})", nil, nil, nil, nil, binds)
assert_equal 1, id
end

assert_deprecated do
updated = @connection.update("UPDATE events SET title = 'foo' WHERE id = #{bind_param.to_sql}", nil, binds)
assert_equal 1, updated
end

assert_deprecated do
result = @connection.select_all("SELECT * FROM events WHERE id = #{bind_param.to_sql}", nil, binds)
assert_equal({ "id" => 1, "title" => "foo" }, result.first)
end

assert_deprecated do
deleted = @connection.delete("DELETE FROM events WHERE id = #{bind_param.to_sql}", nil, binds)
assert_equal 1, deleted
end

assert_deprecated do
result = @connection.select_all("SELECT * FROM events WHERE id = #{bind_param.to_sql}", nil, binds)
assert_nil result.first
end
end

def test_insert_update_delete_with_legacy_binds
binds = [[nil, 1]]
def test_select_all_insert_update_delete_with_casted_binds
binds = [Event.type_for_attribute("id").serialize(1)]
bind_param = Arel::Nodes::BindParam.new(nil)

id = @connection.insert("INSERT INTO events(id) VALUES (#{bind_param.to_sql})", nil, nil, nil, nil, binds)
assert_equal 1, id

@connection.update("UPDATE events SET title = 'foo' WHERE id = #{bind_param.to_sql}", nil, binds)
updated = @connection.update("UPDATE events SET title = 'foo' WHERE id = #{bind_param.to_sql}", nil, binds)
assert_equal 1, updated

result = @connection.select_all("SELECT * FROM events WHERE id = #{bind_param.to_sql}", nil, binds)
assert_equal({ "id" => 1, "title" => "foo" }, result.first)

@connection.delete("DELETE FROM events WHERE id = #{bind_param.to_sql}", nil, binds)
deleted = @connection.delete("DELETE FROM events WHERE id = #{bind_param.to_sql}", nil, binds)
assert_equal 1, deleted

result = @connection.select_all("SELECT * FROM events WHERE id = #{bind_param.to_sql}", nil, binds)
assert_nil result.first
end

def test_insert_update_delete_with_binds
binds = [Relation::QueryAttribute.new("id", 1, Type.default_value)]
def test_select_all_insert_update_delete_with_binds
binds = [Relation::QueryAttribute.new("id", 1, Event.type_for_attribute("id"))]
bind_param = Arel::Nodes::BindParam.new(nil)

id = @connection.insert("INSERT INTO events(id) VALUES (#{bind_param.to_sql})", nil, nil, nil, nil, binds)
assert_equal 1, id

@connection.update("UPDATE events SET title = 'foo' WHERE id = #{bind_param.to_sql}", nil, binds)
updated = @connection.update("UPDATE events SET title = 'foo' WHERE id = #{bind_param.to_sql}", nil, binds)
assert_equal 1, updated

result = @connection.select_all("SELECT * FROM events WHERE id = #{bind_param.to_sql}", nil, binds)
assert_equal({ "id" => 1, "title" => "foo" }, result.first)

@connection.delete("DELETE FROM events WHERE id = #{bind_param.to_sql}", nil, binds)
deleted = @connection.delete("DELETE FROM events WHERE id = #{bind_param.to_sql}", nil, binds)
assert_equal 1, deleted

result = @connection.select_all("SELECT * FROM events WHERE id = #{bind_param.to_sql}", nil, binds)
assert_nil result.first
end
Expand Down
4 changes: 3 additions & 1 deletion activerecord/test/cases/bind_parameter_test.rb
Expand Up @@ -159,7 +159,9 @@ def test_logs_binds_after_type_cast

def test_logs_legacy_binds_after_type_cast
binds = [[@pk, "10"]]
assert_logs_binds(binds)
assert_deprecated do
assert_logs_binds(binds)
end
end

private
Expand Down