Lighthouse has a new layout. Prefer the old one? Return to the old layout, and switch back any time from the link at the top of each page.

There was a problem

This project is archived and is in readonly mode.

[ActiveRecord] create(:association_id => @object.id) vs create(:association_id => @object)

#3188

Lately I've encountered strange behavior of ActiveRecord. Suppose we have classic Post and Comment models (post has many comments, comment belongs to post). There are a few ways to create comment to given post. For example:

Comment.create(:post => @post)
Comment.create(:post_id => @post.id)

I made a mistake and mixed those two:

Comment.create(:post_id => @post)

In this case it always create Comment with post_id = 1 regardless of given Post object. It take me several hours to find this bug in my code and I think that it's not acceptable behavior.

I think that description is enough clear but for formality. How to reproduce:

$ rails -v Rails 2.3.4

$ rails bug && cd bug $ ./script/generate model Post title:string $ ./script/generate model Comment post:references title:string $ rake db:migrate $ ./script/console

first scenario (OK)

p1 = Post.create(:title => "1") Post Create (0.8ms) INSERT INTO "posts" ("updated_at", "title", "created_at") VALUES('2009-09-11 07:41:32', '1', '2009-09-11 07:41:32') => # p2 = Post.create(:title => "2") Post Create (0.5ms) INSERT INTO "posts" ("updated_at", "title", "created_at") VALUES('2009-09-11 07:41:37', '2', '2009-09-11 07:41:37') => # c1 = Comment.create(:title => "comment", :post => p2) Comment Create (1.3ms) INSERT INTO "comments" ("updated_at", "title", "post_id", "created_at") VALUES('2009-09-11 07:41:43', 'comment', 2, '2009-09-11 07:41:43') => # c1.post_id => 2

second scenario (OK)

c2 = Comment.create(:title => "comment", :post_id => p2.id) Comment Create (0.6ms) INSERT INTO "comments" ("updated_at", "title", "post_id", "created_at") VALUES('2009-09-11 07:42:35', 'comment', 2, '2009-09-11 07:42:35') => # c2.post_id => 2

third scenario (BAD)

c3 = Comment.create(:title => "comment", :post_id => p2) Comment Create (0.6ms) INSERT INTO "comments" ("updated_at", "title", "post_id", "created_at") VALUES('2009-09-11 07:42:54', 'comment', 1, '2009-09-11 07:42:54') => # c3.post_id => 1

Reported by Radosław Bułat · September 11th, 2009 @ 08:44 AM

State: new
Milestone: none
Assigned to: nobody
Importance: Low

Activity

  1. Radosław Bułat
    Radosław Bułat

    No comments? It could save a lot of time for me and I believe that others to.

    September 22nd, 2009 @ 11:53 AM

  2. Radosław Bułat
  3. josh
    josh
    • Assigned user changed from josh to Pratik

    September 22nd, 2009 @ 04:16 PM

  4. CancelProfileIsBroken
    CancelProfileIsBroken
    • Tag changed from activerecord, create to activerecord, bugmash, create
    • Assigned user cleared.

    September 25th, 2009 @ 12:22 PM

  5. Elad Meidar
    Elad Meidar

    +1 verified on 2-3-stable and master.

    iv'e attached a patch with a failing test (created on 2-3-stable, applies on master too).

    September 27th, 2009 @ 04:50 PM

  6. sr.iniv.t
    sr.iniv.t

    +1 verified.

    Elad's patch applies cleanly on master and 2-3-stable and the issue is reproducible.

    September 27th, 2009 @ 06:42 PM

  7. Elad Meidar
    Elad Meidar

    i Dug into it, and came up with type_cast as the issue, i added a special case for type casting AR:Base decedent instances as values (defaults to primary key returns as value).

    The only issue is in the test, which still fails.
    I scattered a bunch of debug prints all over the method chain create => attributes_with_quotes' =>read_attribute=>type_castto make sure that the right value is assigned and still, when i access@post.attributesin the test,author_idis as expected but@post.author_idis still 1. (more on this "gist":http://gist.github.com/194887 )

    iv'e attached a patch with the changes and the failing test, + some debug prints to make it clear.

    September 27th, 2009 @ 07:09 PM

  8. Elad Meidar
    Elad Meidar

    argh, markdown fails. :)

    September 27th, 2009 @ 07:09 PM

  9. Matías Flores
    Matías Flores

    I've verified the issue exists and it's easily reproducible

    September 27th, 2009 @ 07:19 PM

  10. Elomar França
    Elomar França

    +1, verified.

    I had some failures when applying the patch from Elad Meidar, both on master and 2-3-stable.

    I think this issue could also be solved in "write_attribute". If the attr is from a belongs_to association and the given value is a AR::Base, write value.id to the attribute, but I couldn't discover how implement it yet :(

    September 27th, 2009 @ 07:27 PM

  11. Evgeniy Dolzhenko
    Evgeniy Dolzhenko

    +1 just spend an hour or so on figuring out the exact issue, and then found this ticket.

    The following snippet from Column#type_cast seems just wrong (or just the intention isn't obvious):

    when :integer   then value.to_i rescue value ? 1 : 0
    

    October 21st, 2009 @ 12:58 PM

  12. Evgeniy Dolzhenko
    Evgeniy Dolzhenko

    @Elad, the problem is that your patch adds new functionality to Column#type_cast but doesn't repeats that for Column#type_cast_code which is user by the attribute read method (@post.author_id in your case)

    October 21st, 2009 @ 01:17 PM

  13. Rizwan Reza
    Rizwan Reza
    • Tag changed from activerecord, bugmash, create to activerecord, create

    February 12th, 2010 @ 12:46 PM

  14. Ryan Bigg
    Ryan Bigg
    • Tag cleared.
    • Importance changed from to Low

    Automatic cleanup of spam.

    October 9th, 2010 @ 10:13 PM

  15. Ryan Bigg
    Ryan Bigg

    Automatic cleanup of spam.

    October 21st, 2010 @ 03:39 AM

  16. Jeff Kreeftmeijer
  17. bingbing
  18. bingbing