ActiveRecord typecasting with overwritten attribute #5549

Closed
cbailster opened this Issue Mar 22, 2012 · 11 comments

Comments

Projects
None yet
5 participants

Not sure if this is an issue or by design but there seems to be a change in the way attributes are handled when they are overwritten in Rails 3.2 vs 3.1 (specifically 3.2.2 vs 3.1.1).

I have a model called Entity. model has an attribute called has_image (stored in a MySQL database as tinyint(1) not null)

without any overwriting (or in rails 3.1.1) in rails console:

> e = Entity.find(1)
> e.has_image
=> false
> e[:has_image]
=> false
> e.read_attribute(:has_image)
=> false

but if I add the following code to app/models/entity.rb

def has_image
  self[:has_image] # or read_attribute(:has_image)
end

then in the console (under rails 3.2.2)

> e = Entity.find(1)
> e.has_image
=> 0
> e[:has_image]
=> 0
> e.read_attribute(:has_image)
=> 0
Owner

tenderlove commented Mar 22, 2012

Strange. What database are you using? What is the schema for the table?

I'm using MySQL 5.5 (5.5.12) and Mysql2 as my connection adapter

migration is:

create_table :entities do |t|
  t.integer :old_id
  t.string  :type,          :null => false
  t.string  :name,          :null => false
  t.string  :url,           :null => false
  t.string  :telephone,     :limit => 40
  t.boolean :has_image,     :null => false, :default => false
  t.binary  :image_data
  t.text    :about
  t.boolean :enabled,       :null => false, :default => false
  t.boolean :can_search,    :null => false, :default => false
  t.boolean :can_view,      :null => false, :default => false
  t.boolean :can_review,    :null => false, :default => false
  t.float   :review_average
  t.float   :review_count,  :null => false, :default => 0
  t.integer :location_id
  t.integer :user_id
  t.timestamps
end

which creates the following table:

CREATE TABLE `entities` (
  `id` int(11) NOT NULL AUTO_INCREMENT,
  `old_id` int(11) DEFAULT NULL,
  `type` varchar(255) NOT NULL,
  `name` varchar(255) NOT NULL,
  `url` varchar(255) NOT NULL,
  `telephone` varchar(40) DEFAULT NULL,
  `has_image` tinyint(1) NOT NULL DEFAULT '0',
  `image_data` blob,
  `about` text,
  `enabled` tinyint(1) NOT NULL DEFAULT '0',
  `can_search` tinyint(1) NOT NULL DEFAULT '0',
  `can_view` tinyint(1) NOT NULL DEFAULT '0',
  `can_review` tinyint(1) NOT NULL DEFAULT '0',
  `subscribable` tinyint(1) NOT NULL DEFAULT '1',
  `review_average` float DEFAULT NULL,
  `review_count` float NOT NULL DEFAULT '0',
  `location_id` int(11) DEFAULT NULL,
  `user_id` int(11) DEFAULT NULL,
  `created_at` datetime NOT NULL,
  `updated_at` datetime NOT NULL,
  ... Snip ... (indexes)
) ENGINE=InnoDB AUTO_INCREMENT=1 DEFAULT CHARSET=utf8

Regardless of whether the attribute is overwritten or not the column is being correctly assessed as a boolean column by the Mysql2 DB adapter:

> Entity.columns[6]
=> #<ActiveRecord::ConnectionAdapters::Mysql2Adapter::Column:0xc6ffc50 @name="has_image", @sql_type="tinyint(1)", @null=false, @limit=1, @precision=nil, @scale=nil, @type=:boolean, @default=false, @primary=false, @coder=nil, @collation=nil>

What ruby version are you using? I couldn't reproduce it using Ruby 1.9.3p0 and MySQL 5.5.14.

I've constructed a minimal example to demonstrate the issue. The only alterations I've made to the clean rails install are to alter a couple of gems in the gemfile (replacing uglifier with closure-compiler and adding therubyracer), and to add two models - Entity, and Doctor (Entity is a STI model - Doctor is a subclass of Entity).

Have created a repo for the example at: https://github.com/cbailster/typecast-test

I have tested the code against the following rubies (all 3 produce the same issue):

  • ruby 1.9.2p180 (2011-02-18 revision 30909) [i686-linux]
  • ruby 1.9.2p290 (2011-07-09 revision 32553) [i686-linux]
  • ruby 1.9.3p125 (2012-02-16 revision 34643) [x86_64-linux]
Owner

tenderlove commented Mar 27, 2012

@cbailster thanks for the sample app. I've reproduced with your sample app, so I'll see what I can find and report back. Thanks!

@tenderlove tenderlove reopened this Mar 27, 2012

Owner

tenderlove commented Mar 27, 2012

I've fixed this on master. Working on 3-2-stable now.

Member

drogus commented Mar 27, 2012

@tenderlove look like here is the same issue: #5607, if it's the same problem please close it as well

Owner

tenderlove commented Mar 27, 2012

Fixing this in 3-2-stable is going to be far less trivial than on master. I'll try to describe the issue, and maybe other people (like @jonleighton) know of a trivial fix.

The main problem is that the implementation of the generated method of has_image is much more complex than just calling read_attribute. In 3-2-stable, the generated implementation of has_image depends on the column type, and looks like this:

attr_name = 'has_image'
missing_attribute(attr_name, caller) unless @attributes.has_key?(attr_name)
(v=@attributes[attr_name]) && ActiveRecord::ConnectionAdapters::SQLiteColumn.value_to_boolean(v)

That generated code has the knowledge of how to typecast. In other words, the casting code is defined inside the method when it's generated. Unfortunately this generated code never calls read_attribute. If we take a look at the implementation of read_attribute in 3-2-stable, we'll see that it calls a class method type_cast_attribute.

The way type_cast_attribute works is to:

  1. Check to see if the method has been generated on an anonymous module. If it has, it calls that method.
  2. Check to see if the methods have been generated at all, if not, generate them, include the module, then recurse.
  3. If the method wasn't defined and the methods have been generated, just hand back the value in the attributes hash.

The problem is that since typecasting is embedded in to the generated method, and we're defining a method of the same name as the generated method, the generated method that contains the typecasting information is never called.

Compare this to the code that's generated on master. The generated method source looks like this:

read_attribute('has_image') { |n| missing_attribute(n, caller) }

And read_attribute contains the knowledge of typecasting.

I'm not totally sure how we can fix this on 3-2-stable. I have to think about it more.

Owner

tenderlove commented Mar 27, 2012

I also need to research 3.1.x to see how this changed. :(

Owner

tenderlove commented Mar 27, 2012

@drogus yes, I think these are the same issue.

Member

jonleighton commented Mar 28, 2012

@tenderlove I am looking into a fix for 3.2

tenderlove added a commit that referenced this issue Mar 28, 2012

Test for #5549.
Cherry-picked from e96d04a.

Conflicts:

	activerecord/lib/active_record/attribute_methods/read.rb
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment