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

prior association method call for has_one relation fires unwanted select query after save #43304

Closed
mtoygar opened this issue Sep 24, 2021 · 1 comment

Comments

@mtoygar
Copy link

mtoygar commented Sep 24, 2021

Steps to reproduce

  • a foo record with a has_one association named bar
  • call foo.association(:bar)
  • call foo.save!
  • it fires a select request for the bar relation
# frozen_string_literal: true

require "bundler/inline"

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

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

  gem "activerecord", "~> 7.0.0.alpha2"
  gem "sqlite3"
end

require "active_record"
require "logger"

ActiveRecord::Base.establish_connection(adapter: "sqlite3", database: ":memory:")
ActiveRecord::Base.logger = Logger.new(STDOUT)

ActiveRecord::Schema.define do
  create_table :bars, force: true do |t|
    t.integer :foo_id
  end

  create_table :foos, force: true do |t|
    t.string :name
  end
end

puts <<~EOF
############################
############################
## DB setup is completed. ##
############################
############################
EOF

class Bar < ActiveRecord::Base
  belongs_to :foo
end

class Foo < ActiveRecord::Base
  has_one :bar
end

def with_association_call
  foo = Foo.create!

  foo.name = 'new one'
  foo.association(:bar)
  foo.save!
end

def without_association_call
  foo = Foo.create!

  foo.name = 'new one'
  foo.save!
end

puts 'with_association_call'
with_association_call
puts 'without_association_call'
without_association_call 

# below is the output of this script, with_association_call fires an unwanted select query for bar association

with_association_call
D, [2021-09-24T17:28:13.637181 #50914] DEBUG -- :   TRANSACTION (0.0ms)  begin transaction
D, [2021-09-24T17:28:13.637389 #50914] DEBUG -- :   Foo Create (0.1ms)  INSERT INTO "foos" DEFAULT VALUES
D, [2021-09-24T17:28:13.637630 #50914] DEBUG -- :   TRANSACTION (0.0ms)  commit transaction
D, [2021-09-24T17:28:13.643616 #50914] DEBUG -- :   TRANSACTION (0.0ms)  begin transaction
D, [2021-09-24T17:28:13.643829 #50914] DEBUG -- :   Foo Update (0.1ms)  UPDATE "foos" SET "name" = ? WHERE "foos"."id" = ?  [["name", "new one"], ["id", 1]]
D, [2021-09-24T17:28:13.647662 #50914] DEBUG -- :   Bar Load (0.1ms)  SELECT "bars".* FROM "bars" WHERE "bars"."foo_id" = ? LIMIT ?  [["foo_id", 1], ["LIMIT", 1]]
D, [2021-09-24T17:28:13.647816 #50914] DEBUG -- :   TRANSACTION (0.0ms)  commit transaction
without_association_call
D, [2021-09-24T17:28:13.648166 #50914] DEBUG -- :   TRANSACTION (0.0ms)  begin transaction
D, [2021-09-24T17:28:13.648294 #50914] DEBUG -- :   Foo Create (0.1ms)  INSERT INTO "foos" DEFAULT VALUES
D, [2021-09-24T17:28:13.648436 #50914] DEBUG -- :   TRANSACTION (0.0ms)  commit transaction
D, [2021-09-24T17:28:13.648743 #50914] DEBUG -- :   TRANSACTION (0.0ms)  begin transaction
D, [2021-09-24T17:28:13.648890 #50914] DEBUG -- :   Foo Update (0.1ms)  UPDATE "foos" SET "name" = ? WHERE "foos"."id" = ?  [["name", "new one"], ["id", 2]]
D, [2021-09-24T17:28:13.649011 #50914] DEBUG -- :   TRANSACTION (0.0ms)  commit transaction

Expected behavior

with_association_call should not fire SELECT "bars".* FROM "bars" WHERE "bars"."foo_id" = ? LIMIT ? query

Actual behavior

with_association_call fires the query

System configuration

Rails version: 7.0.0.alpha2
Ruby version: ruby 2.7.2p137 (2020-10-01 revision 5445e04352) [x86_64-darwin20]

Some ideas about what happens here

I believe the error stems from the implementation of the below save_has_one_association method.

def save_has_one_association(reflection)
association = association_instance_get(reflection.name)
record = association && association.load_target
if record && !record.destroyed?
autosave = reflection.options[:autosave]
if autosave && record.marked_for_destruction?
record.destroy
elsif autosave != false
key = reflection.options[:primary_key] ? public_send(reflection.options[:primary_key]) : id
if (autosave && record.changed_for_autosave?) || record_changed?(reflection, record, key)
unless reflection.through_reflection
record[reflection.foreign_key] = key
if inverse_reflection = reflection.inverse_of
record.association(inverse_reflection.name).inversed_from(self)
end
end
saved = record.save(validate: !autosave)
raise ActiveRecord::Rollback if !saved && autosave
saved
end
end
end
end

An early return like return unless autosave = reflection.options[:autosave] is solving the issue on my local. It seems we dont need to load relation if autosave option is falsy. I'm willing to open a pr if this solution seems plausible to you.

EDIT: We cannot add an early return here as has_one relation wants to save new records by default.
maybe we can add an early return like the one used in save_belongs_to_association 👇 (sth like return unless association && association.loaded?)

return unless association && association.loaded? && !association.stale_target?

Note: I was not able to reproduce this for has_many and belongs_to relations if that helps.

Also, probably related to #42637. Adding an early return seems to solve that issue as well.

@p8 p8 added the activerecord label Oct 4, 2021
@rails-bot
Copy link

rails-bot bot commented Jan 2, 2022

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 team are limited, and so we are asking for your help.
If you can still reproduce this error on the 7-0-stable branch or on main, please reply with all of the information you have about it in order to keep the issue open.
Thank you for all your contributions.

@rails-bot rails-bot bot added the stale label Jan 2, 2022
@rails-bot rails-bot bot closed this as completed Jan 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants