refactor: convert SQL queries to Arel where feasible (#453)

seuros · web-flow · commit 23f8a567fef4 · 2025-07-21T01:02:46.000+01:00
diff --git a/lib/closure_tree/arel_helpers.rb b/lib/closure_tree/arel_helpers.rb
@@ -0,0 +1,83 @@
+# frozen_string_literal: true
+
+module ClosureTree
+  module ArelHelpers
+    # Get model's arel table
+    def model_table
+      @model_table ||= model_class.arel_table
+    end
+
+    # Get hierarchy table from a model class
+    # This method should be called from instance methods where hierarchy_class is available
+    def hierarchy_table_for(model)
+      if model.respond_to?(:hierarchy_class)
+        model.hierarchy_class.arel_table
+      elsif model.class.respond_to?(:hierarchy_class)
+        model.class.hierarchy_class.arel_table
+      else
+        raise ArgumentError, "Cannot find hierarchy_class for #{model}"
+      end
+    end
+
+    # Get hierarchy table using the model_class
+    # This is for Support class methods
+    def hierarchy_table
+      @hierarchy_table ||= begin
+        hierarchy_class_name = options[:hierarchy_class_name] || "#{model_class}Hierarchy"
+        hierarchy_class_name.constantize.arel_table
+      end
+    end
+
+    # Helper to create an Arel node for a table with an alias
+    def aliased_table(table, alias_name)
+      table.alias(alias_name)
+    end
+
+    # Build Arel queries for hierarchy operations
+    def build_hierarchy_insert_query(hierarchy_table, node_id, parent_id)
+      x = aliased_table(hierarchy_table, 'x')
+
+      # Build the SELECT subquery - use SelectManager
+      select_query = Arel::SelectManager.new(x)
+      select_query.project(
+        x[:ancestor_id],
+        Arel.sql(quote(node_id)),
+        x[:generations] + 1
+      )
+      select_query.where(x[:descendant_id].eq(parent_id))
+
+      # Build the INSERT statement
+      insert_manager = Arel::InsertManager.new
+      insert_manager.into(hierarchy_table)
+      insert_manager.columns << hierarchy_table[:ancestor_id]
+      insert_manager.columns << hierarchy_table[:descendant_id]
+      insert_manager.columns << hierarchy_table[:generations]
+      insert_manager.select(select_query)
+
+      insert_manager
+    end
+
+    def build_hierarchy_delete_query(hierarchy_table, id)
+      # Build the innermost subquery
+      inner_subquery_manager = Arel::SelectManager.new(hierarchy_table)
+      inner_subquery_manager.project(hierarchy_table[:descendant_id])
+      inner_subquery_manager.where(
+        hierarchy_table[:ancestor_id].eq(id)
+        .or(hierarchy_table[:descendant_id].eq(id))
+      )
+      inner_subquery = inner_subquery_manager.as('x')
+
+      # Build the middle subquery with DISTINCT
+      middle_subquery = Arel::SelectManager.new
+      middle_subquery.from(inner_subquery)
+      middle_subquery.project(inner_subquery[:descendant_id]).distinct
+
+      # Build the DELETE statement
+      delete_manager = Arel::DeleteManager.new
+      delete_manager.from(hierarchy_table)
+      delete_manager.where(hierarchy_table[:descendant_id].in(middle_subquery))
+
+      delete_manager
+    end
+  end
+end
diff --git a/lib/closure_tree/finders.rb b/lib/closure_tree/finders.rb
@@ -37,15 +37,24 @@ def find_or_create_by_path(path, attributes = {})
     end
 
     def find_all_by_generation(generation_level)
-      s = _ct.base_class.joins(<<-SQL.squish)
-        INNER JOIN (
-          SELECT descendant_id
-          FROM #{_ct.quoted_hierarchy_table_name}
-          WHERE ancestor_id = #{_ct.quote(id)}
-          GROUP BY descendant_id
-          HAVING MAX(#{_ct.quoted_hierarchy_table_name}.generations) = #{generation_level.to_i}
-        ) #{_ct.t_alias_keyword} descendants ON (#{_ct.quoted_table_name}.#{_ct.base_class.primary_key} = descendants.descendant_id)
-      SQL
+      hierarchy_table = self.class.hierarchy_class.arel_table
+      model_table = self.class.arel_table
+
+      # Build the subquery
+      descendants_subquery = hierarchy_table
+                             .project(hierarchy_table[:descendant_id])
+                             .where(hierarchy_table[:ancestor_id].eq(id))
+                             .group(hierarchy_table[:descendant_id])
+                             .having(hierarchy_table[:generations].maximum.eq(generation_level.to_i))
+                             .as('descendants')
+
+      # Build the join
+      join_source = model_table
+                    .join(descendants_subquery)
+                    .on(model_table[_ct.base_class.primary_key].eq(descendants_subquery[:descendant_id]))
+                    .join_sources
+
+      s = _ct.base_class.joins(join_source)
       _ct.scope_with_order(s)
     end
 
@@ -72,14 +81,23 @@ def root
       end
 
       def leaves
-        s = joins(<<-SQL.squish)
-          INNER JOIN (
-            SELECT ancestor_id
-            FROM #{_ct.quoted_hierarchy_table_name}
-            GROUP BY ancestor_id
-            HAVING MAX(#{_ct.quoted_hierarchy_table_name}.generations) = 0
-          ) #{_ct.t_alias_keyword} leaves ON (#{_ct.quoted_table_name}.#{primary_key} = leaves.ancestor_id)
-        SQL
+        hierarchy_table = hierarchy_class.arel_table
+        model_table = arel_table
+
+        # Build the subquery for leaves (nodes with no children)
+        leaves_subquery = hierarchy_table
+                          .project(hierarchy_table[:ancestor_id])
+                          .group(hierarchy_table[:ancestor_id])
+                          .having(hierarchy_table[:generations].maximum.eq(0))
+                          .as('leaves')
+
+        # Build the join
+        join_source = model_table
+                      .join(leaves_subquery)
+                      .on(model_table[primary_key].eq(leaves_subquery[:ancestor_id]))
+                      .join_sources
+
+        s = joins(join_source)
         _ct.scope_with_order(s.readonly(false))
       end
 
@@ -123,22 +141,41 @@ def lowest_common_ancestor(*descendants)
       end
 
       def find_all_by_generation(generation_level)
-        s = joins(<<-SQL.squish)
-          INNER JOIN (
-            SELECT #{primary_key} as root_id
-            FROM #{_ct.quoted_table_name}
-            WHERE #{_ct.quoted_parent_column_name} IS NULL
-          ) #{_ct.t_alias_keyword}  roots ON (1 = 1)
-          INNER JOIN (
-            SELECT ancestor_id, descendant_id
-            FROM #{_ct.quoted_hierarchy_table_name}
-            GROUP BY ancestor_id, descendant_id
-            HAVING MAX(generations) = #{generation_level.to_i}
-          ) #{_ct.t_alias_keyword}  descendants ON (
-            #{_ct.quoted_table_name}.#{primary_key} = descendants.descendant_id
-            AND roots.root_id = descendants.ancestor_id
-          )
-        SQL
+        hierarchy_table = hierarchy_class.arel_table
+        model_table = arel_table
+
+        # Build the roots subquery
+        roots_subquery = model_table
+                         .project(model_table[primary_key].as('root_id'))
+                         .where(model_table[_ct.parent_column_sym].eq(nil))
+                         .as('roots')
+
+        # Build the descendants subquery
+        descendants_subquery = hierarchy_table
+                               .project(
+                                 hierarchy_table[:ancestor_id],
+                                 hierarchy_table[:descendant_id]
+                               )
+                               .group(hierarchy_table[:ancestor_id], hierarchy_table[:descendant_id])
+                               .having(hierarchy_table[:generations].maximum.eq(generation_level.to_i))
+                               .as('descendants')
+
+        # Build the joins
+        # Note: We intentionally use a cartesian product join (CROSS JOIN) here.
+        # This allows us to find all nodes at a specific generation level across all root nodes.
+        # The 1=1 condition creates this cartesian product in a database-agnostic way.
+        join_roots = model_table
+                     .join(roots_subquery)
+                     .on(Arel.sql('1 = 1'))
+
+        join_descendants = join_roots
+                           .join(descendants_subquery)
+                           .on(
+                             model_table[primary_key].eq(descendants_subquery[:descendant_id])
+                             .and(roots_subquery[:root_id].eq(descendants_subquery[:ancestor_id]))
+                           )
+
+        s = joins(join_descendants.join_sources)
         _ct.scope_with_order(s)
       end
 
@@ -151,6 +188,7 @@ def find_by_path(path, attributes = {}, parent_id = nil)
 
         scope = where(path.pop)
         last_joined_table = _ct.table_name
+
         path.reverse.each_with_index do |ea, idx|
           next_joined_table = "p#{idx}"
           scope = scope.joins(<<-SQL.squish)
@@ -161,6 +199,7 @@ def find_by_path(path, attributes = {}, parent_id = nil)
           scope = _ct.scoped_attributes(scope, ea, next_joined_table)
           last_joined_table = next_joined_table
         end
+
         scope.where("#{last_joined_table}.#{_ct.parent_column_name}" => parent_id).readonly(false).first
       end
 
diff --git a/lib/closure_tree/hash_tree_support.rb b/lib/closure_tree/hash_tree_support.rb
@@ -5,17 +5,40 @@ module HashTreeSupport
     def default_tree_scope(scope, limit_depth = nil)
       # Deepest generation, within limit, for each descendant
       # NOTE: Postgres requires HAVING clauses to always contains aggregate functions (!!)
-      having_clause = limit_depth ? "HAVING MAX(generations) <= #{limit_depth - 1}" : ''
-      generation_depth = <<-SQL.squish
-          INNER JOIN (
-            SELECT descendant_id, MAX(generations) as depth
-            FROM #{quoted_hierarchy_table_name}
-            GROUP BY descendant_id
-            #{having_clause}
-          ) #{t_alias_keyword} generation_depth
-            ON #{quoted_table_name}.#{model_class.primary_key} = generation_depth.descendant_id
-      SQL
-      scope_with_order(scope.joins(generation_depth), 'generation_depth.depth')
+
+      # Get the hierarchy table for the scope's model class
+      hierarchy_table_arel = if scope.respond_to?(:hierarchy_class)
+                               scope.hierarchy_class.arel_table
+                             elsif scope.klass.respond_to?(:hierarchy_class)
+                               scope.klass.hierarchy_class.arel_table
+                             else
+                               hierarchy_table
+                             end
+
+      model_table_arel = scope.klass.arel_table
+
+      # Build the subquery using Arel
+      subquery = hierarchy_table_arel
+                 .project(
+                   hierarchy_table_arel[:descendant_id],
+                   hierarchy_table_arel[:generations].maximum.as('depth')
+                 )
+                 .group(hierarchy_table_arel[:descendant_id])
+
+      # Add HAVING clause if limit_depth is specified
+      subquery = subquery.having(hierarchy_table_arel[:generations].maximum.lteq(limit_depth - 1)) if limit_depth
+
+      generation_depth_alias = subquery.as('generation_depth')
+
+      # Build the join
+      join_condition = model_table_arel[scope.klass.primary_key].eq(generation_depth_alias[:descendant_id])
+
+      join_source = model_table_arel
+                    .join(generation_depth_alias)
+                    .on(join_condition)
+                    .join_sources
+
+      scope_with_order(scope.joins(join_source), 'generation_depth.depth')
     end
 
     def hash_tree(tree_scope, limit_depth = nil)
diff --git a/lib/closure_tree/hierarchy_maintenance.rb b/lib/closure_tree/hierarchy_maintenance.rb
@@ -90,16 +90,10 @@ def delete_hierarchy_references
         # It shouldn't affect performance of postgresql.
         # See http://dev.mysql.com/doc/refman/5.0/en/subquery-errors.html
         # Also: PostgreSQL doesn't support INNER JOIN on DELETE, so we can't use that.
-        _ct.connection.execute <<-SQL.squish
-          DELETE FROM #{_ct.quoted_hierarchy_table_name}
-          WHERE descendant_id IN (
-            SELECT DISTINCT descendant_id
-            FROM (SELECT descendant_id
-              FROM #{_ct.quoted_hierarchy_table_name}
-              WHERE ancestor_id = #{_ct.quote(id)}
-                 OR descendant_id = #{_ct.quote(id)}
-            ) #{_ct.t_alias_keyword} x )
-        SQL
+
+        hierarchy_table = hierarchy_class.arel_table
+        delete_query = _ct.build_hierarchy_delete_query(hierarchy_table, id)
+        _ct.connection.execute(delete_query.to_sql)
       end
     end
 
diff --git a/lib/closure_tree/numeric_deterministic_ordering.rb b/lib/closure_tree/numeric_deterministic_ordering.rb
diff --git a/lib/closure_tree/support.rb b/lib/closure_tree/support.rb
