Skip to content

Commit

Permalink
Specified column type for quote_value
Browse files Browse the repository at this point in the history
When calling quote_value the underlying connection sometimes requires
more information about the column to properly return the correct quoted
value.

I ran into this issue when using optimistic locking in JRuby and the
activerecord-jdbcmssql-adapter. In SQLSever 2000, we aren't allowed to
insert a integer into a NVARCHAR column type so we need to format it as
N'3' if we want to insert into the NVARCHAR type. Unfortuantely, without
the column type being passed the connection adapter cannot properly return
the correct quote value because it doesn't know to return N'3' or '3'.

This patch is fairly straight forward where it just passes in the column
type into the quote_value, as it already has the ability to take in the column,
so it can properly handle at the connection level.

I've added the tests required to make sure that the quote_value method
is being passed the column type so that the underlying connection can
determine how to quote the value.

Conflicts:
	activerecord/CHANGELOG.md
	activerecord/lib/active_record/locking/optimistic.rb
  • Loading branch information
alfredw authored and rafaelfranca committed Jul 24, 2013
1 parent c4b93f5 commit 33e1604
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 1 deletion.
10 changes: 10 additions & 0 deletions activerecord/CHANGELOG.md
@@ -1,3 +1,13 @@
## unreleased ##

* When using optimisitc locking, `update` whas not passing the column type to `quote_value`
to allow the connection adapter to properly determine how to quote the value. This was
affecting certain databases that use specific colmn types.
Fix #6763

*Alfred Wong*


## Rails 3.2.14 (Jul 22, 2013) ## ## Rails 3.2.14 (Jul 22, 2013) ##


* Fix merge error when Equality LHS is non-attribute. * Fix merge error when Equality LHS is non-attribute.
Expand Down
2 changes: 1 addition & 1 deletion activerecord/lib/active_record/locking/optimistic.rb
Expand Up @@ -80,7 +80,7 @@ def update(attribute_names = @attributes.keys) #:nodoc:


stmt = relation.where( stmt = relation.where(
relation.table[self.class.primary_key].eq(id).and( relation.table[self.class.primary_key].eq(id).and(
relation.table[lock_col].eq(quote_value(previous_lock_value)) relation.table[lock_col].eq(quote_value(previous_lock_value, self.class.columns_hash[lock_col]))
) )
).arel.compile_update(arel_attributes_values(false, false, attribute_names)) ).arel.compile_update(arel_attributes_values(false, false, attribute_names))


Expand Down
13 changes: 13 additions & 0 deletions activerecord/test/cases/locking_test.rb
@@ -1,4 +1,5 @@
require 'thread' require 'thread'
require 'mocha/setup'
require "cases/helper" require "cases/helper"
require 'models/person' require 'models/person'
require 'models/job' require 'models/job'
Expand All @@ -23,6 +24,18 @@ class ReadonlyNameShip < Ship
class OptimisticLockingTest < ActiveRecord::TestCase class OptimisticLockingTest < ActiveRecord::TestCase
fixtures :people, :legacy_things, :references, :string_key_objects, :peoples_treasures fixtures :people, :legacy_things, :references, :string_key_objects, :peoples_treasures


def test_quote_value_passed_lock_col
p1 = Person.find(1)
assert_equal 0, p1.lock_version

Person.expects(:quote_value).with(0, Person.columns_hash[Person.locking_column]).returns('0').once

p1.first_name = 'anika2'
p1.save!

assert_equal 1, p1.lock_version
end

def test_non_integer_lock_existing def test_non_integer_lock_existing
s1 = StringKeyObject.find("record1") s1 = StringKeyObject.find("record1")
s2 = StringKeyObject.find("record1") s2 = StringKeyObject.find("record1")
Expand Down

0 comments on commit 33e1604

Please sign in to comment.