Rails core - Feb 2008 - Dynamic finders on associations not respecting :include

In trying to diagnose an error I was getting, I found

http://dev.rubyonrails.org/ticket/10211 added :order respect
http://dev.rubyonrails.org/ticket/10227 added :limit

These both patch the construct_scope methods in the ActiveRecord
association classes to pass through the @reflection.options for the
respective option.

My problem was the :include wasn''t getting sent through and my order
was then failing since it used a field retrieved from the include.

Can anyone see a reason why :include shouldn''t be send through as
well?  I''ve done a quick fix on my system and it solved my problem,
but before I try to submit a patch I wanted to see if anyone knew a
reason it wasn''t?

Also, do you think the patch should try to be smart and just send
through @reflection.options in general so in the future we don''t have
to forget adding an option to these few different classes?

An aside: I noticed that the find_or_create_by doesn''t suppose the
passing of any options, unless the find_by -- e.g.
find_or_create_by_title_id(@title.id, :include => :title) doesn''t
work.  Should it?  find_by_title_id(@title.id, :include => :title)
does.

Thanks for your time,
Chris Nolan.ca
http://kekova.ca/
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

On Feb 16, 2008 11:04 AM, Chris Nolan.ca <chrisnolan.ca@gmail.com> wrote:
> Can anyone see a reason why :include shouldn''t be send through as
> well?  I''ve done a quick fix on my system and it solved my
problem,
> but before I try to submit a patch I wanted to see if anyone knew a
> reason it wasn''t?
>
This has been a fairly pervasive problem.  Of the many different methods
used to generate SQL there is varying support for options passed explicitly
to the finder, and options defined by the current scope, options defined by
a reflection.

We''ve been patching holes as we see them but there''s a long
way to go.  I''ve
still got a ticket for one open here: http://dev.rubyonrails.org/ticket/9861

>
> Also, do you think the patch should try to be smart and just send
> through @reflection.options in general so in the future we don''t
have
> to forget adding an option to these few different classes?

Personally, I''d like to see full support for all 3 ways of defining
options
for a database call.  It''s a pretty extensive problem though so, unless
there''s a majority of developers clamoring for it, I''d
recommend limiting
the scope of your patch.

It would be great if you could add some other patches that fix problems one
by one.

::Jack Danger Canty

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

Yes, I''ve got a related patch http://dev.rubyonrails.org/ticket/10990
about :include not being honored on has_many :through.

-Chad

On Feb 16, 2:31 pm, "Jack Danger Canty"
<dangeronra...@gmail.com>
wrote:
> On Feb 16, 2008 11:04 AM, Chris Nolan.ca <chrisnolan...@gmail.com>
wrote:
>
> > Can anyone see a reason why :include shouldn''t be send
through as
> > well?  I''ve done a quick fix on my system and it solved my
problem,
> > but before I try to submit a patch I wanted to see if anyone knew a
> > reason it wasn''t?
>
> This has been a fairly pervasive problem.  Of the many different methods
> used to generate SQL there is varying support for options passed explicitly
> to the finder, and options defined by the current scope, options defined by
> a reflection.
>
> We''ve been patching holes as we see them but there''s a
long way to go.  I''ve
> still got a ticket for one open here:http://dev.rubyonrails.org/ticket/9861
>
>
>
> > Also, do you think the patch should try to be smart and just send
> > through @reflection.options in general so in the future we
don''t have
> > to forget adding an option to these few different classes?
>
> Personally, I''d like to see full support for all 3 ways of
defining options
> for a database call.  It''s a pretty extensive problem though so,
unless
> there''s a majority of developers clamoring for it, I''d
recommend limiting
> the scope of your patch.
>
> It would be great if you could add some other patches that fix problems one
> by one.
>
> ::Jack Danger Canty
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

> Can anyone see a reason why :include shouldn''t be send through as
> well?  I''ve done a quick fix on my system and it solved my
problem,
> but before I try to submit a patch I wanted to see if anyone knew a
> reason it wasn''t?
Nope, just a bug / missing feature.  There are a few things brewing
which would alleviate this kind of thing, but they''re unlikely to land
for 2.1.
> An aside: I noticed that the find_or_create_by doesn''t suppose the
> passing of any options, unless the find_by -- e.g.
> find_or_create_by_title_id(@title.id, :include => :title)
doesn''t
> work.  Should it?  find_by_title_id(@title.id, :include => :title)
> does.
I think that find_or_create_... takes an attributes option as its
second argument, so this kind of thing is a little difficult.


-- 
Cheers

Koz

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---