Skip to content

Commit e6b21da

Browse files
committed
Fix default_order_by_id for association loading
The extension hooks into Dataset#all/each/first to inject ORDER BY id, but Sequel's PlaceholderLiteralizer bypasses these methods entirely. It pre-compiles SQL via Dataset#sql and calls with_sql_all directly, so associations loaded through this optimized path had no deterministic ordering. Override placeholder_literalizer_loader to inject the ordering into the dataset before it gets compiled into a reusable SQL template.
1 parent 340829f commit e6b21da

2 files changed

Lines changed: 35 additions & 1 deletion

File tree

lib/sequel/extensions/default_order_by_id.rb

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,8 @@
22

33
# Sequel extension that adds default ORDER BY id to model queries.
44
#
5-
# Hooks into fetch methods (all, each, first) to add ORDER BY id just before
5+
# Hooks into fetch methods (all, each, first) and placeholder_literalizer_loader
6+
# (used for optimized association loading) to add ORDER BY id just before
67
# execution. This ensures ordering is only added to the final query, not to
78
# subqueries or compound query parts.
89
#
@@ -47,6 +48,13 @@ def first(*, &)
4748
ds.first(*, &)
4849
end
4950

51+
def placeholder_literalizer_loader(&block)
52+
super do |pl, ds|
53+
result_ds = block.call(pl, ds)
54+
result_ds.send(:default_order_by_id) || result_ds
55+
end
56+
end
57+
5058
private
5159

5260
def default_order_by_id

spec/unit/lib/sequel/extensions/default_order_by_id_spec.rb

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,4 +153,30 @@ def capture_sql(&)
153153
end
154154
end
155155
end
156+
157+
describe 'association loading (placeholder_literalizer_loader)' do
158+
let(:child_class) { Class.new(Sequel::Model(db[:test_default_order_join])) }
159+
160+
it 'adds ORDER BY id to association queries' do
161+
child = child_class
162+
parent_class = Class.new(Sequel::Model(db[:test_default_order_main])) do
163+
one_to_many :children, class: child, key: :main_id
164+
end
165+
parent = parent_class.new
166+
parent.values[:id] = 1
167+
sql = capture_sql { parent.children }
168+
expect(sql).to match(/ORDER BY .id./)
169+
end
170+
171+
it 'does not override explicit association order' do
172+
child = child_class
173+
parent_class = Class.new(Sequel::Model(db[:test_default_order_main])) do
174+
one_to_many :children, class: child, key: :main_id, order: Sequel.desc(:id)
175+
end
176+
parent = parent_class.new
177+
parent.values[:id] = 1
178+
sql = capture_sql { parent.children }
179+
expect(sql).to match(/ORDER BY .id. DESC/)
180+
end
181+
end
156182
end

0 commit comments

Comments
 (0)