Skip to content

Commit d142e3c

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 d142e3c

2 files changed

Lines changed: 37 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: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,4 +153,32 @@ 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+
define_singleton_method(:name) { 'TestParent' } # vcap_relations plugin derives reciprocal from self.name
164+
one_to_many :children, class: child, key: :main_id
165+
end
166+
parent = parent_class.new
167+
parent.values[:id] = 1
168+
sql = capture_sql { parent.children }
169+
expect(sql).to match(/ORDER BY .id./)
170+
end
171+
172+
it 'does not override explicit association order' do
173+
child = child_class
174+
parent_class = Class.new(Sequel::Model(db[:test_default_order_main])) do
175+
define_singleton_method(:name) { 'TestParent' }
176+
one_to_many :children, class: child, key: :main_id, order: Sequel.desc(:id)
177+
end
178+
parent = parent_class.new
179+
parent.values[:id] = 1
180+
sql = capture_sql { parent.children }
181+
expect(sql).to match(/ORDER BY .id. DESC/)
182+
end
183+
end
156184
end

0 commit comments

Comments
 (0)