Unexpected behavior when calling `Model.find_by_id` with `true` or `false` about rails

[byroot]

I'd say ideally it should be fixed, but I fear this may break apps, and I'm not convinced this is a big enough problem to take that risk.

If you have time it would be worth to use git blame to find the commit that introduced this behavior to see if there was any historical reason to do so.

[rafaelfranca]

Some databases didn't had boolean, so you needed to transform them in integer. That is the reason for this behavior.

SQLite is one of them

2.1. Boolean Datatype
SQLite does not have a separate Boolean storage class. Instead, Boolean values are stored as integers 0 (false) and 1 (true).

SQLite recognizes the keywords "TRUE" and "FALSE", as of version 3.23.0 (2018-04-02) but those keywords are really just alternative spellings for the integer literals 1 and 0 respectively.

https://www.sqlite.org/datatype3.html

[maniSHarma7575]

Some databases didn't had boolean, so you needed to transform them in integer. That is the reason for this behavior.

@rafaelfranca, I believe the issue lies in how the find, find_by_id, and find_by(id: ...) methods filter based on the primary key id, typically an integer.

The complication arises with databases like SQLite, lacking a distinct Boolean storage class. Consequently, we're converting Boolean values to integers.

When passing a Boolean to these methods, incorrect results occur:

True returns records with id equal to 1.
False returns records with id equal to 0.

Indeed, the desired behavior would be for the methods to return nil when passed a Boolean value. However, currently, they incorrectly return records with id equal to 1 for True and 0 for False

[maniSHarma7575]

@olepalm I believe this behavior is expected since there are test cases for it in ActiveModel Integer, where we serialize boolean values to integers.

rails/activemodel/test/cases/type/integer_test.rb

Lines 41 to 45 in 343e781

	test "casting booleans for database" do
	type = Type::Integer.new
	assert_equal 1, type.serialize(true)
	assert_equal 0, type.serialize(false)
	end

We need to await comments from the Rails core team regarding whether this behavior is expected or necessitates a fix. If a fix is indeed required, then we'll need to update the serialize method accordingly.

I'll submit a pull request once we receive confirmation from the Rails maintenance team.

rails/activemodel/lib/active_model/type/integer.rb

Lines 65 to 68 in 343e781

	def serialize(value)
	return if value.is_a?(::String) && non_numeric_string?(value)
	ensure_in_range(super)
	end

to

  def serialize(value)
    return if value.is_a?(::TrueClass) || value.is_a?(::FalseClass)
    return if value.is_a?(::String) && non_numeric_string?(value)
    ensure_in_range(super)
  end

---

Reproduce Script:

# frozen_string_literal: true

require "bundler/inline"
require "debug"
gemfile(true) do
  source "https://rubygems.org"

  # If you want to test against edge Rails replace the previous line with this:
  gem "rails", github: "rails/rails", branch: "main"

  gem "sqlite3", "~> 1.4"
end

require "active_record"
require "minitest/autorun"
require "logger"

# This connection will do for database-independent bug reports.
ActiveRecord::Base.establish_connection(adapter: "sqlite3", database: ":memory:")
ActiveRecord::Base.logger = Logger.new(STDOUT)

ActiveRecord::Schema.define do
  create_table :posts do |t|
    t.string :title
    t.timestamps
  end
end

class Post < ActiveRecord::Base
end

class BugTest < Minitest::Test
  def test_find_methods
    post = Post.new(title: "Example Post")
    post.save
    post = Post.last
    assert_nil Post.find_by(id: true)
    assert_nil Post.find_by_id(true)
    assert_raises do
      Post.find(true)
    end
  end
end

[maniSHarma7575]

@byroot / @fatkodima Does this need to be fixed, or is it the desired behavior?