This project is archived and is in readonly mode.

#1104 ✓duplicate
David Stevenson

references_eager_loaded_tables? should search tables in :join clauses

Reported by David Stevenson | September 24th, 2008 @ 05:00 PM | in 2.x

When deciding whether to use the new-include-syntax or the old-include-syntax, activerecord searches :select, :order, and :conditions clauses for tables that are != to the base finder's table name. If there are any matches, it falls back to the old style.

This should be updated to ignore tables found in :join clauses. Without this update, the old include syntax fallback will happen more conservatively than it needs to.

We are experiencing this so often that we've hard coded references_eager_loaded_tables? to always return false so we're always using the new style. Of course, that would break lots of stuff, so this is a good compromise.

Patch to follow.

David Stevenson & Joseph Palermo (Pivotal Labs)

Comments and changes to this ticket

  • Frederick Cheung

    Frederick Cheung October 7th, 2008 @ 12:01 AM

    This looks juicy :-) With 2.2RC1 around the corner, would be great to see the patch!

  • David Stevenson

    David Stevenson October 7th, 2008 @ 02:40 AM

    So... this is actually hard. It's not easy to write a REGEX that works to find all the joined in tables from a string. It's not as easy as looking for TABLE_NAME.FIELD_NAME like rails does for everything else.

    Joseph and I have since disabled pre-2.1 style includes across our app. This turned out to be better for us than making rails "guess" better at which style of includes to use.

    More and more, people will be switching away from old includes for performance reasons, so I think the better approach here might be to provide a global configuration on ActiveRecord to disable single-query includes. Naturally, it won't work for every query, but anyone who cares about performance will be optimizing all their queries to work anyway. Without the ability to disable these old-style includes completely, you're always going to be stuck trying to trick rails into believing that your query is safe. No matter how good we make the guessing system, there will always be incorrect guesses, so let's solve this problem at the core.

  • David Stevenson

    David Stevenson October 9th, 2008 @ 04:51 PM

    Opened this ticket instead:

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

  • Rob Sanheim

    Rob Sanheim October 17th, 2008 @ 05:04 AM

    What do you mean by "new include syntax" versus the "old include syntax" ?

  • David Stevenson

    David Stevenson October 17th, 2008 @ 06:12 AM

    Sorry - new include syntax is not a syntax but an implementation. It's the process of preloading associations using a separate query for each level of the include chain.

    The old include "syntax" created a single query to prefetch all the associated data. This was usually slower, but allowed you to use conditions on tables in the include chain.

  • Frederick Cheung

    Frederick Cheung December 17th, 2008 @ 04:00 PM

    • State changed from “new” to “duplicate”

    #528 is all about this (and has patches too)

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

<h2 style="font-size: 14px">Tickets have moved to Github</h2>

The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>

People watching this ticket

Tags

Referenced by

Pages