Respect the database's CURSOR_SHARING setting by default#2626
Merged
Conversation
3b94a38 to
d8b0f06
Compare
1c0de00 to
40da4f2
Compare
Closed
3 tasks
Collaborator
Author
On hold pending #2628Comparing the
(5 master runs vs 2 PR runs; effect is well above the per-run variance.) This is exactly the dictionary-query trade-off the PR body flagged: with Holding this PR until #2628 (binding the literals in those dictionary queries at the call sites) lands. Once the dictionary queries are properly bound, removing the |
This was referenced Apr 28, 2026
yahonda
added a commit
that referenced
this pull request
Apr 28, 2026
Reverts the trio of changes that introduced and scheduled the test_prepared_statements workflow: - #2623 (Add CI workflow forcing prepared_statements: true) - #2624 (Honor ORACLE_ENHANCED_PREPARED_STATEMENTS in bug report templates) - #2625 (Run test_prepared_statements daily) The motivation for that workflow was to cover the prepared_statements arm of the cursor_sharing × prepared_statements matrix discussed in #2622. Once #2626 lands the cursor_sharing default no longer depends on prepared_statements at all, so that matrix dimension collapses and the dedicated daily run is no longer earning its keep relative to the overall workflow count. The hook can be reintroduced if a future prepared_statements-dependent code path makes it valuable again. Refs #2622. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
3 tasks
yahonda
added a commit
that referenced
this pull request
Apr 28, 2026
Adds an env-driven hook in spec/spec_helper.rb and a workflow_dispatch-triggered job (CRuby 4.0 + JRuby 10.1.0.0) that runs the existing spec suite under ORACLE_ENHANCED_PREPARED_STATEMENTS_FALSE=1, mirroring rails/rails' MYSQL_PREPARED_STATEMENTS env-var convention but inverted (force false instead of force true) since the AbstractAdapter default applicable to this adapter is already true. The matching ORACLE_ENHANCED_PREPARED_STATEMENTS=1 (force true) hook that #2623 added and #2627 reverted is intentionally not re-introduced here: that arm collapsed once the cursor_sharing default stopped depending on prepared_statements (#2626), and a CI grid duplicating the AbstractAdapter default no longer earns its keep. The false arm, on the other hand, is a distinct code path that has accumulated bugs (#272, #2477/#2485 for CLOB/BLOB, and #2634 for NCLOB) precisely because nothing in CI was running it. Once green this should be promoted to schedule: cron for daily runs (separate follow-up). Refs #2622, #2634. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
yahonda
added a commit
to yahonda/oracle-enhanced
that referenced
this pull request
Apr 29, 2026
Replaces literal interpolation in the remaining
oracle_enhanced/{schema_statements,structure_dump}.rb dictionary queries
with bind variables, so the same SQL text is reused across calls
regardless of which table/object name is being looked up. Concretely:
- schema_statements.rb#tablespace — :table_name
- structure_dump.rb#structure_dump_primary_key — :table_name
- structure_dump.rb#structure_dump_unique_keys — :table_name
- structure_dump.rb#drop_sql_for_object — :object_type
Without this, each unique value produces a fresh shared cursor on the
server, which is what the legacy session-level cursor_sharing = force
default was masking. With these queries bound, removing that default
(rsim#2626) no longer regresses CI runtime on schemas with many objects.
Closes rsim#2628.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
40da4f2 to
c766fe2
Compare
Stops the adapter from issuing `ALTER SESSION SET cursor_sharing = force`
at session login. Introduces :default as the new effective default for
the :cursor_sharing connection option; with :default (or unset) the
adapter does not run any ALTER SESSION and the database's instance-level
setting is what the session sees. Explicit values ('exact', 'force')
continue to issue the corresponding ALTER SESSION.
Oracle's documented software default for CURSOR_SHARING is EXACT
(https://docs.oracle.com/en/database/oracle/oracle-database/26/refrn/CURSOR_SHARING.html).
Most installations have not changed it, and many users have not been
aware that oracle_enhanced has been silently overriding it to FORCE at
the session level since 2009 (commit ebb60f3).
The implicit `force` made sense at the time it was added: ActiveRecord
did not yet bind literals, so the adapter sent SQL with literals inlined
and `force` was the only way the server could collapse that into a
single shared cursor. That premise no longer holds:
* The AbstractAdapter default for prepared_statements is true since
Rails 7.1 (rails/rails#45945, commit a0fd15ee7e), inherited by
oracle_enhanced. Application SQL is bound at the AR layer, so 'force'
has nothing left to rewrite for that traffic.
* All all_* / user_* dictionary queries inside oracle_enhanced are
now bind-driven (rsim#2629 finished the last four), so dictionary
access doesn't depend on 'force' for shared-cursor reuse either.
* Keeping 'force' on by default also keeps connections exposed to the
rsim#2619 hang combination on amd64 Oracle Database (cached cursor +
a literal the server rewrites + RETURNING ... INTO). rsim#2620 already
removed the exposure for the typical Rails ORM path; dropping the
implicit force default removes it for raw-SQL callers as well.
After this PR, sessions run with whatever CURSOR_SHARING the database
is configured with — for almost all installations that means Oracle's
documented default EXACT, which is what users were already (unknowingly)
running on the server side. The adapter simply stops imposing its own
value on top.
Anyone who relied on FORCE — typically connections still configured
with prepared_statements: false, where AR interpolates application-SQL
literals at the visitor level — can opt back in with
cursor_sharing: 'force' in database.yml.
Tests use a separate SYSTEM AR connection (SystemObserver) only to read
v$parameter / v$ses_optimizer_env (normalized to upper-case to match
across the two views); the test sessions themselves use the regular
CONNECTION_PARAMS user.
Refs rsim#2622.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
c766fe2 to
ecc6e81
Compare
This was referenced Apr 29, 2026
yahonda
added a commit
that referenced
this pull request
Apr 29, 2026
Removes the `skip` added in 45f8b2b. The spec (`primary_key_trigger_spec.rb` — "does not raise NoMethodError for :returning_id Symbol when logging") was disabled because it triggered the SQL*Net half-duplex deadlock cataloged in #2619: ruby-oci8 + `cursor_sharing = force` + `RETURNING ... INTO :returning_id`. Adapter default `cursor_sharing` is no longer `force` after #2626; it is `:default` (the database's instance-level setting, which is `EXACT` on all but explicitly-tuned installs). Without `force` the server does not rewrite the surrounding literal, so the deadlock combination is no longer reachable from the default test configuration, and the spec runs cleanly again. Verified locally on a `cursor_sharing = exact` instance (the documented Oracle default): * `prepared_statements: true` -> 1 example, 0 failures * `prepared_statements: false` -> 1 example, 0 failures
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Stops applying
cursor_sharing = forcefrom the adapter at session login. Introduces:defaultas the new effective default for the:cursor_sharingconnection option; with it (or unset) the adapter does not run anyALTER SESSION SET cursor_sharingand the database's instance-level setting is what the session sees. Explicit values ('exact','force') continue to issue the correspondingALTER SESSION.:cursor_sharingin config:defaultALTER SESSION(database's instance-level setting kept)'force','exact')ALTER SESSION SET cursor_sharing = valueArgumentErrorWhy
Oracle's documented software default for
CURSOR_SHARINGisEXACT. Most users running an Oracle Database have not changed it fromEXACT, and many are not even aware thatoracle_enhancedhas been silently overriding it toFORCEat the session level since 2009 (ebb60f3).The implicit
forcedefault made sense at the time it was added: Active Record didn't bind literals yet, so the adapter sent SQL with literals inlined, andforcewas the only way the server could collapse that into a single shared cursor.That premise no longer holds:
prepared_statementsbecametruein Rails 7.1 (rails/rails#45945, commita0fd15ee7e) —oracle_enhancedinherits this default. Withprepared_statements: true, Active Record binds application SQL literals before sending, so the server sees identical SQL text regardless of bind values, andforce's rewrite has nothing left to do for that traffic.all_*/user_*dictionary queries insideoracle_enhanceditself are now bind-driven (Bind literals in dictionary queries #2629 finished the last four), so dictionary access doesn't depend onforcefor shared-cursor reuse either.forceon by default also keeps connections exposed to the OCI8 + cursor_sharing=force x INSERT ... RETURNING INTO :returning_id hangs in SQL*Net half-duplex deadlock #2619 hang combination on amd64 Oracle Database (cached cursor + a literal the server rewrites +RETURNING ... INTO). Honor intent.prepare in _exec_insert to avoid OCI8 half-duplex deadlock #2620 removed the exposure for the typical Rails ORM path; dropping the implicitforcedefault removes it for raw-SQL callers too.After this PR, sessions run with whatever
CURSOR_SHARINGthe database is configured with — for almost all installations that means the documented Oracle defaultEXACT, which is what users were already (unknowingly) running on the server level. The adapter simply stops imposing its own value on top.Opting in to the legacy behavior
Anyone who actually relied on
force— typically connections still configured withprepared_statements: false, where Active Record interpolates application-SQL literals at the visitor level — can opt back in with one line indatabase.yml:(Setting
prepared_statements: falseis independent and orthogonal — having both, just one, or neither is a deliberate choice.)CI matrix — Run RSpec duration (seconds)
Run RSpecstep only (Oracle Client install / container start excluded; identical across rows). Lower is better; numbers within ±10s on this CI are noise.prepared_statements: true(adapter default)prepared_statements: false(explicit)cursor_sharing: 'force'(today's adapter-imposed default):default(proposed default, after #2629 merged)Sources of each cell:
(true, force)— master baseline before this PR, avg of 5 recent runs(true, :default)— measurement run #25042855383 (combined branch of Respect the database's CURSOR_SHARING setting by default #2626 and Bind literals in dictionary queries #2629; Bind literals in dictionary queries #2629 is now in master)(false, force)— measurement run #25043429031 (master withprepared_statements: falsehard-coded intoCONNECTION_PARAMSfor measurement)(false, :default)— measurement run #25043431071 (same hard-coded variant on the combined branch)Both rows are within noise. The earlier observation that
:defaultalone (without #2629) regressed by +20–43% is preserved in this comment for the record; #2629 already cancelled that out.Tests
describe \"cursor_sharing\"block inconnection_spec.rbcovers: unset (default),:default, explicit:force, explicit:exact, and the existingArgumentErrorfor unsupported values.CONNECTION_PARAMSuser. Observation goes through a separateSystemObserverAR connection (SYSTEM) readingv$parameterandv$ses_optimizer_env(both normalized to upper-case so the comparison is stable across views), since the regular user has nov$access by default.Doc update
The
:cursor_sharingdoc comment inoracle_enhanced_adapter.rbis updated to describe the new default, the accepted values including:default, and the explicitcursor_sharing: 'force'opt-in for the legacy behavior.References
EXACT;FORCEandEXACTare the only valid values;SIMILARwas removed)prepared_statements: truethe AbstractAdapter default starting with Rails 7.1oracle_enhanced— original 2009 commit introducing theforcedefault:ebb60f3Refs #2622, #2628.