Forum: Rails-core (closed, excessive spam) Odditiy in find_initial

81b61875e41eaa58887543635d556fca?d=identicon&s=25 Frederick Cheung (Guest)
on 2008-04-24 11:05
(Received via mailing list)
I stumbled across this today:

def find_initial(options)
   options.update(:limit => 1) unless options[:include]
   find_every(options).first
end

And I can't think why the special casing of include is necessary.
Maybe there was a time when  :include didn't do the right thing, but
for at least some time :include has handled this properly, and so any
one doing Foo.find :first, :include => [...] is inadvertently making
rails and the db do a lot more work than they think?
Is this just a relic or is there some reason I missed ?

Fred
9ad79288563ad0d55baba41432adc6d2?d=identicon&s=25 Aliaksey Kandratsenka (Guest)
on 2008-04-24 11:26
(Received via mailing list)
У Чцв, 24/04/2008 у 10:04 +0100, Frederick Cheung піша:
> I stumbled across this today:
>
> def find_initial(options)
>    options.update(:limit => 1) unless options[:include]
>    find_every(options).first
> end
Several month ago I asked same question. I didn't searched history, but
I guess it's artifact of some very ancient times.

It works perfectly without this (actually, with :include it always
fetches whole result set and cuts it in memory, which is stupid).

Our local version of rails has this turned off for almost year now. And
has it long before new :include implementation landed. So it's safe to
kill this. I hadn't yet time to post a patch for this though.
>
> And I can't think why the special casing of include is necessary.
> Maybe there was a time when  :include didn't do the right thing, but
> for at least some time :include has handled this properly, and so any
> one doing Foo.find :first, :include => [...] is inadvertently making
> rails and the db do a lot more work than they think?
> Is this just a relic or is there some reason I missed ?
>
> Fred

--
Aliaksey Kandratsenka <alkondratenko@gmail.com>
81b61875e41eaa58887543635d556fca?d=identicon&s=25 Frederick Cheung (Guest)
on 2008-04-24 11:46
(Received via mailing list)
On 24 Apr 2008, at 10:25, Aliaksey Kandratsenka wrote:

> I guess it's artifact of some very ancient times.
>
> It works perfectly without this (actually, with :include it always
> fetches whole result set and cuts it in memory, which is stupid).
Pretty much what I thought. I can't quite think how I'd write a test
to verify that it's not doing this though.

Fred
9ad79288563ad0d55baba41432adc6d2?d=identicon&s=25 Aliaksey Kandratsenka (Guest)
on 2008-04-24 12:00
(Received via mailing list)
У Чцв, 24/04/2008 у 12:26 +0300, Aliaksey Kandratsenka піша:
> It works perfectly without this (actually, with :include it always
> fetches whole result set and cuts it in memory, which is stupid).
>
> Our local version of rails has this turned off for almost year now. And
> has it long before new :include implementation landed. So it's safe to
> kill this. I hadn't yet time to post a patch for this though.

Actually, with git it's easier. Here we go.

--
Aliaksey Kandratsenka <alkondratenko@gmail.com>
9ad79288563ad0d55baba41432adc6d2?d=identicon&s=25 Aliaksey Kandratsenka (Guest)
on 2008-04-24 12:03
(Received via mailing list)
У Чцв, 24/04/2008 у 10:45 +0100, Frederick Cheung піша:
> >> end
> > Several month ago I asked same question. I didn't searched history,
> > but
> > I guess it's artifact of some very ancient times.
> >
> > It works perfectly without this (actually, with :include it always
> > fetches whole result set and cuts it in memory, which is stupid).
> Pretty much what I thought. I can't quite think how I'd write a test
> to verify that it's not doing this though.
It seems, only mocking can help.
>
> Fred

--
Aliaksey Kandratsenka <alkondratenko@gmail.com>
65654a5946d86bff45a14e157ad3cf14?d=identicon&s=25 Xavier Shay (Guest)
on 2008-04-25 05:27
(Received via mailing list)
What happens when you specify a :conditions hash that relies on the
included association?

On Apr 24, 7:04 pm, Frederick Cheung <frederick.che...@gmail.com>
9ad79288563ad0d55baba41432adc6d2?d=identicon&s=25 Aliaksey Kandratsenka (Guest)
on 2008-04-27 09:28
(Received via mailing list)
У Чцв, 24/04/2008 у 20:26 -0700, Xavier Shay піша:
> What happens when you specify a :conditions hash that relies on the
> included association?
>
It's not very obvious, but it's supported.
Whole bunch of code is there to support it (and according to git history
it was there since late 2005 (svn r2675).
Look at
ActiveRecord::Associations::ClassMethods#add_limited_ids_condition!

Not adding limit was commited in r1811 along with initial support
for :limit and :include. Actually, even then it could be smarter.
I.e. :limit => 1 could be left if all included associations
are :belong_to or :has_one.

Then when proper support was implemented, somebody simply forgot to
remove it.

--
Aliaksey Kandratsenka <alkondratenko@gmail.com>
Efa76b164a7de4a5730e4fa397cc4425?d=identicon&s=25 Michael Koziarski (Guest)
on 2008-04-27 10:04
(Received via mailing list)
>  Then when proper support was implemented, somebody simply forgot to
>  remove it.

Thanks for looking into that.  I've checked and we have what *looks*
like fairly decent coverage in eager_test, so I've removed it.

http://github.com/rails/rails/commit/361aaa04ef2f3...

Cheers!

--
Cheers

Koz
This topic is locked and can not be replied to.