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.

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