Skip to content

Translate Arel-level .returning(...) to Oracle's RETURNING ... INTO :bind for direct-Arel callers (Rails #47161) #2796

@yahonda

Description

@yahonda

Background

Rails #47161 (merged 2026-05-14) added a chainable .returning(...) API on Arel::InsertManager / Arel::UpdateManager / Arel::DeleteManager, plus a returning attribute on the corresponding statement nodes (default []). When that attribute is non-empty, Arel::Visitors::ToSql appends a PostgreSQL-style RETURNING <expr-list> to the compiled SQL.

The PR is Arel-only. ActiveRecord itself does not call .returning(...), so AR-driven flows (Model.create, Model#update, etc.) keep producing o.returning == [] and the new visitor branch is inert.

Why this matters for oracle-enhanced

A user who hand-builds Arel and opts in to the new API on Oracle gets PostgreSQL-style SQL that Oracle rejects:

t  = Arel::Table.new(:employees)
im = Arel::InsertManager.new.into(t)
im.insert([[t[:name], "x"]])
im.returning(Arel.star)
ActiveRecord::Base.connection.to_sql(im.ast)
# => INSERT INTO "EMPLOYEES" ("NAME") VALUES ('x') RETURNING *

Oracle needs RETURNING <cols> INTO :bind and won't parse a bare RETURNING *. Today the default Arel::Visitors::ToSql runs because oracle-enhanced's Arel::Visitors::Oracle12 and Arel::Visitors::Oracle do not override visit_Arel_Nodes_InsertStatement / visit_Arel_Nodes_DeleteStatement. OracleCommon#visit_Arel_Nodes_UpdateStatement (lib/arel/visitors/oracle_common.rb:106-113) only strips ORDER BY before calling super, so the new RETURNING emission flows through unchanged.

There's no regression today — AR-driven INSERT continues to go through OracleEnhanced::DatabaseStatements#sql_for_insert (lib/active_record/connection_adapters/oracle_enhanced/database_statements.rb:283-304), which uses Oracle's RETURNING INTO form. This issue is about the direct-Arel surface, not the AR path.

Two options

(a) Translate: override visit_Arel_Nodes_InsertStatement / visit_Arel_Nodes_UpdateStatement / visit_Arel_Nodes_DeleteStatement in OracleCommon to emit Oracle's RETURNING <cols> INTO :bind form when o.returning is non-empty. Arel doesn't model OUT binds, so the adapter still has to allocate and consume them — overlapping work with #2795 (UPDATE returning for virtual columns).

(b) Raise: have those same visitor methods raise NotImplementedError when o.returning is non-empty, citing the canonical Oracle alternative (OracleEnhanced::DatabaseStatements#sql_for_insert and, once #2795 lands, an equivalent UPDATE path). Matches the project's existing opt-in-flag-raise pattern.

(a) is the eventual goal but is significant work and likely waits on #2795. (b) is cheap and saves a user from getting an inscrutable ORA-* parse error.

Open questions to resolve before implementation

  • Is there an actual user hitting this? If not, (b) might be all that's needed for now.
  • Should the DELETE visitor be in scope too, or only INSERT/UPDATE? Oracle supports DELETE ... RETURNING ... INTO :bind natively, so consistency argues for all three.
  • Does composite_primary_key interact with this for multi-PK INSERTs?

References

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions