Article tags missing due to improper constructed SQL in ActiveRecord?

Hey folks,

I've noticed an issue with Mephisto tags that seems to
result from an  
improper constructed SQL query in Article#find_all_by_tags.
I'm not  
sure if this is a Mephisto or a ActiveRecord issue. Also,
I'm not  
even sure if it's a known issue (I've searched bug trackers
and  
Google, but found nothing).

Problem: Article#find_all_by_tags(tag_names, ...) returns
Article  
objects which are eagerly pre-populated with only those tags
that are  
specified through tag_names.

When you compare <http://
blog.hasmanythrough.com/tags/tests> with  
<http://blog.hasmanythrough.com/2007/5/2/getting-a
rbitrary-with- 
assert_difference> you see thats the tag "edge"
is missing on the  
tags page for this article. I've noticed the same behaviour
on my own  
blog, e.g. compare: <http://www
.artweb-design.de/tags/rails> to  
<http://www.artweb-design.de/2007/4/27/improved-
rails-bundled-css- 
javascript-assets-plugin>

The logs tell that there are two queries constructed, one
for  
"Article Load IDs For Limited Eager Loading", the
other one actually  
loading the articles with tags and users pre-populated. But
despite  
of this the second query uses the tag_name condition (as
such defined  
by Mephisto) - which I believe is the culprit for the issue
above.

Reading through ActiveRecord I've come to the impression
that this is  
a limitation to has_many :through eager loading. Is it?
ActiveRecord  
seems to construct the select_limited_ids_list stuff while
leaving  
the original :condition untouched. Am I missing something?

So far I've come up with this humble, but working
work-around for  
this issue: http://pastie.caboo.se/5
9436

Any ideas on this?


-- 
sven fuchs			fon:  +49 (58 45) 98 89 58
artweb design			fax:  +49 (58 45) 98 89 57
breite straße 65		www: http://www.artweb-design.
de
de-29468 bergen		mail: svenfuchsartweb-design.de



--~--~---------~--~----~------------~-------~--~----~
Mephisto: http://mephistoblog.com |
Mephisto Help: http://mephisto.stikipad
.com

To post to this group, send email to MephistoBloggooglegroups.com
To unsubscribe from this group, send email to
MephistoBlog-unsubscribegooglegroups.com
For more options, visit this group at http
://groups-beta.google.com/group/MephistoBlog
-~----------~----~----~----~------~----~------~--~---

On 5/6/07, Sven Fuchs <svenfuchsartweb-design.de>
wrote:
>
> Hey folks,
>
> I've noticed an issue with Mephisto tags that seems to
result from an
> improper constructed SQL query in
Article#find_all_by_tags. I'm not
> sure if this is a Mephisto or a ActiveRecord issue.
Also, I'm not
> even sure if it's a known issue (I've searched bug
trackers and
> Google, but found nothing).
>
> Problem: Article#find_all_by_tags(tag_names, ...)
returns Article
> objects which are eagerly pre-populated with only those
tags that are
> specified through tag_names.
>
> When you compare <http://
blog.hasmanythrough.com/tags/tests> with
> <http://blog.hasmanythrough.com/2007/5/2/getting-a
rbitrary-with-
> assert_difference> you see thats the tag
"edge" is missing on the
> tags page for this article. I've noticed the same
behaviour on my own
> blog, e.g. compare: <http://www
.artweb-design.de/tags/rails> to
> <http://www.artweb-design.de/2007/4/27/improved-
rails-bundled-css-
> javascript-assets-plugin>
>
> The logs tell that there are two queries constructed,
one for
> "Article Load IDs For Limited Eager Loading",
the other one actually
> loading the articles with tags and users pre-populated.
But despite
> of this the second query uses the tag_name condition
(as such defined
> by Mephisto) - which I believe is the culprit for the
issue above.
>
> Reading through ActiveRecord I've come to the
impression that this is
> a limitation to has_many :through eager loading. Is it?
ActiveRecord
> seems to construct the select_limited_ids_list stuff
while leaving
> the original :condition untouched. Am I missing
something?
>
> So far I've come up with this humble, but working
work-around for
> this issue: http://pastie.caboo.se/5
9436
>
> Any ideas on this?

If it's a bug with ActiveRecord, I'd rather just fix it
there.  Anyway
you can come up with a failing test case for this in the AR
tests?  A
unit-tested fix would benefit everyone.

-- 
Rick Olson
http://lighthouseapp.com

http://weblog.techno-
weenie.net
http://mephistoblog.com

--~--~---------~--~----~------------~-------~--~----~
Mephisto: http://mephistoblog.com |
Mephisto Help: http://mephisto.stikipad
.com

To post to this group, send email to MephistoBloggooglegroups.com
To unsubscribe from this group, send email to
MephistoBlog-unsubscribegooglegroups.com
For more options, visit this group at http
://groups-beta.google.com/group/MephistoBlog
-~----------~----~----~----~------~----~------~--~---

Am 07.05.2007 um 03:23 schrieb Rick Olson:
> If it's a bug with ActiveRecord, I'd rather just fix it
there.  Anyway
> you can come up with a failing test case for this in
the AR tests?  A
> unit-tested fix would benefit everyone.

Yes, I'll take a stab at it.

Thanks!


-- 
sven fuchs			fon:  +49 (58 45) 98 89 58
artweb design			fax:  +49 (58 45) 98 89 57
breite straße 65		www: http://www.artweb-design.
de
de-29468 bergen		mail: svenfuchsartweb-design.de



--~--~---------~--~----~------------~-------~--~----~
Mephisto: http://mephistoblog.com |
Mephisto Help: http://mephisto.stikipad
.com

