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

ActiveRecord does not cast the value passed in Hash #35205

Closed
sinsoku opened this issue Feb 9, 2019 · 4 comments · Fixed by #36314
Closed

ActiveRecord does not cast the value passed in Hash #35205

sinsoku opened this issue Feb 9, 2019 · 4 comments · Fixed by #36314

Comments

@sinsoku
Copy link
Contributor

sinsoku commented Feb 9, 2019

Steps to reproduce

# frozen_string_literal: true

require "bundler/inline"

gemfile(true) do
  source "https://rubygems.org"

  git_source(:github) { |repo| "https://github.com/#{repo}.git" }

  gem "rails", github: "rails/rails"
  gem "sqlite3"
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, force: true do |t|
  end

  create_table :comments, force: true do |t|
    t.integer :post_id
  end

  create_table :likes, force: true do |t|
    t.integer :comment_id
  end
end

class Post < ActiveRecord::Base
  has_many :comments
end

class Comment < ActiveRecord::Base
  belongs_to :post

  has_many :likes
end

class Like < ActiveRecord::Base
  belongs_to :comment
end

class BugTest < Minitest::Test
  def test_association_stuff
    with_comments = Post.joins(:comments).where(comments: { id: 'foo' })
    assert_equal with_comments.to_sql,
      'SELECT "posts".* FROM "posts" INNER JOIN "comments" ON "comments"."post_id" = "posts"."id" WHERE "comments"."id" = 0' # OK

    with_likes_merge = Post.joins(comments: :likes).merge(Like.where(id: 'foo'))
    assert_equal with_likes_merge.to_sql,
      'SELECT "posts".* FROM "posts" INNER JOIN "comments" ON "comments"."post_id" = "posts"."id" INNER JOIN "likes" ON "likes"."comment_id" = "comments"."id" WHERE "likes"."id" = 0' # OK

    with_likes = Post.joins(comments: :likes).where(likes: { id: 'foo' })
    assert_equal with_likes.to_sql,
      'SELECT "posts".* FROM "posts" INNER JOIN "comments" ON "comments"."post_id" = "posts"."id" INNER JOIN "likes" ON "likes"."comment_id" = "comments"."id" WHERE "likes"."id" = 0'
    # Failure:
    # BugTest#test_association_stuff [test.rb:60]:
    # --- expected
    # +++ actual
    # @@ -1 +1 @@
    # -"SELECT \"posts\".* FROM \"posts\" INNER JOIN \"comments\" ON \"comments\".\"post_id\" = \"posts\".\"id\" INNER JOIN \"likes\" ON \"likes\".\"comment_id\" = \"comments\".\"id\" WHERE \"likes\".\"id\" = 'foo'"
    # +"SELECT \"posts\".* FROM \"posts\" INNER JOIN \"comments\" ON \"comments\".\"post_id\" = \"posts\".\"id\" INNER JOIN \"likes\" ON \"likes\".\"comment_id\" = \"comments\".\"id\" WHERE \"likes\".\"id\" = 0"
  end
end

Expected behavior

I expected "foo" to be cast for bigint column.

Actual behavior

I get a query that uses string in bigint column.

If you use PostgreSQL, then it occurs ActiveRecord::StatementInvalid.

Post.joins(comments: :likes).where(likes: { id: 'foo' }).all
#   Post Load (7.3ms)  SELECT  "posts".* FROM "posts" INNER JOIN "comments" ON "comments"."post_id" = "posts"."id" INNER JOIN "likes" ON "likes"."comment_id" = "comments"."id" WHERE "likes"."id" = $1 LIMIT $2  [["id", "foo"], ["LIMIT", 11]]
# Traceback (most recent call last):
# ActiveRecord::StatementInvalid (PG::InvalidTextRepresentation: ERROR:  invalid input syntax for integer: "foo")
# : SELECT  "posts".* FROM "posts" INNER JOIN "comments" ON "comments"."post_id" = "posts"."id" INNER JOIN "likes" ON "likes"."comment_id" = "comments"."id" WHERE "likes"."id" = $1 LIMIT $2

System configuration

Rails version: master(25d8afb)

Ruby version: ruby 2.6.1p33 (2019-01-30 revision 66950) [x86_64-darwin15]

@dmitry
Copy link
Contributor

dmitry commented Feb 10, 2019

Where does "foo" should be casted, via activerecord type caster or directly in the query?

This will raise an error in PG:

SELECT 'foo'::integer;

@yawboakye
Copy link
Contributor

@dmitry ActiveRecord casts (basically "foo".to_i) before making the eventual SQL.

@sinsoku
Copy link
Contributor Author

sinsoku commented Mar 18, 2019

ActiveRecord is basically cast the value using to_i:

with_comments = Post.joins(:comments).where(comments: { id: 'foo' })
with_comments.to_sql,
#=> SELECT "posts".* FROM "posts" INNER JOIN "comments" ON "comments"."post_id" = "posts"."id" WHERE "comments"."id" = 0

However, ActiveRecord does not cast when using nested joins and Hash value:

with_likes = Post.joins(comments: :likes).where(likes: { id: 'foo' })
with_likes.to_sql
#=> SELECT "posts".* FROM "posts" INNER JOIN "comments" ON "comments"."post_id" = "posts"."id" INNER JOIN "likes" ON "likes"."comment_id" = "comments"."id" WHERE "likes"."id" = 'foo'

I was wondering if this was the intended behavior of ActiveRecord, so I reported an issue.

@yawboakye
Copy link
Contributor

@sinsoku it's most likely a bug. feel free to submit a PR. I looked through the tests and didn't find any case for not serializing integer IDs to integers.

kamipo added a commit to kamipo/rails that referenced this issue May 21, 2019
Unfortunately, a11a8ff had no effect as long as using bind param, and
was not tested.

This ensures making the intent of a11a8ff, which fall back to type
casting from the connection adapter.

Fixes rails#35205.

```
% ARCONN=postgresql bundle exec ruby -w -Itest test/cases/relation/where_test.rb -n test_type_casting_nested_joins
Using postgresql
Run options: -n test_type_casting_nested_joins --seed 55730

# Running:

E

Error:
ActiveRecord::WhereTest#test_type_casting_nested_joins:
ActiveRecord::StatementInvalid: PG::InvalidTextRepresentation: ERROR:  invalid input syntax for integer: "2-foo"

rails test test/cases/relation/where_test.rb:30

Finished in 0.245778s, 4.0687 runs/s, 0.0000 assertions/s.
1 runs, 0 assertions, 0 failures, 1 errors, 0 skips
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants