Skip to content

Convert SQL queries to Arel where feasible #453

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jul 21, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
83 changes: 83 additions & 0 deletions lib/closure_tree/arel_helpers.rb
Original file line number Diff line number Diff line change
@@ -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
105 changes: 72 additions & 33 deletions lib/closure_tree/finders.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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

Expand Down Expand Up @@ -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'))
Comment on lines +167 to +169
Copy link
Preview

Copilot AI Jul 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] Using raw SQL '1 = 1' for a cross join condition is not idiomatic Arel. Consider using a proper cross join or documenting why this cartesian product is intentional.

Suggested change
join_roots = model_table
.join(roots_subquery)
.on(Arel.sql('1 = 1'))
join_roots = Arel::Nodes::JoinSource.new(
model_table,
roots_subquery
)

Copilot uses AI. Check for mistakes.


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

Expand All @@ -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)
Expand All @@ -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

Expand Down
45 changes: 34 additions & 11 deletions lib/closure_tree/hash_tree_support.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
14 changes: 4 additions & 10 deletions lib/closure_tree/hierarchy_maintenance.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Loading