Arel::Nodes::NamedFunction is unconditionally non-retryable, blocking AR retry for several oracle-enhanced predicates

[yahonda]

Background

AR core's with_raw_connection(allow_retry: true) reconnect-and-retry mechanism was introduced in rails/rails commit eabcff22a86213521eecafbf2dbf3ad82714f941 ("Retry known idempotent SELECT queries on connection-related exceptions", PR #51336). The retryability of a SELECT is computed during Arel compilation:

• The collector starts with retryable = true (set by AR's to_sql_and_binds).
• Various visitor methods can flip it to false: visit_Arel_Nodes_DeleteStatement / UpdateStatement / InsertStatement / BoundSqlLiteral (always) and visit_Arel_Nodes_SqlLiteral (via collector.retryable &&= o.retryable).
• The final collector.retryable is returned to to_sql_and_binds and passed as allow_retry: into with_raw_connection.

Arel::Nodes::SqlLiteral got a retryable: kwarg as part of the same commit, so library-internal literals can opt in. Recent PRs in this repo wired that through for two visitor-injected SqlLiterals on Oracle:

• Mark the injected ROWNUM SqlLiteral as retryable in the pre-12c Arel visitor #2766 — pre-12c ROWNUM injection
• Mark the order_hacks-injected ORDER BY SqlLiteral as retryable #2770 — order_hacks alias_N__ ORDER-BY rewrite

But Arel::Nodes::NamedFunction got no such kwarg. Its visitor is hard-coded to flip retryability off:

# activerecord/lib/arel/visitors/to_sql.rb
def visit_Arel_Nodes_NamedFunction(o, collector)
  collector.retryable = false
  collector << o.name
  collector << "("
  collector << "DISTINCT " if o.distinct
  inject_join(o.expressions, collector, ", ") << ")"
end

The commit message frames this as conservative: "non-idempotent node types (functions, SQL literals, etc)". A function call could mutate state, be time-dependent, or call out to a stored procedure, so Arel cannot prove idempotence and defaults to non-retryable.

Impact on this adapter

oracle-enhanced uses NamedFunction in three places that build read-only Oracle SQL but inadvertently force the surrounding SELECT to non-retryable:

Location	Function	Triggered by
lib/arel/visitors/oracle_common.rb visit_Arel_Nodes_Equality	DBMS_LOB.COMPARE(...)	Model.where(clob_col: value) and any = comparison on a text / binary column (Oracle cannot use plain = against CLOB / BLOB)
lib/arel/visitors/oracle_common.rb visit_Arel_Nodes_Matches	UPPER(...) (both sides)	Case-insensitive LIKE on Oracle
lib/active_record/connection_adapters/oracle_enhanced/context_index.rb ContextIndexClassMethods#contains	CONTAINS(col, ?, label) and SCORE(label)	Model.contains(:col, "query") (Oracle Text full-text search)

In every case, AR-level retry never engages for the affected queries even though the underlying SQL is a plain idempotent SELECT. PR #2771 attempted to address the contains case by marking the child SqlLiterals retryable: true, but CI surfaced the real cause — the surrounding NamedFunction already flipped the collector before the SqlLiteral visit ran. That PR is closed.

Generic AR usage hits the same wall (aggregate functions: Model.count, Model.sum, etc. all go through NamedFunction), but those are a Rails-wide concern rather than oracle-enhanced specific.

Possible paths forward

Upstream (preferred long-term)

File a Rails issue / PR adding a retryable: kwarg to Arel::Nodes::NamedFunction, mirroring the SqlLiteral API: default false, opt-in true for callers that know the function is pure. The visitor becomes:

def visit_Arel_Nodes_NamedFunction(o, collector)
  collector.retryable &&= o.retryable
  collector << o.name
  ...
end

Once that lands, this adapter can mark DBMS_LOB.COMPARE, UPPER, LOWER, CONTAINS, SCORE as retryable. Aggregate functions become a candidate too, which would benefit every AR adapter.

Local workaround (short-term, optional)

Override visit_Arel_Nodes_NamedFunction in OracleCommon with an allowlist of known-safe function names (DBMS_LOB.COMPARE, UPPER, LOWER, CONTAINS, SCORE, possibly NVL / COALESCE / DECODE): keep collector.retryable untouched for those, set it to false otherwise.

The override is plausible but diverges from Rails behaviour for non-Oracle visitors that use the same function names. Worth doing only if the upstream change is going to take a long time.

Related work

• Mark the injected ROWNUM SqlLiteral as retryable in the pre-12c Arel visitor #2766 — pre-12c ROWNUM SqlLiteral retryable (merged)
• Mark the order_hacks-injected ORDER BY SqlLiteral as retryable #2770 — order_hacks ORDER-BY SqlLiteral retryable (merged)
• Mark Model.contains scope's SqlLiterals as retryable #2771 — Model.contains scope SqlLiterals retryable (closed; superseded by this issue)
• Remove driver-level retry and the auto_retry API (#2760 Steps 4-6) #2768 — driver-level retry removal (Migrate from auto_retry to Rails' connection_retries retry mechanism #2760 Steps 4-6); makes AR retry the only mechanism, so the NamedFunction blocker is now the dominant non-retryable surface