1

I am attempting to upgrade my code from ActiveRecord 3 to ActiveRecord 4, and I believe I have encountered a bug/regression in the boolean query support for ActiveRecord + SQLite3.

Here is the output of an IRB session running ActiveRecord 4.0.2 where SQLite3 is the database back-end:

2.0.0p353 :040 > StoreItem.where(item_class: 1, enabled: 1).order(item_order: :desc).count
 => 4 
2.0.0p353 :041 > StoreItem.where(item_class: 1, enabled: true).order(item_order: :desc).count
 => 0 

As a point of comparison, here is the same output when Mysql 5.5 is the database back-end:

2.0.0p353 :005 > StoreItem.where(item_class: 1, enabled: 1).order(item_order: :desc).count
 => 4 
2.0.0p353 :006 > StoreItem.where(item_class: 1, enabled: true).order(item_order: :desc).count
 => 4 

Now, let's see what happens when running with AR 3.2.14:

SQLite3:

2.0.0p353 :005 > StoreItem.where(item_class: 1, enabled: 1).order(item_order: :desc).count
 => 0 
2.0.0p353 :006 > StoreItem.where(item_class: 1, enabled: true).order(item_order: :desc).count
 => 4 

Mysql 5.5:

2.0.0p353 :001 > StoreItem.where(item_class: 1, enabled: 1).order(item_order: :desc).count
 => 4 
2.0.0p353 :002 > StoreItem.where(item_class: 1, enabled: true).order(item_order: :desc).count
 => 4 

As you can see, ActiveRecord 3.2.14 and 4.0.2 do the exact opposite thing in SQLite3 when presented with boolean queries.

I just checked the actual generated SQL and it is identical. The first query looks like this:

SELECT COUNT(*) FROM "store_items" WHERE "store_items"."item_class" = 1 AND "store_items"."enabled" = 1

The second looks like this:

SELECT COUNT(*) FROM "store_items" WHERE "store_items"."item_class" = 1 AND "store_items"."enabled" = 't'

Thus, perhaps there has been a change in SQLite3 from 1.3.5 to 1.3.8 in its treatment of boolean column values?

Is this a known bug, and can anyone comment on the cause?

esilver
  • 27,713
  • 23
  • 122
  • 168

2 Answers2

2

I have debugged the guts of both ActiveRecord 3.2.14 and 4.0.2. Here is the bug from start to finish:

SQLite allows you to insert any arbitrary string/number as a column value for boolean columns. Thus, you can insert 1 or 't' for a boolean column value.

ActiveRecord maps boolean values to string types 't' or 'f' when interacting with SQLite, not 1 or 0. The reasons behind that decision may stem from Postgres, but that is how it is for now.

When ActiveRecord creates a record for the first time in 3.2.14, at line 365 in persistence.rb, it creates a mapping of all model fields and inserts all fields, whether or not they have been changed. Here is the create method:

def create
  attributes_values = arel_attributes_values(!id.nil?)

  new_id = self.class.unscoped.insert attributes_values

  self.id ||= new_id if self.class.primary_key

  IdentityMap.add(self) if IdentityMap.enabled?
  @new_record = false
  id
end

This causes ActiveRecord to generate an insert statement like the following (note the presence of enabled, our boolean column):

INSERT INTO "store_items" ("created_at", "enabled", "other_columns....") VALUES (?, ?, ?)  [["created_at", 2014-01-11 21:47:24 UTC], ["enabled", true], ["other_colummns", ...]]

In ActiveRecord 4.0.2, the file dirty.rb, line 78, now calls the create_record method of persistence.rb (at line 507). Create record looks like this:

def create_record(attribute_names = @attributes.keys)
  attributes_values = arel_attributes_with_values_for_create(attribute_names)

  new_id = self.class.unscoped.insert attributes_values
  self.id ||= new_id if self.class.primary_key

  @new_record = false
  id
end

Because create_record now accepts an argument that lists only those columns that have changed, it generates an insert statement that does NOT include columns whose default values match what you are inserting. Thus, an insert statement that used a boolean value that matched a default would look like this:

INSERT INTO "store_items" ("created_at", "other_columns....") VALUES (?, ?, ?)  [["created_at", 2014-01-11 21:47:24 UTC], ["other_colummns", ...]]

