Skip to content

Reconsider unconditional cursor_sharing = force default; missing test coverage of prepared_statements × cursor_sharing matrix #2622

@yahonda

Description

@yahonda

Background

ActiveRecord::ConnectionAdapters::OracleEnhancedAdapter issues
ALTER SESSION SET cursor_sharing = force on every connection unless the user
explicitly overrides :cursor_sharing in the connection config:

https://github.com/rsim/oracle-enhanced/blob/HEAD/lib/active_record/connection_adapters/oracle_enhanced_adapter.rb#L856-L863

private def configure_connection
  super
  cursor_sharing = (@config[:cursor_sharing] || "force").to_s.upcase
  ...
  execute("alter session set cursor_sharing = #{cursor_sharing}", "SCHEMA")

The ALTER runs regardless of the connection's prepared_statements setting.
The doc comment for the same option (L102) says "no default value", which no
longer matches the code.

Why the default exists

cursor_sharing = force was introduced as the adapter default in
ebb60f3 (2009).
At that time Rails 2.x had no prepared-statement support, so any literal
embedded in AR-generated SQL would create a fresh shared-pool cursor on the
server. cursor_sharing = force was a sensible blanket workaround: the
server rewrote literals to system-generated bind variables (:"SYS_B_0" ...)
and the cursor cache stayed bounded.

Rails 3.1+ added prepared statements; with prepared_statements: true
(today's default for Rails) AR already parameterises its own literals, so
cursor_sharing = force does not contribute additional cursor reuse on that
path. The original justification for forcing force no longer applies to the
common Rails configuration.

Why this matters now

Issue #2619 documents a server-side condition where the combination of
cursor_sharing = force + a server-rewritten literal + RETURNING ... INTO :bind

  • a re-executed (cached) cursor causes the client execute() call to never
    return on amd64 Oracle Database (verified across 11.2, 19c, 21c, 23ai). PR Honor intent.prepare in _exec_insert to avoid OCI8 half-duplex deadlock #2620
    addresses one specific raw-SQL path that triggered it in the test suite, but
    the trigger is created in the first place by setting cursor_sharing = force
    on a connection that is otherwise binding everything correctly.

If the adapter did not apply cursor_sharing = force for connections with
prepared_statements: true, condition (2) of #2619 would not be met and
that whole class of behavior would not surface for typical Rails callers
without any per-call workaround.

Test coverage gap

The reason this interaction was not caught earlier in CI is that the spec
suite does not exercise the cursor_sharing × prepared_statements matrix:

  • CONNECTION_PARAMS in spec/spec_helper.rb:144 does not set
    prepared_statements, so it inherits the Rails default true. There is
    no spec that establishes a connection with prepared_statements: false
    and runs the full battery against it.
  • The cursor_sharing tests
    (spec/active_record/connection_adapters/oracle_enhanced/connection_spec.rb:39-52, 281-285)
    verify only that the alter session ran and the value persists across
    reconnects. They do not exercise repeated execute on a cached cursor under
    cursor_sharing = force, which is exactly the path OCI8 + cursor_sharing=force x INSERT ... RETURNING INTO :returning_id hangs in SQL*Net half-duplex deadlock #2619 hangs on.
  • prepared_statements: false is only exercised in
    spec/active_record/oracle_enhanced/type/text_spec.rb:245-289 (CLOB
    handling), and it is set by mutating @prepared_statements on an
    already-established connection rather than via establish_connection.

Proposal

A. Make the force default conditional on prepared_statements.

Apply cursor_sharing = force only when @config[:cursor_sharing] is set
explicitly, or when @config[:prepared_statements] is false. For
prepared_statements: true (the Rails default), leave the server's
cursor_sharing at its instance-level value. This removes the trigger for
#2619 from the most common configuration without changing behavior for users
who deliberately opt into force or who have prepared statements off.

C. Add a cursor_sharing × prepared_statements CI matrix.

Run the spec suite under both prepared_statements: true and
prepared_statements: false, and add a regression spec that re-executes a
cached cursor with a literal + RETURNING ... INTO :bind (the #2619 shape)
to ensure future changes do not silently re-introduce the trigger.

Sub-option: accept :default as an explicit "leave server alone" value.

Independently of A, allow :cursor_sharing => :default (or nil) to mean
"do not run ALTER SESSION SET cursor_sharing at all; keep the server's
instance-level setting." Useful for callers who want to defer to the
server's configuration regardless of prepared_statements. With A in place
this is the new effective behavior for prepared_statements: true; the
explicit value gives a clean way to express the same intent at the call
site. Either :default or nil could also become the new bare default if
the adapter eventually wants to drop the implicit "force" entirely.

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