This project is archived and is in readonly mode.

#5124 ✓resolved
David Genord II

[PATCH] ActiveRecord count fails when run with multiple includes and a join

Reported by David Genord II | July 15th, 2010 @ 04:45 PM

If you run count on an ActiveRecord model with multiple includes and a join it will fail because it is unable to find the association to include. JoinDependency#graft does not specify the base join when a suitable join is not found for the association being grafted in the dependency causing the second include to happen against the wrong join.

I have created a simple patch with a test which triggers the issue and fixes it. The test is not a great use case, but it is simple and triggers the error, so it is all I think we need.

Comments and changes to this ticket

  • Ken Collins

    Ken Collins July 15th, 2010 @ 07:58 PM

    Is this related to #4869?

    Just curious. When I found the bug in #4869 it was under a count that had an belongs_to include. I just saw my patch for it was applied to 2.3.x but still is in master for 3.x.

  • Ernie Miller

    Ernie Miller July 15th, 2010 @ 08:14 PM

    Good catch. As the guy who did the original refactor in associations.rb to add the graft method, I wanted to weigh in.

    def find_parent_in(other_join_dependency)

    other_join_dependency.joins.detect do |join|
      self.parent == join
    end
    

    end

    The other JoinBase is other_join_dependency.joins.first, so if it were equal it would be getting used. I'm assuming that this is happening because they're technically not "equal" -- that is, one JoinBase has preexisting values in its table_joins attribute. I'm not saying we should change "==" on JoinBase because it's technically correct to consider all attributes in testing equality, but just giving a bit more background as to what's going on.

    It seems like your fallback is as good as any, as if the association doesn't exist on the JoinBase it'll raise an error during build.

    So I guess that's a long way to saying: +1. :p

  • David Genord II

    David Genord II July 15th, 2010 @ 08:22 PM

    Ken
    These errors do not appear to be linked. To trigger this error you must be using both includes and joins, running a count operation, and it only effects association lookup.

  • Repository

    Repository July 16th, 2010 @ 08:30 AM

    • State changed from “new” to “resolved”

    (from [1fcf4e8ecbe4db2443a88d1e06c06667cff5b762]) JoinDependency#graft does not properly set parent join [#5124 state:resolved]

    Signed-off-by: Pratik Naik pratiknaik@gmail.com
    http://github.com/rails/rails/commit/1fcf4e8ecbe4db2443a88d1e06c066...

  • Anatoliy Lysenko

    Anatoliy Lysenko October 19th, 2010 @ 07:34 PM

    I think the original test isn't good enough, so your fix doesn't work for me.

    When you use plain include, when each included association is associated with base model it works:
    Author.joins(:special_posts).includes([:posts, :categorizations])
    When find_parents_in(self) can't find parent for posts we just join it to Author - pass.

    But fail when you use cascade includes.
    Category.joins(:categorizations).includes([{:posts=>:comments}, :authors])
    When find_parents_in(self) can't find parent for :posts we join it to Category - pass.
    When find_parents_in(self) can't find parent for :comments we join to Category - fail. "ActiveRecord::ConfigurationError: Association named 'comments' was not found; perhaps you misspelled it?". Because Category haven't comments association.

    I can't find the reason, so I just change JoinBase#== method as Ernie suggested.

    Patch is attached. But it fix only symptom... not sure...

  • Anatoliy Lysenko

    Anatoliy Lysenko October 20th, 2010 @ 08:25 AM

    I dislike my patch and create separate ticket #5845

  • Jeff Kreeftmeijer

    Jeff Kreeftmeijer November 8th, 2010 @ 08:25 AM

    • Tag cleared.
    • Importance changed from “” to “Low”

    Automatic cleanup of spam.

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>

Attachments

Referenced by

Pages