To post to this group, send email to MephistoBloggooglegroups.com
To unsubscribe from this group, send email to
MephistoBlog-unsubscribegooglegroups.com
For more options, visit this group at http
://groups-beta.google.com/group/MephistoBlog
-~----------~----~----~----~------~----~------~--~---

Ok, here's a failing unit-test based on the AR
tests/fixtures.

http://pastie.caboo.se/5
9528

Mephisto's Article#find_all_by_tags relies on the assumption
that  
ActiveRecord behaves like spec'ed here.

I'm not really sure that ActiveRecord really should behave
as  
such ... and if it should: that it's possible to tweak
ActiveRecord  
accordingly:

# should find all posts that are tagged 'General' but
pre-populate  
all tags regardless of what's in :conditions
Post.find(1, :include => [:tags], :conditions =>
['tags.name IN (?)',  
'General'])

Personally I think that it'd be hard to fix ActiveRecord to
act this  
way.

Do you want me to raise this on the Rails mailinglist?



Am 07.05.2007 um 12:07 schrieb Sven Fuchs:
> Am 07.05.2007 um 03:23 schrieb Rick Olson:
>> If it's a bug with ActiveRecord, I'd rather just
fix it there.   
>> Anyway
>> you can come up with a failing test case for this
in the AR tests?  A
>> unit-tested fix would benefit everyone.
>
> Yes, I'll take a stab at it.
>
> Thanks!
>
>
> -- 
> sven fuchs			fon:  +49 (58 45) 98 89 58
> artweb design			fax:  +49 (58 45) 98 89 57
> breite straße 65		www: http://www.artweb-design.
de
> de-29468 bergen		mail: svenfuchsartweb-design.de
>
>
>
> >

-- 
sven fuchs			fon:  +49 (58 45) 98 89 58
artweb design			fax:  +49 (58 45) 98 89 57
breite straße 65		www: http://www.artweb-design.
de
de-29468 bergen		mail: svenfuchsartweb-design.de



--~--~---------~--~----~------------~-------~--~----~
Mephisto: http://mephistoblog.com |
Mephisto Help: http://mephisto.stikipad
.com

To post to this group, send email to MephistoBloggooglegroups.com
To unsubscribe from this group, send email to
MephistoBlog-unsubscribegooglegroups.com
For more options, visit this group at http
://groups-beta.google.com/group/MephistoBlog
-~----------~----~----~----~------~----~------~--~---

Hey Rick,

don't know if you've seen this thread on the ror users list:
http:// 
www.ruby-forum.com/topic/108163

The underlying ActiveRecord behaviour (:condition effects
:included/ 
eagerly loaded tags) seems to be not that easily changed.

What would you propose to do about this? Do you want to have
a look  
at this issue/AR behaviour yourself to probably
"fix" that in AR? Do  
you want me to add a ticket/patch to change Mephisto?

Thanks!

Sven

Am 07.05.2007 um 03:23 schrieb Rick Olson:
> On 5/6/07, Sven Fuchs <svenfuchsartweb-design.de> wrote:
>>
>> Hey folks,
>>
>> I've noticed an issue with Mephisto tags that seems
to result from an
>> improper constructed SQL query in
Article#find_all_by_tags. I'm not
>> sure if this is a Mephisto or a ActiveRecord issue.
Also, I'm not
>> even sure if it's a known issue (I've searched bug
trackers and
>> Google, but found nothing).
>>
>> Problem: Article#find_all_by_tags(tag_names, ...)
returns Article
>> objects which are eagerly pre-populated with only
those tags that are
>> specified through tag_names.
>>
>> When you compare <http://
blog.hasmanythrough.com/tags/tests> with
>> <http://blog.hasmanythrough.com/2007/5/2/getting-a
rbitrary-with-
>> assert_difference> you see thats the tag
"edge" is missing on the
>> tags page for this article. I've noticed the same
behaviour on my own
>> blog, e.g. compare: <http://www
.artweb-design.de/tags/rails> to
>> <http://www.artweb-design.de/2007/4/27/improved-
rails-bundled-css-
>> javascript-assets-plugin>
>>
>> The logs tell that there are two queries
constructed, one for
>> "Article Load IDs For Limited Eager
Loading", the other one actually
>> loading the articles with tags and users
pre-populated. But despite
>> of this the second query uses the tag_name
condition (as such defined
>> by Mephisto) - which I believe is the culprit for
the issue above.
>>
>> Reading through ActiveRecord I've come to the
impression that this is
>> a limitation to has_many :through eager loading. Is
it? ActiveRecord
>> seems to construct the select_limited_ids_list
stuff while leaving
>> the original :condition untouched. Am I missing
something?
>>
>> So far I've come up with this humble, but working
work-around for
>> this issue: http://pastie.caboo.se/5
9436
>>
>> Any ideas on this?
>
> If it's a bug with ActiveRecord, I'd rather just fix it
there.  Anyway
> you can come up with a failing test case for this in
the AR tests?  A
> unit-tested fix would benefit everyone.
>
> -- 
> Rick Olson
> http://lighthouseapp.com

> http://weblog.techno-
weenie.net
> http://mephistoblog.com
>
> >

-- 
sven fuchs			fon:  +49 (58 45) 98 89 58
artweb design			fax:  +49 (58 45) 98 89 57
breite straße 65		www: http://www.artweb-design.
de
de-29468 bergen		mail: svenfuchsartweb-design.de



--~--~---------~--~----~------------~-------~--~----~
Mephisto: http://mephistoblog.com |
Mephisto Help: http://mephisto.stikipad
.com

To post to this group, send email to MephistoBloggooglegroups.com
To unsubscribe from this group, send email to
MephistoBlog-unsubscribegooglegroups.com
For more options, visit this group at http
://groups-beta.google.com/group/MephistoBlog
-~----------~----~----~----~------~----~------~--~---