Skip to content

Rails 8.1 support#2480

Open
andynu wants to merge 13 commits into
rsim:masterfrom
andynu:rails-8-1-support
Open

Rails 8.1 support#2480
andynu wants to merge 13 commits into
rsim:masterfrom
andynu:rails-8-1-support

Conversation

@andynu
Copy link
Copy Markdown
Collaborator

@andynu andynu commented Oct 23, 2025

This PR brings full Rails 8.1 compatibility to the Oracle Enhanced adapter, building on excellent work by @akostadinov (Aleksandar N. Kostadinov) in PR #2471.

I've added only two additional fixes:

  • CLOB insertion with disabled prepared statements,
  • removal of the obsolete ExplainSubscriber reference that Rails 8.1 refactored away.

Credits

The vast majority of this work was done by Aleksandar N. Kostadinov (@akostadinov) and Daria Mayorova (@mayorova). Their comprehensive compatibility updates fixed nearly all Rails 8.1 issues.

Changes from @akostadinov's PR #2471

Core Rails 8.1 API Compatibility

  • Column constructor signature change (Eliminate queries loading dumped model schema on Postgres rails/rails#54333)
    • Updated OracleEnhanced::Column.new to accept cast_type as second parameter
    • Modified schema_statements.rb and schema_creation.rb to call lookup_cast_type and pass cast types to column
      constructors
    • Updated tests to use new column constructor signature
  • Schema dumping method rename
    • Changed dump_schema_information → dump_schema_versions throughout codebase
    • Updated method calls and comments in schema_statements.rb and test specs
  • Native database types refactoring
    • Moved native_database_types to class method to support Rails 8.1's static type resolution
    • Instance method now delegates to class method
    • Maintains backward compatibility with emulate_booleans_from_strings setting
  • Distinct column API changes (by @mayorova)
    • Updated distinct_relation_for_primary_key to use Rails 7.1+ distinct column API
    • Changed internal method call from previous API to distinct(column_name)

Test Infrastructure & Coverage

  • Rails 7.1 composite primary key tests
    • Added comprehensive test suite for composite primary keys (composite_spec.rb)
    • Tests unique constraints, validations, and composite key behavior
    • 84 lines of new test coverage
  • Test reliability improvements
    • Fixed emulate_booleans test to properly clear type map and re-establish connections
    • Changed from instance variable @conn to leased connections for thread safety
    • Added proper connection cleanup in test teardown
  • Schema enhancement follow-up (Remove `OracleEnhancedAdapter.default_tablespaces[native_database_typ… #1544)
    • Updated create_sequence call to use proper keyword arguments
    • Modernized sequence creation syntax

My Contributions (this PR)

CLOB Insertion Fix

Rails 8.1 ExplainSubscriber Removal

  • Removed obsolete ActiveRecord::ExplainSubscriber reference (commit 85618af)
    • Rails 8.1 removed ExplainSubscriber class
      (rails/rails@f488878)
    • ExplainSubscriber functionality refactored into ExplainRegistry with lazy subscription
    • Rails 8.1 now subscribes only when .explain is first called
    • Removed manual subscription line from spec_helper.rb that was causing NameError

Testing

All tests pass against Rails 8.1 (8-1-stable branch).

@yahonda
Copy link
Copy Markdown
Collaborator

yahonda commented Dec 9, 2025

Would you drop these commits from this pull request? They should be for 8.0, not 8.1.

f34d8d6
0235b57
1153e51
b898c80

@andynu andynu force-pushed the rails-8-1-support branch from e4b0156 to 29a404c Compare December 9, 2025 14:52
@andynu
Copy link
Copy Markdown
Collaborator Author

andynu commented Dec 9, 2025

Certainly. Dropped.

@andynu andynu force-pushed the rails-8-1-support branch 3 times, most recently from e458c74 to e3e858a Compare December 9, 2025 15:29
@andynu
Copy link
Copy Markdown
Collaborator Author

andynu commented Dec 9, 2025

Better. Rather than dropping, I just rebased this on top of master again. Since those commits were from earlier.

@akostadinov
Copy link
Copy Markdown
Contributor

Amazing work @andynu!

I just want to point your attention also to #2483 as a big improvement of performance and memory usage for workloads involving LOBs. Basically I remove this write_lobs stuff altogether because it is completely redundant. This also allows much more efficient LOB reading from the server. IDK if you would want to amend your LOB fixes or maybe it can be merged/discussed further after this PR is merged. But wanted to point out to you. Or if you prefer I can create a pull request to your branch with it. Will be happy to get your feedback in either case.

# After setting large objects to empty, select the OCI8::LOB
# and write back the data.
before_create :record_lobs_for_create
after_create :enhanced_write_lobs
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For create, shouldn't it just write all non-empty and non-null lobs instead of tracking them?

But anyway, I think this LOB callback writing stuff needs to go unless there are non-easily adaptable use-cases for disabling prepared statements as discussed in #2483

P.S. adding this here, doesn't it mean write_lobs was not used for a long time on create already? And that people just use prepared statements and don't need this?

Copy link
Copy Markdown
Collaborator

@yahonda yahonda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ff45875

Could you please move this commit to a separate pull request? This appears to be an issue with the Active Record Oracle enhanced adapter 8.0, which is independent from the 8.1 support work.

Copy link
Copy Markdown
Collaborator

@yahonda yahonda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

7cbb567

Could you move this commit to a separate pull request? Regarding the content itself, I think the troubleshooting note might benefit from more specific guidance—as written, it feels a bit general to be actionable.

Copy link
Copy Markdown
Collaborator

@yahonda yahonda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

e3e858a

Could you move this commit to a separate pull request? It doesn't seem required for Rails 8.1 support.

Copy link
Copy Markdown
Collaborator

@yahonda yahonda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3c15a11

Could you reference the Rails commit or changelog entry for this "activerecord 7.1 distinct column api change"?

@andynu andynu force-pushed the rails-8-1-support branch from 151e629 to 942c50e Compare January 26, 2026 19:27
@andynu
Copy link
Copy Markdown
Collaborator Author

andynu commented Jan 26, 2026

Updated per review feedback. This PR is now stacked on:

  1. Fix CLOB/BLOB insertion when prepared_statements is false #2485 — Fix CLOB/BLOB insertion when prepared_statements: false
  2. Support ROWID-based LOB writes for tables without primary keys #2486 — Support ROWID-based LOB writes for tables without primary keys

I'm actively using this branch in production, and need those fixes, I could conceivably branch again for me needs, and flip the order of these PRs, but my preference would be for a review and decision on those two PRs before merging this one if at all possible. Also both stem from a rails 8.1 change (query_log_tags_enabled) forcing prepared_statements: false; So while the functionality under discussion there could have affected folks prior to rails 8.1, I think people will be more likely to run into it with rails 8.1.

Changes in this update:

@andynu andynu force-pushed the rails-8-1-support branch from 942c50e to fdb161d Compare January 26, 2026 19:36
@yahonda
Copy link
Copy Markdown
Collaborator

yahonda commented Jan 29, 2026

Thank you for your work on this.

My request would be to keep this pull request minimal for Rails 8.1 support and separate those fixes #2485 #2486 into their own pull requests, so that we can backport them to address issues occurring in 8.0 as well. If those fixes are included in this Rails 8.1 pull request, it would be difficult to cherry-pick them back to 8.0.

With that in mind, I'd prefer to first merge the minimal Rails 8.1 support, and then review those two pull requests separately afterward. I hope this approach works for you.

@yahonda
Copy link
Copy Markdown
Collaborator

yahonda commented Jan 29, 2026

In #2489, I temporarily changed the Rails branch in Oracle enhanced adapter's master from main to 8-0-stable. This should allow #2485 and #2486 to be reviewed before #2480.

@andynu
Copy link
Copy Markdown
Collaborator Author

andynu commented Jan 29, 2026 via email

@andynu andynu force-pushed the rails-8-1-support branch 2 times, most recently from bc88c65 to 151e629 Compare February 4, 2026 15:03
andynu and others added 13 commits February 4, 2026 10:05
Add ability to write LOB data to tables that don't have a primary key by:

1. Exposing cursor.rowid method in OCI connection wrapper
2. Capturing ROWID after INSERT in exec_insert (@last_insert_rowid)
3. Using ROWID in write_lobs WHERE clause when no PK is available
4. Supporting composite primary keys (Array) in write_lobs

The ROWID approach works because:
- Ruby-oci8's cursor.rowid returns the ROWID of the last inserted row
- ROWID uniquely identifies any row regardless of table structure
- The after_create callback fires immediately after INSERT on same connection

Also includes ORA-01741 diagnostic logging for empty column detection.
…ents

This spec clearly demonstrates the critical difference between LOB handling
with prepared statements (bind parameters) vs without (raw SQL with empty_clob()).

Key points demonstrated:
- With prepared_statements: true, LOB data flows through type_cast() which
  creates OCI8::CLOB temp LOBs. Data is populated BEFORE INSERT.
- With prepared_statements: false, SQL contains empty_clob() literals.
  The write_lobs callback is REQUIRED to populate LOB data after INSERT.

This test suite will FAIL if the lob.rb callbacks are removed, proving
they are necessary for backwards compatibility with prepared_statements: false.

Related to: rsim#2483
Rails 8.1 removed the ExplainSubscriber class (commit f488878f1bc),
causing test failures with: NameError: uninitialized constant
ActiveRecord::ExplainSubscriber

The ExplainSubscriber functionality was refactored into ExplainRegistry
with lazy subscription - instead of subscribing at initialization,
Rails 8.1 now subscribes only when .explain is first called via
ExplainRegistry.start.

The spec_helper.rb line that manually subscribed to ExplainSubscriber
is no longer needed or valid. Rails 8.1 handles the subscription
automatically and lazily.

Related Rails commit:
- f488878f1bc "Refactor ExplainRegistry to only be subscribed once used"
- Author: Jean Boussier
- Date: Thu Sep 25 10:37:09 2025 +0200
- Link:
rails/rails@f488878
Switch from pre-release alpha constraint to stable release constraint
for ActiveRecord 8.1.0 dependency, following Rails 8.1 stable release.
Document Rails 8.1 support with installation instructions, including
the gem version constraint for activerecord-oracle_enhanced-adapter 8.1.
@andynu andynu force-pushed the rails-8-1-support branch from 151e629 to 81e1a6a Compare February 4, 2026 15:05
@andynu
Copy link
Copy Markdown
Collaborator Author

andynu commented Feb 16, 2026

After discussion with @yahonda on #2486, I've removed e3e858a "ROWID + composite key fix in database_statements.rb + oci_connection.rb" from this PR.

@andynu
Copy link
Copy Markdown
Collaborator Author

andynu commented Mar 26, 2026

@yahonda Is there anything I can help with that might unblock a release?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependencies Pull requests that update a dependency file

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CLOB/BLOB Inserts Fail When prepared_statements: false (which happens when config.active_record.query_log_tags_enabled = true)

4 participants