Note that the boolean column "enabled" is missing, because in this case, our default value of true/1 matched what we were inserting the first time we created the record.

Because enabled is NOT specified by the insert statement generated by ActiveRecord, SQLite gives it the value of 1, instead of the value of 't', which is what ActiveRecord 3.1.14 used to give it.

Ultimately, to work around this bug, don't include a default value on boolean columns or ensure that you are changing it to something that is not the default to force ActiveRecord to actually set it to a 't' or 'f' value on create.

Thus, change this:

class CreateStoreItems < ActiveRecord::Migration
  def change
    create_table :store_items do |t|
      t.boolean :enabled, :null => false, :default => 1
    end
  end
end

to this

class CreateStoreItems < ActiveRecord::Migration
  def change
    create_table :store_items do |t|
      t.boolean :enabled, :null => false
    end
  end
end

Alternatively, if you 'twiddle' the boolean value instead of relying on the default, you can correct the bug as well.

esilver
  • 27,713
  • 23
  • 122
  • 168
0

ActiveRecord has a history of confusion with SQLite's booleans. You can read about some of the issues in Rails3 over here:

Rails 3 SQLite3 Boolean false

The executive summary is:

  • SQLite uses C-style zero and one booleans natively rather than a real boolean type.
  • MySQL uses C-style zero and one booleans natively rather than a real boolean type.
  • The SQLite driver (up to Rails3 at least) mistakenly used 't' and 'f' strings for boolean literals. These are actually the string representations of PostgreSQL's native boolean type, the SQLite driver seems to have accidentally inherited them from the base driver which was probably written for PostgreSQL.

That means that, assuming enabled is a boolean column, enabled: 1 and enabled: true should be identical in MySQL but that's a bug in your code that is accidentally assuming things about the underlying database's behavior. In Rails3, enabled: 1 and enabled: true would be entirely different things with SQLite and they'd be different things in PostgreSQL regardless of the which ActiveRecord version you were using.

In the Rails4 version of ActiveRecord, I'm guessing (no, I haven't traced the execution) that the SQLite boolean confusion is sometimes handled in quote with this kludgery:

case value
  #...
  when true, false
    if column && column.type == :integer
      value ? '1' : '0'
    else
      value ? quoted_true : quoted_false
    end

so if you send in a real Ruby boolean, it will should get kludged into the right value for the database but if you send in an integer, all bets are off. Similar nonsense appears in type_cast.

I think you have some work to do:

  1. Stop assuming that 1 and true are the same thing, same goes for 0 and false. If you want to query the database for a boolean value then use true and false.
  2. Repair your SQLite database so that all the boolean columns use 't' and 'f' to match the SQLite driver's confusion. You'll also want to check the values of all your boolean columns in MySQL just to make sure, MySQL also plays fast'n'loose with the rules so it can do strange things behind your back. MySQL isn't as fast'n'loose as SQLite but they'll both bend the rules in the name of friendliness.
  3. Stop using two different database systems unless you're prepared to enforce portability by hand. Database portability is largely a myth unless you are very very careful, only speak to the database in baby-talk SQL, or you write your own portability layer and write to that API instead of the ORM's native API. No, AR won't save you.

I tend to think that ActiveRecord should throw an exception if you give it something like boolean_column: 1 or where('c in (?)', empty_array); telling me that I'm being an idiot would be friendly, helping me to quietly make a mess of things is not.

Community
  • 1
  • 1
mu is too short
  • 426,620
  • 70
  • 833
  • 800
  • Thanks for the thoughts. I use both mysql and Sqlite3 because I run my rspec tests using an in-memory SQLite3 database, which I believe is a common setup in the ruby world. Because the SQlite3 is in-memory, there is nothing to "repair". I had never before used 1/true interchangeably until trying to debug this issue. – esilver Jan 11 '14 at 22:04
  • It may be common but it is still a bad idea (as bad as Rails defaulting to SQLite or did they fix that foolishness in v4?) because database portability is a myth. AR likes to pretend that it completely hides the database from but that's simply not true; consider something simple like the difference between inserting a string of length 10 in a `varchar(5)` in SQLite (the whole thing goes in), MySQL (truncated to 5 or get an exception depending on server configuration), and PostgreSQL (exception from the database). And no, validations won't save you. – mu is too short Jan 11 '14 at 22:12