POC: resolve describe() via DBMS_UTILITY.NAME_RESOLVE#2531
Closed
yahonda wants to merge 1 commit into
Closed
Conversation
Collaborator
Author
|
Mark as "Ready for review" to run all CI. Not intended to merge this pull request itself. |
Contributor
There was a problem hiding this comment.
Pull request overview
This draft POC replaces the adapter’s OracleEnhanced::Connection#describe implementation (previously a multi-dictionary lookup with manual synonym recursion) with a single server-side DBMS_UTILITY.NAME_RESOLVE call, adding the necessary OUT-bind plumbing for both OCI8 (MRI) and JDBC (JRuby) paths, plus a benchmark script and regression coverage.
Changes:
- Rework
describe()to resolve tables/views/synonyms viaDBMS_UTILITY.NAME_RESOLVEinstead of queryingall_tables/all_views/all_synonyms. - Add driver-specific
_resolve_namehelpers for OCI8 and JDBC to support NAME_RESOLVE OUT parameters. - Extend test setup + regression specs (including public synonym coverage) and add a reproducible benchmark script.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| spec/support/create_oracle_enhanced_users.sql | Grants test users permissions needed to create/drop public synonyms. |
| spec/spec_helper.rb | Purges the recyclebin before the suite to reduce cross-run interference. |
| spec/active_record/connection_adapters/oracle_enhanced/connection_spec.rb | Adds a regression spec covering describe() resolution across table/view/private+public synonyms. |
| script/benchmark_describe.rb | Adds a benchmark to compare describe() performance across branches/approaches. |
| lib/active_record/connection_adapters/oracle_enhanced/oci_connection.rb | Adds OCI8 _resolve_name implementation via an anonymous PL/SQL block with OUT binds. |
| lib/active_record/connection_adapters/oracle_enhanced/jdbc_connection.rb | Adds JDBC _resolve_name implementation via CallableStatement OUT parameters. |
| lib/active_record/connection_adapters/oracle_enhanced/connection.rb | Replaces dictionary-query-based describe() with NAME_RESOLVE-based resolution and updated error wrapping. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
5 tasks
yahonda
added a commit
to yahonda/oracle-enhanced
that referenced
this pull request
Apr 15, 2026
Two fixes for issues Copilot flagged on rsim#2531: 1. Over-quoting case-preserving names broke schema resolution. valid_table_name? returns false whenever any dotted part has mixed case, so inputs like "sys.test_Mixed" went through the else branch and were wrapped as "sys"."test_Mixed" — Oracle then searched for a lowercase schema and missed SYS. Normalize each dotted part individually: upcase it if it's a valid unquoted identifier, otherwise wrap in quotes. "sys.test_Mixed" now becomes SYS."test_Mixed". Added regression spec that exercises a mixed-case quoted table qualified with its owner. 2. The generic "rescue => e" re-wrapped ArgumentError (raised for the unsupported @dblink case) as ConnectionException, changing the exception type vs. master. Bundle ArgumentError with the ConnectionException re-raise so it passes through unchanged. While touching that line, also fixed a pre-existing typo in the raise itself: `raise ArgumentError "db link is not supported"` (without comma) actually raises NoMethodError, not ArgumentError. Added a guard spec covering the intended behavior. Full rspec suite: 425 examples, 0 failures, 6 pending on both CRuby 4.0.2 and JRuby 10.0.5.0. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Collaborator
Author
Benchmark after Copilot review fixes (commit 3b12cec)Re-ran Avg ms per
Absolute numbers aren't meaningful on their own (local-only, single machine, one container). The point is that the deltas are symmetrically distributed around zero, i.e., noise, not regression. Per-part quoting runs once per The POC's relative standing vs. master and #2521 from the earlier benchmark in the commit log is unchanged. |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @conn.exec "CREATE TABLE test_describe_all (id NUMBER)" rescue nil | ||
| @conn.exec "CREATE VIEW test_describe_all_v AS SELECT * FROM test_describe_all" rescue nil | ||
| @conn.exec "CREATE SYNONYM test_describe_all_syn FOR test_describe_all" rescue nil | ||
| @conn.exec "CREATE PUBLIC SYNONYM test_describe_all_pub FOR #{@owner}.test_describe_all" rescue nil |
There was a problem hiding this comment.
Using CREATE PUBLIC SYNONYM ... rescue nil can mask the “name already used” error and leave an existing public synonym in place (possibly pointing somewhere else), making the spec flaky and potentially destructive if parallel runs share a DB. Prefer CREATE OR REPLACE PUBLIC SYNONYM (or explicitly drop first) so the test deterministically sets the synonym target.
Suggested change
| @conn.exec "CREATE PUBLIC SYNONYM test_describe_all_pub FOR #{@owner}.test_describe_all" rescue nil | |
| @conn.exec "CREATE OR REPLACE PUBLIC SYNONYM test_describe_all_pub FOR #{@owner}.test_describe_all" |
| RSpec.configure do |config| | ||
| config.before(:suite) do | ||
| ActiveRecord::Base.establish_connection(CONNECTION_PARAMS) | ||
| ActiveRecord::Base.connection.execute("PURGE RECYCLEBIN") |
There was a problem hiding this comment.
PURGE RECYCLEBIN in a global before(:suite) hook can fail in environments where the recycle bin is disabled or the user lacks the needed privilege, which would abort the entire test run. Consider rescuing ActiveRecord::StatementInvalid (as DatabaseTasks#purge already does) or making this conditional so the suite still runs when the purge cannot be performed.
Suggested change
| ActiveRecord::Base.connection.execute("PURGE RECYCLEBIN") | |
| begin | |
| ActiveRecord::Base.connection.execute("PURGE RECYCLEBIN") | |
| rescue ActiveRecord::StatementInvalid | |
| # Ignore environments where recycle bin purge is unsupported or unauthorized. | |
| end |
Comment on lines
+38
to
+41
| parts = name.split(".").map do |p| | ||
| OracleEnhanced::Quoting.valid_table_name?(p) ? p.upcase : %("#{p}") | ||
| end | ||
| else | ||
| raise OracleEnhanced::ConnectionException, %Q{"DESC #{name}" failed; does it exist?} | ||
| _resolve_name(parts.join(".")) |
There was a problem hiding this comment.
When building quoted identifier parts via %("#{p}"), embedded double quotes in p are not escaped. For a valid Oracle identifier containing a quote (represented as "" inside the quoted name), this will generate an invalid name string and make NAME_RESOLVE fail. Consider escaping embedded quotes (and/or reusing the adapter's existing identifier-quoting helper) before wrapping the part in double quotes.
Comment on lines
+56
to
+59
| def safe_exec(conn, sql) | ||
| conn.execute(sql) | ||
| rescue ActiveRecord::StatementInvalid | ||
| # idempotent: ignore "already exists" / "does not exist" |
There was a problem hiding this comment.
safe_exec rescues all ActiveRecord::StatementInvalid, which can silently hide unexpected failures (e.g., missing privileges, invalid SQL) and still run the benchmark with an incomplete fixture set, producing misleading results. Consider only suppressing the specific “already exists” / “does not exist” Oracle errors (or checking object existence first) and re-raising anything else.
Suggested change
| def safe_exec(conn, sql) | |
| conn.execute(sql) | |
| rescue ActiveRecord::StatementInvalid | |
| # idempotent: ignore "already exists" / "does not exist" | |
| IGNORABLE_ORACLE_ERROR_CODES = %w[ | |
| ORA-00942 | |
| ORA-00955 | |
| ORA-01432 | |
| ORA-01434 | |
| ORA-04043 | |
| ].freeze | |
| def ignorable_statement_invalid?(error) | |
| messages = [error.message, error.cause&.message].compact | |
| messages.any? do |message| | |
| IGNORABLE_ORACLE_ERROR_CODES.any? { |code| message.include?(code) } | |
| end | |
| end | |
| def safe_exec(conn, sql) | |
| conn.execute(sql) | |
| rescue ActiveRecord::StatementInvalid => e | |
| # idempotent: ignore only Oracle "already exists" / "does not exist" | |
| raise unless ignorable_statement_invalid?(e) |
6 tasks
Replaces the UNION ALL catalog query inside
SchemaStatements#resolve_data_source_name with a single
DBMS_UTILITY.NAME_RESOLVE PL/SQL call. Private and public synonyms
are chased server-side, so the iterative loop + visited Set used to
guard against circular synonyms is no longer needed -- Oracle
reports a cycle natively as ORA-00980 ("synonym translation is no
longer valid").
DBMS_UTILITY.NAME_RESOLVE is documented for Oracle 8i (released
1999), so version support is not a concern on any supported
release. See the Oracle8i Supplied PL/SQL Packages Reference,
Release 2 (8.1.6), A76936-01:
https://docs.oracle.com/cd/A87860_01/doc/appdev.817/a76936/dbms_uti.htm
Design notes
1. Driver-layer helpers. OCIConnection#name_resolve uses an
anonymous PL/SQL block with named binds. JDBCConnection#name_resolve
uses CallableStatement + registerOutParameter. Both return
[schema, object_name]; the caller is the adapter's
SchemaStatements mix-in, not the raw driver.
2. Instrumentation preserved. NAME_RESOLVE bypasses select_one
entirely, so we wrap the call in an explicit sql.active_record
SCHEMA notification. The regression spec that subscribes to
that event and was introduced alongside the move-describe
refactor stays green.
3. Case handling. NAME_RESOLVE uppercases unquoted input, so
mixed-case identifiers like "test_Mixed" fail with ORA-06564
unless quoted. normalize_name_for_name_resolve upcases each
dotted part when the part is a valid unquoted identifier and
wraps it in double quotes otherwise; "sys.test_Mixed" becomes
SYS."test_Mixed" rather than the all-quoted
"sys"."test_Mixed" that would send Oracle hunting for a
lowercase schema.
4. context=0. NAME_RESOLVE's context argument matters:
context=1 covers PL/SQL objects only and raises ORA-04047 for
tables/views. context=0 covers tables, views, and synonyms.
Trade-off: the select_one-based resolve_data_source_name benefits
from the AR query cache on repeated describes in the same scope;
NAME_RESOLVE always hits the server. The win is round-trip count
(one call instead of UNION ALL + synonym loop), which dominates
on cold-cache / synonym-heavy schemas -- see benchmark results in
PR rsim#2531.
The benchmark script (script/benchmark_describe.rb) is retargeted
to adapter.send(:resolve_data_source_name, name) so all three
implementations (master UNION ALL, PR rsim#2521 all_objects, this POC)
can be measured against a shared 1000-object fixture. A
before(:suite) PURGE RECYCLEBIN hook keeps the suite from
accumulating stale BIN$... dictionary rows across rspec runs.
create_oracle_enhanced_users.sql adds CREATE/DROP PUBLIC SYNONYM
grants required by the benchmark's public-synonym fixtures.
Full rspec suite:
- CRuby 4.0.2: 453 examples, 0 failures, 6 pending
- JRuby 10.0.5.0: 458 examples, 0 failures, 6 pending
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
yahonda
force-pushed
the
poc-dbms-utility-name-resolve
branch
from
3b12cec to
2e9f613
Compare
6 tasks
Collaborator
Author
yahonda
added a commit
to yahonda/oracle-enhanced
that referenced
this pull request
Apr 29, 2026
Replaces the UNION ALL catalog query inside
SchemaStatements#resolve_data_source_name with a single
DBMS_UTILITY.NAME_RESOLVE PL/SQL call. Private and public synonyms
are chased server-side, so the iterative loop + visited Set used to
guard against circular synonyms is no longer needed -- Oracle
reports a cycle natively as ORA-00980 ("synonym translation is no
longer valid").
DBMS_UTILITY.NAME_RESOLVE is documented for Oracle 8i (released
1999), so version support is not a concern on any supported
release. See the Oracle8i Supplied PL/SQL Packages Reference,
Release 2 (8.1.6), A76936-01:
https://docs.oracle.com/cd/A87860_01/doc/appdev.817/a76936/dbms_uti.htm
Design notes
1. Driver-layer helpers. OCIConnection#name_resolve uses an
anonymous PL/SQL block with named binds. JDBCConnection#name_resolve
uses CallableStatement + registerOutParameter. Both return
[schema, object_name]; the caller is the adapter's
SchemaStatements mix-in, not the raw driver.
2. Instrumentation preserved. NAME_RESOLVE bypasses select_one
entirely, so we wrap the call in an explicit sql.active_record
SCHEMA notification. The regression spec that subscribes to
that event and was introduced alongside the move-describe
refactor stays green.
3. Case handling. NAME_RESOLVE uppercases unquoted input, so
mixed-case identifiers like "test_Mixed" fail with ORA-06564
unless quoted. normalize_name_for_name_resolve upcases each
dotted part when the part is a valid unquoted identifier and
wraps it in double quotes otherwise; "sys.test_Mixed" becomes
SYS."test_Mixed" rather than the all-quoted
"sys"."test_Mixed" that would send Oracle hunting for a
lowercase schema.
4. context=0. NAME_RESOLVE's context argument matters:
context=1 covers PL/SQL objects only and raises ORA-04047 for
tables/views. context=0 covers tables, views, and synonyms.
Trade-off: the select_one-based resolve_data_source_name benefits
from the AR query cache on repeated describes in the same scope;
NAME_RESOLVE always hits the server. The win is round-trip count
(one call instead of UNION ALL + synonym loop), which dominates
on cold-cache / synonym-heavy schemas -- see benchmark results in
PR rsim#2531.
The benchmark script (script/benchmark_describe.rb) is retargeted
to adapter.send(:resolve_data_source_name, name) so all three
implementations (master UNION ALL, PR rsim#2521 all_objects, this POC)
can be measured against a shared 1000-object fixture. A
before(:suite) PURGE RECYCLEBIN hook keeps the suite from
accumulating stale BIN$... dictionary rows across rspec runs.
create_oracle_enhanced_users.sql adds CREATE/DROP PUBLIC SYNONYM
grants required by the benchmark's public-synonym fixtures.
Full rspec suite:
- CRuby 4.0.2: 453 examples, 0 failures, 6 pending
- JRuby 10.0.5.0: 458 examples, 0 failures, 6 pending
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
yahonda
added a commit
to yahonda/oracle-enhanced
that referenced
this pull request
Apr 29, 2026
Replaces the UNION ALL catalog query inside
SchemaStatements#resolve_data_source_name with a single
DBMS_UTILITY.NAME_RESOLVE PL/SQL call. Private and public synonyms
are chased server-side, so the iterative loop + visited Set used to
guard against circular synonyms is no longer needed -- Oracle
reports a cycle natively as ORA-00980 ("synonym translation is no
longer valid").
DBMS_UTILITY.NAME_RESOLVE is documented for Oracle 8i (released
1999), so version support is not a concern on any supported
release. See the Oracle8i Supplied PL/SQL Packages Reference,
Release 2 (8.1.6), A76936-01:
https://docs.oracle.com/cd/A87860_01/doc/appdev.817/a76936/dbms_uti.htm
Design notes
1. Driver-layer helpers. OCIConnection#name_resolve uses an
anonymous PL/SQL block with named binds. JDBCConnection#name_resolve
uses CallableStatement + registerOutParameter. Both return
[schema, object_name]; the caller is the adapter's
SchemaStatements mix-in, not the raw driver.
2. Instrumentation preserved. NAME_RESOLVE bypasses select_one
entirely, so we wrap the call in an explicit sql.active_record
SCHEMA notification. The regression spec that subscribes to
that event and was introduced alongside the move-describe
refactor stays green.
3. Case handling. NAME_RESOLVE uppercases unquoted input, so
mixed-case identifiers like "test_Mixed" fail with ORA-06564
unless quoted. normalize_name_for_name_resolve upcases each
dotted part when the part is a valid unquoted identifier and
wraps it in double quotes otherwise; "sys.test_Mixed" becomes
SYS."test_Mixed" rather than the all-quoted
"sys"."test_Mixed" that would send Oracle hunting for a
lowercase schema.
4. context=0. NAME_RESOLVE's context argument matters:
context=1 covers PL/SQL objects only and raises ORA-04047 for
tables/views. context=0 covers tables, views, and synonyms.
Trade-off: the select_one-based resolve_data_source_name benefits
from the AR query cache on repeated describes in the same scope;
NAME_RESOLVE always hits the server. The win is round-trip count
(one call instead of UNION ALL + synonym loop), which dominates
on cold-cache / synonym-heavy schemas -- see benchmark results in
PR rsim#2531.
The benchmark script (script/benchmark_describe.rb) is retargeted
to adapter.send(:resolve_data_source_name, name) so all three
implementations (master UNION ALL, PR rsim#2521 all_objects, this POC)
can be measured against a shared 1000-object fixture. A
before(:suite) PURGE RECYCLEBIN hook keeps the suite from
accumulating stale BIN$... dictionary rows across rspec runs.
create_oracle_enhanced_users.sql adds CREATE/DROP PUBLIC SYNONYM
grants required by the benchmark's public-synonym fixtures.
Full rspec suite:
- CRuby 4.0.2: 453 examples, 0 failures, 6 pending
- JRuby 10.0.5.0: 458 examples, 0 failures, 6 pending
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.
Status: open for comparison alongside #2521 / #2560
Updated 2026-04-21: this POC, #2521, and #2560 all target the describe
path from different angles. Fresh local benchmarks below on both 23c and
11g make the trade space concrete. The 11g synonym case in particular
surprised me -- #2521 alone leaves it at ~166 ms/call on 11g (vs POC's
~0.28 ms), which is the same ALL_SYNONYMS pain #2560 is trying to work
around with the USER_* fallback.
Not a push to merge -- posted for discussion.
Summary
POC for the approach @matthewtusker suggested in
#2521 (comment):
replace the catalog query inside
SchemaStatements#resolve_data_source_name(formerlyOracleEnhanced::Connection#describe) with a singleDBMS_UTILITY.NAME_RESOLVEcall. The package resolves private and publicsynonyms server-side, so no manual synonym recursion is needed.
Opened as draft for discussion alongside #2521 -- not a push to
merge, just runnable code to compare.
Rebased onto current master
The original POC branch targeted
Connection#describe, which has sincebeen removed: #2545 moved describe logic into
SchemaStatements#resolve_data_source_name, routed it throughselect_onefor logging / instrumentation / query-cache integration, andadded a
Set-based iterative synonym loop with cycle detection. Thisbranch now reapplies NAME_RESOLVE as a driver-level helper
(
OCIConnection#name_resolve/JDBCConnection#name_resolve) thatresolve_data_source_namedelegates to, with thesql.active_recordSCHEMA notification wrapped around the call so the instrumentation
contract introduced by #2545 stays intact.
Benchmark: master vs PR #2521 vs this POC
CRuby 4.0.2, 900-object fixture (700 tables + 100 views + 100 private
synonyms). Public synonyms left out because the setup requires grants
I haven't applied on my local box yet. Avg ms per
resolve_data_source_name()call (lower is better). Commits measured:38907d56-select_one+ 4-way UNION ALL acrossall_tables/all_views/all_synonyms.ae3e3d2c- singleall_objectsquery with a secondaryall_synonymslookup when the first hit is a synonym.2e9f613a-DBMS_UTILITY.NAME_RESOLVE(server-side).Oracle 23ai (local gvenzl/oracle-free container)
Oracle 11g XE (local gvenzl/oracle-xe:11 container)
The 11g numbers line up with #2560's measurements (668 s / 1399 calls
= ~478 ms/call for the master UNION ALL path). Two 11g-specific
observations worth flagging:
tables/views, ending up essentially on par with POC), because the
single
all_objectsquery sidesteps the UNION-ALL pain that hurtmaster most.
the secondary
all_synonymslookup re-enters the ALL_* painReplace slow UNION ALL query with optimized all_objects query #2521's main query escaped. This is structurally the same issue
Use USER_* dictionary views to speed up Oracle 11g #2560's USER_SYNONYMS fallback is trying to fix, just from a
different direction. POC bypasses it entirely because NAME_RESOLVE
follows the synonym chain inside the PL/SQL engine and never
queries a dictionary view from Ruby.
On 23c the differences are consistent with the 11g picture but
compressed by ~1000x because 23c's dictionary views are orders of
magnitude cheaper. Absolute numbers are machine-specific; only the
ratios are meaningful.
Oracle version support
DBMS_UTILITY.NAME_RESOLVEis documented for Oracle 8i (1999), so it'savailable on every version this adapter realistically targets in 2026.
See the Oracle8i Supplied PL/SQL Packages Reference, Release 2 (8.1.6).
Implementation notes
Subtleties the spec suite surfaced:
NAME_RESOLVE'scontextparameter matters --context=1is PL/SQLobjects only and raises
ORA-04047for tables/views.context=0covers tables, views and synonyms (verified against Oracle 23ai).
NAME_RESOLVEuppercases unquoted input, so case-preserving (quoted)identifiers like
"test_Mixed_Comments"failed withORA-06564.normalize_name_for_name_resolvewraps each dotted part in doublequotes when the original identifier is not upcase-normalized by
valid_table_name?, sosys.test_MixedbecomesSYS."test_Mixed"rather than
"sys"."test_Mixed".Circular synonyms surface as
ORA-00980("synonym translation is nolonger valid") from NAME_RESOLVE, not the SQL layer's
ORA-01775("looping chain of synonyms"). Our
resolve_data_source_namewrapsthat as an
OracleEnhanced::ConnectionException-- no Ruby-side stackoverflow, clean error.
OUT-bind plumbing lives on both the OCI8 path (anonymous PL/SQL block
with named binds) and the JDBC path (
CallableStatement+registerOutParameter) so it runs on MRI and JRuby alike.Trade-offs worth calling out
resolve_data_source_nameruns itscatalog query through
select_one, which participates in theActiveRecord query cache -- a repeated describe in the same request
can be effectively free. NAME_RESOLVE is a PL/SQL call and always
hits the server. The
sql.active_recordevent is still emitted (viainstrumenter.instrument), so logging / subscribers see the call,but there's no cache layer in front of it.
all_*dictionaries. Introducing aDBMS_UTILITYcall is a stylebreak.
than the SQL in Replace slow UNION ALL query with optimized all_objects query #2521. That's the main cost to weigh against the
~2-3.5x speedup over Replace slow UNION ALL query with optimized all_objects query #2521 on 23c (up to ~600x on 11g synonym paths).
ALL_*→USER_*fallback with three hand-picked swaps and a
same_schema_as_user?helper (explicitly scheduled for removal when 11.2 support drops).
On the describe path, NAME_RESOLVE subsumes Use USER_* dictionary views to speed up Oracle 11g #2560's
resolve_data_source_name UNIONbucket (~86% of Use USER_* dictionary views to speed up Oracle 11g #2560's 11gwall-clock savings) without per-query branching. Use USER_* dictionary views to speed up Oracle 11g #2560's wins on
synonyms/foreign_keys(~50s on the 11g suite) remaincomplementary -- not touched by this POC.
Test plan
bundle exec rspecpasses on CRuby 4.0.2 (453 examples, 0 failures, 6 pending)bundle exec rspecpasses on JRuby 10.0.5.0 (458 examples, 0 failures, 6 pending)resolve_data_source_nameGenerated with Claude Code