Skip to content

Commit b958a27

Browse files
yahondaclaude
andcommitted
Reorder UpdateStatement, tighten comment, and cover empty-orders path
Three follow-ups from the review of #2781: - Reorder visit_Arel_Nodes_UpdateStatement to sit between split_order_string and visit_Arel_Nodes_HomogeneousIn in oracle_common.rb. Groups the ORDER BY handling (order_hacks + split_order_string + UpdateStatement) together and keeps the IN-list cluster (HomogeneousIn + In + NotIn) intact. - Rewrite the comment to make explicit that this override only handles the ORDER BY part. With a LIMIT, the statement is passed through to super so Oracle's own error for the unsupported LIMIT-in-UPDATE surfaces (the visitor does not try to smooth that over). - Add a fifth visitor-level spec example covering the no-orders short-circuit (`return` from the if guard) — previously implicit via sibling tests that did not set orders. Refs #2663 (task B). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 20808f2 commit b958a27

2 files changed

Lines changed: 19 additions & 12 deletions

File tree

lib/arel/visitors/oracle_common.rb

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,18 @@ def split_order_string(string)
9898
array
9999
end
100100

101+
# Oracle does not allow ORDER BY in UPDATE statements. Strip it
102+
# when no LIMIT is present; with a LIMIT, pass through so super
103+
# surfaces Oracle's own error (LIMIT in UPDATE is unsupported too).
104+
def visit_Arel_Nodes_UpdateStatement(o, collector)
105+
if o.orders.any? && o.limit.nil?
106+
o = o.dup
107+
o.orders = []
108+
end
109+
110+
super
111+
end
112+
101113
# To avoid ORA-01795: maximum number of expressions in a list is 1000
102114
# tell ActiveRecord to limit us to 1000 ids at a time
103115
def visit_Arel_Nodes_HomogeneousIn(o, collector)
@@ -145,18 +157,6 @@ def visit_Arel_Nodes_HomogeneousIn(o, collector)
145157
collector
146158
end
147159

148-
def visit_Arel_Nodes_UpdateStatement(o, collector)
149-
# Oracle does not allow ORDER BY/LIMIT in UPDATEs.
150-
if o.orders.any? && o.limit.nil?
151-
# However, there is no harm in silently eating the ORDER BY clause if no LIMIT has been provided,
152-
# otherwise let the user deal with the error
153-
o = o.dup
154-
o.orders = []
155-
end
156-
157-
super
158-
end
159-
160160
def visit_Arel_Nodes_In(o, collector)
161161
attr, values = o.left, o.right
162162
return super unless values.is_a?(Array)

spec/active_record/connection_adapters/oracle_enhanced/arel/update_statement_spec.rb

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,4 +48,11 @@ def build_update(orders: [], limit: nil)
4848
sql = compile(stmt, visitor: oracle)
4949
expect(sql).not_to match(/ORDER BY/i)
5050
end
51+
52+
it "is a no-op when the statement has no orders" do
53+
stmt = build_update
54+
sql = compile(stmt)
55+
expect(sql).not_to match(/ORDER BY/i)
56+
expect(stmt.orders).to eq([])
57+
end
5158
end

0 commit comments

Comments
 (0)