Ruby on Rails | Screencasts | Download | Documentation | Weblog | Community | Source

Ticket #7576 (new defect)

Opened 3 years ago

Last modified 2 years ago

[PATCH] has_and_belongs_to_many :finder_sql only interpolated once per class

Reported by: alexkwolfe Assigned to: core
Priority: high Milestone: 2.x
Component: ActiveRecord Version: edge
Severity: normal Keywords:
Cc: theflinnster@gmail.com, bitsweat

Description

A has_and_belongs_to_many association using :finder_sql with interpolation is only evaluated once per class. This bug becomes evident when the interpolated sql query contains a call to an instance method (like #id()).

Without this patch, ActiveRecord caches the SQL query after the first interpolation, making all subsequent calls to the same association (even on different instances) use the same query string. The patch provided causes the interpolation to occur with each call to the association. Test case included.

For example, when post.categories is a has_and_belongs_to_many association defined with :finder_sql that contains a call to #{id}: 

categories = Post.find(1).categories
categories == Post.find(2).categories 
=> true

After the patch: 

categories = Post.find(1).categories
categories == Post.find(2).categories 
=> false

Attachments

has_and_belongs_to_many_interpolation.diff (2.5 kB) - added by alexkwolfe on 02/16/07 19:28:25.

Change History

02/16/07 19:28:25 changed by alexkwolfe

  • attachment has_and_belongs_to_many_interpolation.diff added.

02/16/07 19:31:41 changed by alexkwolfe

  • summary changed from [PATCH] has_and_belongs_to_many :finder_sql interpolation evaluated once per class to [PATCH] has_and_belongs_to_many :finder_sql only interpolated once per class.

Clarified summary.

02/18/07 22:35:40 changed by alexkwolfe

OK. So the example in the original ticket wasn't exactly clear. I'll try again:

Here's the setup. The categories association is defined as a has_and_belongs_to_many with :finder_sql that requires interpolation with #id(): 

Post.find(1).categories.create(:name => 'Awesome')
Post.find(2).categories.create(:name => 'Neato')

Before the patch, Post 2 uses the SQL interpolated during loading of Post 1's categories. The :finder_sql is only interpolated once: 

Post.find(1).categories == Post.find(2).categories 
=> true

Post.find(1).categories.collect(&:name) 
=> ["Awesome"]

Post.find(2).categories.collect(&:name) 
=> ["Awesome"]

After the patch, SQL is re-interpolated for each instance's call to #categories. Now each Post instance correctly loads the appropriate Categories: 

Post.find(1).categories == Post.find(2).categories 
=> false

Post.find(1).categories.collect(&:name)
=> ["Awesome"] 

Post.find(2).categories.collect(&:name)
=> ["Neato"]

06/06/07 13:51:47 changed by nielsm

  • priority changed from normal to high.

Any update on integrating this into 1.2.4? This bug is creating a major problem in my code base, so having this brought into the 1.2.4 release would be great.

06/27/07 04:35:58 changed by acts_as_flinn

  • cc set to theflinnster@gmail.com.

I just tested that patch against edge (REVISION_7136) and all test pass. Also, afaik this ticket is a dup of 3403 and 5548.

12/17/07 20:06:00 changed by tamersalama

ref #1732

02/14/08 09:13:46 changed by Somebee

Time to add yet? Bug has been known for over a year, and is fixed(?) Having problems with it right now at 2.0.2

02/14/08 15:17:25 changed by alexkwolfe

If you'd like to get the patch committed, please test the patch and give this bug a +1. It will only be considered after three +1s.

http://dev.rubyonrails.org/wiki#Creatingapatch

02/14/08 15:39:43 changed by acts_as_flinn

This was originally tested by the submitter and myself. It doesn't look like that many people are needing it but there are enough to indicate there is a problem, see dup Ticket #1732, Ticket #3403 and Ticket #5548

02/15/08 17:06:57 changed by pantoniades

Hi,

I believe I need this, and would be happy to test. Is the protocol outlined here?

Philip

04/04/08 21:36:10 changed by sebwin

I've been using this patch in production for severa months now and it resolves the issue. So here's my +1!

(follow-up: ↓ 12 ) 06/21/08 01:56:39 changed by bitsweat

  • cc changed from theflinnster@gmail.com to theflinnster@gmail.com, bitsweat.

(in reply to: ↑ 11 ) 07/25/08 18:57:13 changed by ander

+1