Assocation saving backward incompatibility in Rails 1.2.4

Today I ran across test failures in my app when upgrading from Rails
1.2.3 to 1.2.4. I determined the issue is caused by r7076 (http://
dev.rubyonrails.org/changeset/7076) which tries to save associated
records if and only if the association is already loaded. Here is a
simple test case that passes in Rails 1.2.3 and fails in Rails 1.2.4:

class Computer < ActiveRecord::Base
  has_many :change_logs

  before_save do |computer|
    computer.change_logs.create(:message => "Computer created!") if
computer.new_record?
  end
end

class ChangeLog < ActiveRecord::Base
end

def test_save_on_parent_saves_child_even_when_not_loaded
  computer = Computer.create!
  assert_equal "Computer created!", computer.change_logs.first.message
end

Upon further investigation I discovered r7511 (http://
dev.rubyonrails.org/changeset/7511) which adds a specific check and
exception when a user attempts to use the behavior that r7076
eliminated. That changeset was never backported to the 1.2 branch.

I see three options for resolving this:

1. Backport r7511 to the 1.2 branch. This would give users appropriate
feedback if they trip over this behavior, but still break backwards
compatibility for AR.

2. Revert r7076 to preserve full backwards compatibility in the 1.2
branch.

3. Attempt to find a solution that addresses the concern in ticket
#8713 (http://dev.rubyonrails.org/ticket/8713) while still maintaining
backward compatibility with Rails 1.2.3. To this end, I have been
toying with the following patch which passes all existing AR tests:

Index: lib/active_record/associations.rb

Today I ran across test failures in my app when upgrading from Rails
1.2.3 to 1.2.4. I determined the issue is caused by r7076 (http://
dev.rubyonrails.org/changeset/7076) which tries to save associated
records if and only if the association is already loaded. Here is a
simple test case that passes in Rails 1.2.3 and fails in Rails 1.2.4:

class Computer < ActiveRecord::Base
  has_many :change_logs

  before_save do |computer|
    computer.change_logs.create(:message => "Computer created!") if
computer.new_record?
  end
end

class ChangeLog < ActiveRecord::Base
end

def test_save_on_parent_saves_child_even_when_not_loaded
  computer = Computer.create!
  assert_equal "Computer created!", computer.change_logs.first.message
end

Upon further investigation I discovered r7511 (http://
dev.rubyonrails.org/changeset/7511) which adds a specific check and
exception when a user attempts to use the behavior that r7076
eliminated. That changeset was never backported to the 1.2 branch.

I see three options for resolving this:

1. Backport r7511 to the 1.2 branch. This would give users appropriate
feedback if they trip over this behavior, but still break backwards
compatibility for AR.

I'd prefer not to do this.

2. Revert r7076 to preserve full backwards compatibility in the 1.2
branch.

The performance implications can be considerable, so lets try and
avoid this too.

3. Attempt to find a solution that addresses the concern in ticket
#8713 (http://dev.rubyonrails.org/ticket/8713) while still maintaining
backward compatibility with Rails 1.2.3. To this end, I have been
toying with the following patch which passes all existing AR tests:

This approach seems most promising

Depending on the preference expressed here, I can work up an
appropriate ticket/patch to execute one of those options. My
preference at the moment is for either #2 or #3. (Of course I have a
vested interest in a Rails 1.2 app that depends on this behavior :wink: )

What do you guys think?

A patch for the 3rd option would be the best bet, assuming the current
behaviour makes sense.

Can you whip up a patch for stable which has a failing test?