Skip to content

Commit

Permalink
fix: cast to char in child_ancestry_sql / rebuild_counter_cache
Browse files Browse the repository at this point in the history
Before
======

We were casting to a fixed length char field. This can truncate parts of the path

After
=====

Using the conversion built into concat, so no need to CAST.

MySQL works best with CHAR
Postgresql works better with VARCHAR
  • Loading branch information
RongRongTeng authored and kbrock committed Jul 13, 2023
1 parent 49484a3 commit d196dd4
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 5 deletions.
4 changes: 2 additions & 2 deletions lib/ancestry/materialized_path.rb
Original file line number Diff line number Diff line change
Expand Up @@ -94,8 +94,8 @@ def ancestry_root

def child_ancestry_sql
%{
CASE WHEN #{table_name}.#{ancestry_column} IS NULL THEN CAST(#{table_name}.#{primary_key} AS CHAR)
ELSE #{concat("#{table_name}.#{ancestry_column}", "'#{ancestry_delimiter}'", "CAST(#{table_name}.#{primary_key} AS CHAR)")}
CASE WHEN #{table_name}.#{ancestry_column} IS NULL THEN #{concat("#{table_name}.#{primary_key}")}
ELSE #{concat("#{table_name}.#{ancestry_column}", "'#{ancestry_delimiter}'", "#{table_name}.#{primary_key}")}
END
}
end
Expand Down
2 changes: 1 addition & 1 deletion lib/ancestry/materialized_path2.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ def ancestry_root
end

def child_ancestry_sql
concat("#{table_name}.#{ancestry_column}", "CAST(#{table_name}.#{primary_key} AS CHAR)", "'#{ancestry_delimiter}'")
concat("#{table_name}.#{ancestry_column}", "#{table_name}.#{primary_key}", "'#{ancestry_delimiter}'")
end

def ancestry_depth_sql
Expand Down
22 changes: 20 additions & 2 deletions test/concerns/counter_cache_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -88,16 +88,34 @@ def test_counter_cache_when_updating_record
end

def test_setting_counter_cache
AncestryTestDatabase.with_model :depth => 2, :width => 2, :counter_cache => true do |model, roots|
AncestryTestDatabase.with_model :depth => 3, :width => 2, :counter_cache => true do |model, roots|
# ensure they are successfully built
roots.each do |lvl0_node, lvl0_children|
assert_equal 2, lvl0_node.reload.children_count
lvl0_children.each do |lvl1_node, lvl1_children|
assert_equal 2, lvl1_node.reload.children_count
lvl1_children.each do |lvl2_node, _lvl2_children|
assert_equal 0, lvl2_node.reload.children_count
end
end
end

model.update_all(model.counter_cache_column => 0)
# ensure they are successfully broken
roots.each do |lvl0_node, _lvl0_children|
assert_equal 0, lvl0_node.reload.children_count
end
model.rebuild_counter_cache!

# ensure they are successfully built
roots.each do |lvl0_node, _lvl0_children|
roots.each do |lvl0_node, lvl0_children|
assert_equal 2, lvl0_node.reload.children_count
lvl0_children.each do |lvl1_node, lvl1_children|
assert_equal 2, lvl1_node.reload.children_count
lvl1_children.each do |lvl2_node, _lvl2_children|
assert_equal 0, lvl2_node.reload.children_count
end
end
end
end
end
Expand Down

0 comments on commit d196dd4

Please sign in to comment.