[PATCH] find(:first) forgets the 'LIMIT 1' when eager loading is used, severely hurting MySQL performance in some cases

Doesn't the join require no limit, otherwise if you have multiple dependent records you will only get one of them?

Replying to evan:

Doesn't the join require no limit, otherwise if you have multiple dependent records you will only get one of them?

In the case of a belongs_to or a has_one, there'll be at most one associated row, so it should include the LIMIT 1 when it's find(:first). This was the case I described, although perhaps I didn't make it clear enough.

That said, in the case of eager loading of a has_many or has_and_belongs_to_many, I suggest it should ignore the eager loading when this is done with a find(:first), as the performance penalty of missing out the LIMIT and hence having to join the entire table instead of just the single row, would more than negate the small benefit from avoiding one extra query to find the associated objects of that one first row.

Just a heads-up: the bug still exists in edge and I'm working on it.

I attached a patch with tests that fixes this issue with :belongs_to and :has_one associations.

I agree that the whole :include option is highly questionable when using find(:first). Fixing it would be relatively easy, just changing

          options.update(:limit => 1) unless options[:include]

to

          options.update(:limit => 1)
          options.delete(:include)

in find_initial (I think). However, making that change breaks a lot of existing tests (read assumptions) so I would like to hear the core's opinion on that.

But like said, the current fix doesn't break anything and fixes the issue for the most obvious cases.

Oh, forgot to say, I tested the current patch with MySQL, PostgreSQL and SQLite, but it should work also for Oracle and DB2 (testing against /LIMIT 1|rownum <= 1/). Haven't a chance to test it against them, though.

Updated patch to test both belongs_to and has_one cases.

I think find_every is now clever enough to limit returned results no matter what associations are included. So find_initial could be reduced to:

def find_initial(options)
  options.update(:limit => 1)
  find_every(options).first
end

I've discussed this with jarkko and will investigate further.

Note that a workaround in the meantime is to use:

find(:all, :limit => 1).first