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

Use memoization for collection associations ids reader #21092

Merged
merged 1 commit into from Aug 6, 2015
Merged

Use memoization for collection associations ids reader #21092

merged 1 commit into from Aug 6, 2015

Conversation

meinac
Copy link
Contributor

@meinac meinac commented Aug 2, 2015

As far as I can see from the source of the Rails we don't need to set @association_ids to nil to force ActiveRecord to execute a query because if user changes the association either by calling records_writer or ids_writer the relation will be marked as loaded and rails will return ids from loaded collection.

Fixes #21082

@@ -62,8 +62,10 @@ def ids_reader
record.send(reflection.association_primary_key)
end
else
column = "#{reflection.quoted_table_name}.#{reflection.association_primary_key}"
scope.pluck(column)
@association_ids ||= (
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extra space before the =

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@egilburg I don't think extra space before = is valid ruby syntax.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I think you are taking about next line. I'm going to fix it.

@rafaelfranca rafaelfranca reopened this Aug 6, 2015
rafaelfranca added a commit that referenced this pull request Aug 6, 2015
Use memoization for collection associations ids reader
@rafaelfranca rafaelfranca merged commit 22625a3 into rails:master Aug 6, 2015
microweb10 added a commit to AyuntamientoMadrid/consul that referenced this pull request Apr 17, 2019
Failure:

  Budget::Stats#total_votes returns the number of total votes
  Failure/Error: create(:budget_ballot_line, investment:
  create(:budget_investment, :selected, budget: budget))

    ActiveRecord::RecordInvalid:
      Validation failed: Money insufficient funds
      spec/models/budget/stats_spec.rb:109

Associations ids changed since this PR
rails/rails#21092

By forcing the object to be reloaded, the result is as expected.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants