Skip to content

Bind literals in dictionary queries#2629

Merged
yahonda merged 1 commit into
rsim:masterfrom
yahonda:bind-dictionary-queries-2628
Apr 29, 2026
Merged

Bind literals in dictionary queries#2629
yahonda merged 1 commit into
rsim:masterfrom
yahonda:bind-dictionary-queries-2628

Conversation

@yahonda

@yahonda yahonda commented Apr 28, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Summary

Replaces literal interpolation with bind variables in the four remaining dictionary queries that were still building per-call SQL text in oracle_enhanced/{schema_statements,structure_dump}.rb. With this change the same SQL text is reused across calls, so the server can share a single cursor regardless of which table / object is being inspected.

File Method Bound parameter
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

The remaining all_* / user_* queries in those files (and in oracle_enhanced_adapter.rb) either already use binds or have no per-call literal interpolation (SYS_CONTEXT('userenv', 'current_schema') for owner, fixed secondary = 'N', etc.).

Why

Without binding, each unique value embedded in the WHERE clause produces a fresh shared cursor on the server. The legacy session-level cursor_sharing = force default masked this by getting the server to rewrite literals into system-generated binds (:"SYS_B_0" ...). #2626 removes that default, which surfaces the per-call cursor cost on schemas with many tables/objects (the spec suite alone showed +20-43% on the Run RSpec step).

With these queries bound at the call sites, removing the cursor_sharing default no longer regresses CI runtime.

Execution path: always prepared, regardless of prepared_statements: setting

These queries reach OracleEnhanced::DatabaseStatements#perform_query as a raw SQL string (<<~SQL.squish) plus an explicit binds: array. AR's to_sql_and_binds only substitutes literals for Arel-driven queries (via Arel::Collectors::SubstituteBinds when prepared_statements? is false); for raw SQL with explicit binds, the SQL and binds pass through unchanged. perform_query itself does not consult the connection's prepared_statements flag — it always calls raw_connection.prepare(sql), caches the cursor in @statements[sql], and binds via bind_params.

Net effect: the four queries modified here are executed as prepared statements with bind variables in both prepared_statements: true and prepared_statements: false connections. This is consistent with how the existing bind_string callers in oracle_enhanced_adapter.rb and oracle_enhanced/schema_statements.rb already behave, so the PR doesn't introduce a new exception — it simply enrolls four more dictionary queries in the existing pattern.

If a user genuinely wants per-call parsed cursors for these queries (e.g. to honor the spirit of prepared_statements: false even for SCHEMA queries), that would require splitting perform_query into a "prepare-and-cache" vs "prepare-without-cache" branch, which is out of scope for this PR and tracked as a separate follow-up.

Test plan

Closes #2628.

This was referenced Apr 28, 2026
Closed
Merged
Closed
@yahonda @claude
Bind literals in dictionary queries
5521068 
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>
@yahonda yahonda force-pushed the bind-dictionary-queries-2628 branch from fe9cbbe to 5521068 Compare April 29, 2026 11:54
@yahonda yahonda merged commit 5784326 into rsim:master Apr 29, 2026
12 checks passed
@yahonda yahonda deleted the bind-dictionary-queries-2628 branch April 29, 2026 12:27
@yahonda yahonda mentioned this pull request Apr 29, 2026
Merged
3 tasks
yahonda added a commit to yahonda/oracle-enhanced that referenced this pull request Apr 29, 2026
@yahonda
Honor prepared_statements: false in perform_query and _exec_insert
437b0e3 
Both `OracleEnhanced::DatabaseStatements#perform_query` and
`#_exec_insert` previously took the same branch whenever binds were
present: cache the cursor in `@statements[sql]`, bind values, execute.
That branch ignored the connection's `prepared_statements` flag, so a
connection configured with `prepared_statements: false` (or one running
inside an `unprepared_statement do ... end` block) still had its
bind-ful queries prepared and cached client-side.

