This project is archived and is in readonly mode.

#1948 ✓stale
Farrel

Transaction block sets model ID to non-existent row

Reported by Farrel | February 12th, 2009 @ 07:58 AM | in 3.x

I'm on Rails 2.2.2 with MySQL 5.0.70 with InnoDB table types. I'm running into a wierd bug where if I have two models in a transaction and the second model raises an exception, the first model still has it's ID set and it's new_record status set to false despite the row never existing.

The following is a console session showing this:

client_user = new_client_user( :first_name => nil ) # Will cause a validation to fail => #far...@bosman.com", first_name: nil, last_name: "Sim", active: true> establishment = new_establishment( :brand => brand ) => #Establishment.transaction{ establishment.save!; client_user.save! } rescue false => false establishment.id => 10 establishment.new_record? => false client_user.id => nil establishment.reload ActiveRecord::RecordNotFound: Couldn't find Establishment with ID=10 ... Establishment.count => 4

Here's an explanation of the session: 1) I initialise a new client user, with first_name set to nil so it will fail a validation 2) I then initialise a new establishment model 3) I try and save both in a transaction block. The client_user.save! call raises an exception causing the block to roll back. 4) Yet establishment has an ID set to 10 and is marked as not being a new record. 5) As expected client_user has no ID 6) Calling reload on establishment throws an error because the row does not exist 7) Counting up the actual rows we see it is below the ID assigned to establishment (I did this a few times, the first time the assigned ID was 5, then 6, then 7 etc)

This causes all sorts of problems in my app when the returned @establishment is passed to form_for, it thinks the establishment exists and tries to PUT to '/establishments/10' instead of POST'ing to '/establishments'.

Comments and changes to this ticket

  • Farrel

    Farrel February 12th, 2009 @ 08:01 AM

    Whoops, here's the code block formatted correctly:

    
    >> client_user = new_client_user( :first_name => nil ) # Will cause validation to fail
    => #<ClientUser id: nil, login: "rene173744", email_address: "far...@bosman.com", first_name: nil, last_name: "Sim", active: true>
    >> establishment = new_establishment( :brand => brand )
    => #<Establishment id: nil, brand_id: 4, name: "Pretoria GrandWest" ,active: true>
    >> Establishment.transaction{ establishment.save!; client_user.save!  } rescue false
    => false
    >> establishment.id
    => 10
    >> establishment.new_record?
    => false
    >> client_user.id
    => nil
    >> establishment.reload
    ActiveRecord::RecordNotFound: Couldn't find Establishment with ID=10
    ...
    >> Establishment.count
    => 4
    
  • CancelProfileIsBroken
  • John Trupiano

    John Trupiano August 9th, 2009 @ 04:48 PM

    • Tag changed from bugmash to bugmash, verified

    verified

  • John Trupiano

    John Trupiano August 9th, 2009 @ 05:14 PM

    Failing test case patch attached. Problem still unresolved.

  • David Trasbo

    David Trasbo August 9th, 2009 @ 07:29 PM

    verified

    Reproducible in Rails 2.3.2 using MySQL.

  • Rizwan Reza

    Rizwan Reza August 9th, 2009 @ 07:49 PM

    verified

    +1 This is a relatively important bug. The test case fails.

  • Derander

    Derander August 9th, 2009 @ 08:51 PM

    +1 verified.

    Ran the test, the test seems to be appropriate. Does fail.

  • Hugo Peixoto

    Hugo Peixoto August 9th, 2009 @ 10:58 PM

    verified in master.
    +1 does seem like an important bug.

  • Tristan Dunn

    Tristan Dunn August 9th, 2009 @ 11:27 PM

    +1

    Verified on master, but seems like it's going to be a fairly tricky fix.

  • Jeremy Kemper

    Jeremy Kemper August 9th, 2009 @ 11:36 PM

    • Assigned user set to “Jeremy Kemper”
    • State changed from “new” to “open”

    Fixing this well is difficult without piling on more hacks. It points to a missing Transaction class to encapsulate this behavior, including records "enrolled" in the transaction.

  • Joe Van Dyk

    Joe Van Dyk September 27th, 2009 @ 01:43 AM

    +1 verified on sqlite3 as well.

  • Elad Meidar

    Elad Meidar September 27th, 2009 @ 04:48 AM

    +1 verified Mysql and sqlite 3 Sounds like some kind of a GC is required when rescuing within a transaction. i traced it up until rollback_active_record_state! on transactions.rb, but it seems that the rollback is being preformed only on the instance that raised the exception and not all instances in the transaction as it should.

  • Pieter

    Pieter December 4th, 2009 @ 05:38 PM

    +1 verified mysql and sqlite3.

    see below for an rspec example which shows the unexpected behaviour


    require 'spec_helper'
    describe Author do
    self.use_transactional_fixtures = false

    after(:each) do

    {mkd-extraction-c86ddf888e2dd7a0001e9ea8099dc0d0}

    end

    it "should behave normal when using transactions" do

    {mkd-extraction-0fc41ba6a9a845baba6e5580443a47ac}

    end end

  • Amit

    Amit December 9th, 2009 @ 10:07 PM

    +1 breaks nested forms with validation.

    Isn't it a critical issue? basically it means that transactions containing multiple objects doesn't work.

  • Rizwan Reza

    Rizwan Reza February 12th, 2010 @ 12:46 PM

    • Tag changed from bugmash, verified to verified
  • Jeremy Kemper

    Jeremy Kemper May 4th, 2010 @ 06:48 PM

    • Milestone changed from 2.x to 3.x
  • Santiago Pastorino

    Santiago Pastorino February 2nd, 2011 @ 04:41 PM

    • Importance changed from “” to “”

    This issue has been automatically marked as stale because it has not been commented on for at least three months.

    The resources of the Rails core team are limited, and so we are asking for your help. If you can still reproduce this error on the 3-0-stable branch or on master, please reply with all of the information you have about it and add "[state:open]" to your comment. This will reopen the ticket for review. Likewise, if you feel that this is a very important feature for Rails to include, please reply with your explanation so we can consider it.

    Thank you for all your contributions, and we hope you will understand this step to focus our efforts where they are most helpful.

  • Santiago Pastorino

    Santiago Pastorino February 2nd, 2011 @ 04:41 PM

    • State changed from “open” to “stale”

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

<h2 style="font-size: 14px">Tickets have moved to Github</h2>

The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>

Attachments

Tags

Pages