Skip to content

Commit 763fedb

Browse files
committed
Address Copilot code review comments
- Use Arel.star.count instead of counting specific column for SQL COUNT(*) - Replace raw SQL 'DISTINCT' with Arel's .distinct method - Document intentional use of cartesian product join in find_all_by_generation These changes improve code consistency while maintaining the same functionality. All tests pass with these modifications.
1 parent aecef8d commit 763fedb

3 files changed

Lines changed: 5 additions & 2 deletions

File tree

lib/closure_tree/arel_helpers.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ def build_hierarchy_delete_query(hierarchy_table, id)
7070
# Build the middle subquery with DISTINCT
7171
middle_subquery = Arel::SelectManager.new
7272
middle_subquery.from(inner_subquery)
73-
middle_subquery.project(Arel.sql('DISTINCT descendant_id'))
73+
middle_subquery.project(inner_subquery[:descendant_id]).distinct
7474

7575
# Build the DELETE statement
7676
delete_manager = Arel::DeleteManager.new

lib/closure_tree/finders.rb

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,9 @@ def find_all_by_generation(generation_level)
161161
.as('descendants')
162162

163163
# Build the joins
164+
# Note: We intentionally use a cartesian product join (CROSS JOIN) here.
165+
# This allows us to find all nodes at a specific generation level across all root nodes.
166+
# The 1=1 condition creates this cartesian product in a database-agnostic way.
164167
join_roots = model_table
165168
.join(roots_subquery)
166169
.on(Arel.sql('1 = 1'))

lib/closure_tree/numeric_deterministic_ordering.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ def _ct_sum_order_by(node = nil)
6767

6868
query = hierarchy_table
6969
.project(
70-
hierarchy_table[:ancestor_id].count.as('total_descendants'),
70+
Arel.star.count.as('total_descendants'),
7171
hierarchy_table[:generations].maximum.as('max_depth')
7272
)
7373

0 commit comments

Comments
 (0)