take-five / activerecord-hierarchical_query

Simple DSL for creating recursive queries with ActiveRecord
MIT License
119 stars 24 forks source link

Fixes #join_recursive for models with an order clause in the default scope #15

Closed walski closed 7 years ago

walski commented 7 years ago

Hey,

I've been trying to use the gem for a model that uses an order statement in it's default scope like this:

class LinkedItem < ActiveRecord::Base
  belongs_to :parent, class_name: 'LinkedItem', foreign_key: :parent_id

  default_scope -> { order('name ASC') }
end

This lead to queries as:

SELECT "linked_items".* FROM "linked_items" INNER JOIN (WITH RECURSIVE "linked_items__recursive" AS ( SELECT "linked_items"."id", "linked_items"."parent_id" FROM "linked_items" WHERE "linked_items"."id" = $1 UNION ALL SELECT "linked_items"."id", "linked_items"."parent_id" FROM "linked_items" INNER JOIN "linked_items__recursive" ON "linked_items__recursive"."parent_id" = "linked_items"."id"  ORDER BY name ASC ) SELECT "linked_items__recursive".* FROM "linked_items__recursive") AS "linked_items__recursive" ON "linked_items"."id" = "linked_items__recursive"."id"

which failed on PostgreSQL due to the ORDER BY name ASC part with this error:

ERROR:  ORDER BY in a recursive query is not implemented at character 404

This fixes the problem by allowing to add a reorder statement to the child scope.

walski commented 7 years ago

@take-five any idea why this is failing for AR < 4.1

coveralls commented 7 years ago

Coverage Status

Changes Unknown when pulling a8eec1f1df405fac170ef7f397e3a15b0ffa7a59 on walski:fixForDefaultScopeOrdering into \ on take-five:master**.

take-five commented 7 years ago

Hey @walski Thanks for the report. I appreciate your PR, but I don't fully agree on proposed solution. I think it's better to remove any default orderings when building recursive query (default scope will apply to outer resulting query anyway).

Please check out this commit https://github.com/take-five/activerecord-hierarchical_query/commit/1b0e034a2cc4114dac2b16e501b0c856ab98e3ab

gem 'activerecord-hierarchical_query', github: 'take-five/activerecord-hierarchical_query', ref: '1b0e034a2c'

Please let me know if it works for you and I'll release patched version.

walski commented 7 years ago

Hey @take-five that works for me, as well. I've tested it in our project and all specs are green :+1:

Thanks for you efforts!

take-five commented 7 years ago

@walski Version 0.1.5 has been pushed to rubygems.org