Odditiy in find_initial

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

У Чцв, 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.

У Чцв, 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).

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

У Чцв, 24/04/2008 у 12:26 +0300, Aliaksey Kandratsenka піша:

У Чцв, 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.

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

0001-add-limit-1-in-ActiveRecord-Base-find_initial.patch (855 Bytes)

У Чцв, 24/04/2008 у 10:45 +0100, Frederick Cheung піша:

>
> У Чцв, 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).
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.

What happens when you specify a :conditions hash that relies on the
included association?

У Чцв, 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.

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/361aaa04ef2f33cb1fe49497c73bc13e6b72addc

Cheers!