Rails Ruby on Rails → 2.x

Comments and changes to this ticket

• You flagged this item as spam.
  

  David Stevenson October 9th, 2008 @ 05:34 PM

  Here's the patch... pretty trivial.

  • ar-option-to-disable-single-query-includes.diff 5.7 KB

• You flagged this item as spam.
  

  David Stevenson October 9th, 2008 @ 05:44 PM

  • Tag changed from “activerecord, config, include, query, single” to “activerecord, config, include, join, left, outer, patch, query, single”

  Previous patch had a bad line in it, which didn't change the functionality but was misleading. Removed line from the diff and resubmitting.

  • ar-option-to-disable-single-query-includes-fixed.diff 5.2 KB
• 

  DHH October 16th, 2008 @ 09:15 AM

  • State changed from “new” to “wontfix”

  I don't understand the argument that "projects that are serious about include performance" should only want this. If we can come up with a fail-safe heuristic that is always faster, why wouldn't everyone want that? I take it that there are specific reasons why the previous style includes are still being selected at times.

  In any case, I don't see it being a general option. If anything, it should be a per-query option. But not until we've tried to make the underlying selector better.

• 

  Frederick Cheung October 16th, 2008 @ 11:17 AM

  We do have heuristics, but they don't always get it right. In particular they don't work when you specify a table via the :joins option, largely because in the most general case parsing a join statement is a) hard and b) database dependant (we might be able to do something for the simpler :joins => :some_association case)

• You flagged this item as spam.
  

  David Stevenson October 16th, 2008 @ 06:50 PM

  I think it might be possible to find all the tables from :joins, but it would probably be easier to approach this problem the other way around:

  Why don't we make a blacklist of tables that trigger a fallback, rather than a whitelist of allowed tables? It's easier to find all the "bad" tables by traversing the include chain. This wouldn't allow for the case when you want to :join and :include from the same table, however, but would be overall an improvement.

  Since the performance difference between the old-style and new-style is pretty big, I'd imagine that high performance projects are going to want to switch ALL their queries over. I think the best way to accomplish that is to just disable the old style includes. With good test coverage, it's easy to find the queries that reference included tables and fix them up... this only took us about 15-30 minutes.

• You flagged this item as spam.
  

  David Stevenson October 16th, 2008 @ 06:52 PM

  I had originally opened this ticket:

  http://rails.lighthouseapp.com/p...

  which I guess I could try to come up with a patch for. But it seems to be orthogonal to this configuration ticket. Even if it guesses better, it would still be useful to disable these across the board (especially for finding "bad queries" that reference included tables).

• 

  Frederick Cheung October 16th, 2008 @ 07:10 PM

  Even that can get hairy when table aliases are involved but at that point it's quite tricky to write the conditions anyway. Definitely worth a try

  On 16 Oct 2008, at 18:50, Lighthouse support@lighthouseapp.com wrote: