This project is archived and is in readonly mode.
Old-single-query-includes should be globally disableable
-
David StevensonHere's the patch... pretty trivial.
-
David Stevenson
- 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.
-
DHH- 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 CheungWe 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)
-
David Stevenson
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.
-
David Stevenson
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
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:
