Okay, so I've realized that just posting an "oi, patch over here" message doesn't quite convince anyone to spend their time sorting through the whole thing themselves, so here's a run-down of the ticket and my patch, in the hopes of convincing rails-core that the bug is important and the patch is solid.
The problem: Author.find(1).posts != Author.find(1, :include => [:posts]).posts if Author has_many :posts, :order => anything The :order option of an association isn't respected by eager loading, which forces developers to either repeat the :order option for each eagerly loaded find, or to actually sort things themselves. The workarounds are neither pretty, DRY, nor necessary. This is not a design decision nor a technical limitation; this is just functionality which never got added in. The solution: My patch adds ~20 lines of code to ActiveRecord::Associations::ClassMethods#construct_finder_sql_with_included_associations which run through the various associations in the join dependency, extract all :order options, expand them to their full form (e.g., 'posts.published_at' instead of 'published_at', also minding table aliases), and append them to the SQL's ORDER BY clause. If there is no existing :order option in the find itself, the ORDER BY clause begins with the primary key of the model being searched, which is the default SQL order. This patch passes all existing unit tests without modification, and I've added two units tests which have a combined total of 38 assertions which check that the eagerly-loaded and non-eagerly-loaded associations both have the exact same order. The code is written to standards, has comments to explain any tricky bits, and doesn't rely on high magic. It doesn't break any existing code, works with STI/explicit class names/primary keys, doesn't overshadow the find method's :order options, and smells faintly of apple pie. So please, take a look at this. I'm willing to go point on this bug, and if the patch isn't satisfying, I'm willing to do whatever it takes to get it in. Finally, I'm willing to resort to bribery/capitalism and buy any member of rails-core in the San Francisco Bay Area a pint (or equivalent) who either accepts this patch or fixes this bug. The ticket: http://dev.rubyonrails.org/ticket/3438 My patch: http://dev.rubyonrails.org/attachment/ticket/3438/eager_loading_order_patch_with_unit_tests.diff Thanks! -- Coda Hale http://blog.codahale.com _______________________________________________ Rails-core mailing list Rails-core@lists.rubyonrails.org http://lists.rubyonrails.org/mailman/listinfo/rails-core
