Test: #2531 after #2535#2538
Closed
yahonda wants to merge 7 commits into
Closed
Conversation
Per the suggestion in rsim#2521 (comment), replace the UNION ALL query against all_tables / all_views / all_synonyms with a single DBMS_UTILITY.NAME_RESOLVE call. The package resolves private and public synonyms internally, so the manual synonym recursion is removed. DBMS_UTILITY.NAME_RESOLVE is documented for Oracle 8i (released 1999), so version support is not a concern for any software being written in 2026. 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 OUT-bind plumbing is added on both the OCI8 path (anonymous PL/SQL block with named binds) and the JDBC path (CallableStatement + registerOutParameter) so the POC can be exercised on both MRI and JRuby. Two subtleties the spec suite surfaced: 1. NAME_RESOLVE's context parameter matters. context=1 is PL/SQL objects only and raises ORA-04047 for tables / views; context=0 covers tables, views, and synonyms (verified empirically against Oracle 23ai). 2. NAME_RESOLVE uppercases unquoted input, so case-preserving (quoted) identifiers like "test_Mixed_Comments" failed with ORA-06564. Each dotted part of the name is wrapped in double quotes when the original identifier was not upcase-normalized by valid_table_name?. With those in place the full rspec suite is green on CRuby 4.0.2 and JRuby 10.0.5.0 (423 examples, 0 failures, 6 pending on both). The regression test and PUBLIC SYNONYM grants from PR rsim#2521 are included as the acceptance bar. Open questions this POC exists to answer: - Readability: anonymous PL/SQL block vs. SQL against all_* dictionaries. - Consistency: the rest of the adapter reads from all_* dictionaries; a PL/SQL call is a style break. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Adds script/benchmark_describe.rb comparing OracleEnhanced::Connection#describe across three implementations on a 1000-object schema (700 tables + 100 views + 100 private synonyms + 100 public synonyms): - master: UNION ALL over all_tables / all_views / all_synonyms - PR rsim#2521: single all_objects query - this POC: DBMS_UTILITY.NAME_RESOLVE Benchmark environment: - CPU: AMD Ryzen 9 7940HS (16 threads) - RAM: 60 GiB - OS: Ubuntu (kernel 7.0, x86_64) - Database: Oracle Database 23.26.1 Free, docker image oracle/database:23.26.1-free on localhost (no network hop) - Ruby: CRuby 4.0.2 (ruby-oci8 HEAD) / JRuby 10.0.5.0 (ojdbc17.jar) The absolute numbers below are not meaningful on their own — they reflect this one machine, one container, and one Oracle release. Only the relative differences between the three implementations matter. Shared fixtures across all six runs so every implementation hits the same shared-pool / dictionary cache state. Avg ms per describe() call (lower is better): case master PR rsim#2521 POC master PR rsim#2521 POC CRuby CRuby CRuby JRuby JRuby JRuby tables 0.744 0.311 0.104 1.501 0.723 0.308 views 0.647 0.251 0.107 0.957 0.507 0.254 private synonyms 1.440 1.157 0.234 2.184 1.908 0.235 public synonyms 1.275 0.990 0.258 1.862 1.585 0.230 all mixed 0.842 0.432 0.111 1.054 0.633 0.186 The "all mixed" row is the most representative of real Rails workloads: the POC is ~7.6x faster than master and ~3.9x faster than PR rsim#2521 on CRuby, and ~5.7x / ~3.4x on JRuby. The synonym win is the largest — NAME_RESOLVE follows the synonym server-side in one round trip, whereas PR rsim#2521 still issues a second all_synonyms query. Also adds a before(:suite) hook that runs PURGE RECYCLEBIN. Dropped tables from earlier spec runs accumulate in USER_RECYCLEBIN and can mask issues (ORA-00955 on re-create, stale BIN\$... entries in all_objects). The hook makes every rspec invocation start clean. Fixture counts stay tunable via TABLE_COUNT / VIEW_COUNT / SYNONYM_COUNT. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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>
Fix the adapter to support self.table_name = "table@db_link" end-to-end: - describe (connection.rb): query all_tables@link / all_views@link - column_definitions: append @link to all_tab_cols, all_col_comments - pk_and_sequence_for: append @link to all_sequences, all_constraints; add .to_s for Symbol - table_exists?: add return false - visit_Arel_Table (oracle_common.rb): @link in FROM only - quote_table_name: keep existing strip behavior Add database link spec (oracle_enhanced_remote_link) to test remote table access via RemoteEmployee model. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…resolve conflict)
rsim#2535 changed describe to actually query the remote data dictionary via all_tables@link instead of raising ArgumentError. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Collaborator
Author
|
Closing the test has been done. |
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
1 participant
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.
Verify PR #2531 (DBMS_UTILITY.NAME_RESOLVE) applies cleanly after PR #2535 (db link support) merges. Conflicts resolved: connection.rb db-link branch preserved; create_oracle_enhanced_users.sql combined both grant sets.