This was inconsistent with the user's intent for `prepared_statements:
false` — namely, do not keep prepared cursor handles around — and it
also masked the real cost of bound dictionary queries (rsim#2629) for those
connections, since the server-side shared pool was being primed by the
cached client-side handle just like in the prepared case.

Add a `prepared_statements?` branch on the bind-present path:

  if prepared_statements?
    @Statements[sql] ||= raw_connection.prepare(sql)   # cached, reused
    cursor = @Statements[sql]
    cached = true
  else
    cursor = raw_connection.prepare(sql)               # one-shot
  end
  cursor.bind_params(type_casted_binds)

When `prepared_statements?` is false the cursor is prepared, bound, and
released by `cursor.close unless cached` at the tail of the surrounding
`log` block. The Oracle DB shared pool may still reuse the parsed plan
based on SQL-text matching, but no client-side handle is retained —
analogous to PostgreSQL's `exec_params` path.

Three secondary fixes go with the branch:

- Add a `rescue` inside the `with_retry` block in both methods that
  closes the one-shot cursor when `exec` raises, so a retry that
  re-prepares does not leave the previous handle dangling. The close is
  swallowed (`cursor.close rescue nil`) so it never masks the original
  error. Cached cursors stay in `@statements` and are released by the
  pool's `dealloc` when `@statements.clear` runs.
- Make `with_retry`'s `@statements.clear` conditional on
  `prepared_statements?`. With the new branch the pool is empty in
  unprepared mode, so the clear is wasted work.
- Both ruby-oci8 and the JDBC backend use the same
  `prepare(sql) → bind → exec → close` lifecycle through the shared
  `Cursor` abstraction, so the change applies uniformly to both.

The two existing statement-pool specs in `oracle_enhanced_adapter_spec`
(`should clear older cursors`, `should cache UPDATE statements with
bind variables`) asserted behavior that is now specific to
`prepared_statements: true`; gate them on `@conn.prepared_statements?`.
A new symmetric spec covers the `prepared_statements: false` path:
bind-ful UPDATE no longer grows the pool.
yahonda added a commit to yahonda/oracle-enhanced that referenced this pull request Apr 29, 2026
@yahonda @claude
Respect the database's CURSOR_SHARING setting by default
c766fe2 
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>
yahonda added a commit to yahonda/oracle-enhanced that referenced this pull request Apr 29, 2026
@yahonda @claude
Respect the database's CURSOR_SHARING setting by default
ecc6e81 
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>
@yahonda yahonda mentioned this pull request Apr 29, 2026
Merged
3 tasks
yahonda added a commit to yahonda/oracle-enhanced that referenced this pull request Apr 29, 2026
@yahonda
Unify dictionary queries on plain select_value(s) with bind variables
905d11c 
Extends the bind-variable pattern introduced in rsim#2629 to the remaining
all_* / user_* dictionary queries that were still routing through the
`select_values_forcing_binds` / `select_value_forcing_binds` helpers.
After this commit every dictionary query in `OracleEnhancedAdapter`
calls plain `select_values(sql, name, binds)` /
`select_value(sql, name, binds)` with a `:placeholder` raw SQL and an
explicit `binds` array — one consistent shape across the file.

As a consequence, the two helpers no longer have any callers and are
removed.

Methods touched (`oracle_enhanced_adapter.rb`):

  * identity_primary_key?
  * trigger_backed_primary_key?
  * pk_and_sequence_for (×2 callsites — sequence lookup and PK lookup)
  * primary_keys
  * temporary_table?

The `_forcing_binds` helpers were added in rsim#2125 (2021) to make
dictionary queries use bind variables even inside an
`unprepared_statement do ... end` block, by temporarily removing the
connection from `prepared_statements_disabled_cache` so AR's visitor
wouldn't inline the binds. Two recent changes made that mechanism
unnecessary:

  1. rsim#2629 finished converting the remaining literal-interpolated
     dictionary queries to raw SQL with `:placeholder` markers and
     explicit `binds` arrays. AR's visitor isn't involved on the
     raw-SQL-with-binds path, so there is nothing to inline.
  2. rsim#2651 made `OracleEnhanced::DatabaseStatements#perform_query`
     honor `prepared_statements?` directly. Inside an
     `unprepared_statement` block, dictionary queries now
     prepare-bind-exec-close one-shot instead of caching the cursor
     in `@statements`, but bind values still flow to the server.
     SQL-text matching keeps the shared cursor reusable in the
     database's shared pool — the original server-side cursor-sharing
     goal of rsim#2125 is preserved without the helper.

The helpers were defined under `private` and were not part of the
documented adapter API.
@yahonda yahonda mentioned this pull request Apr 29, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Bind literals in dictionary queries (all_*) instead of relying on cursor_sharing = force

1 participant

@yahonda