oracle-enhanced icon indicating copy to clipboard operation
oracle-enhanced copied to clipboard

Rails 7.2 support!

Open andynu opened this issue 10 months ago • 7 comments

This makes significant progress towards supporting Rails 7.2

andynu avatar Jan 31 '25 19:01 andynu

  • Emulate OracleAdapter isn't working because the loading process changed significantly. oracle_enhanced_connection is never called.
  • The fix here for Model.contains context index searching is a work around for that specific case, but I'm worried that there is a more general conversion from activerecord bind variables '?' to compatible oracle bind variables :N that is not well handled.
  • The timezone tests do not reliably run for me, perhaps oracle client version related.

andynu avatar Jan 31 '25 20:01 andynu

I've been looking at contains errors for a fork we have at work. The ? binds issue is bigger than contains, it affects anytime a user tries to write a query with a where with a ?, like Book.where("title = ?", params[:title]). The cause seems to be that the oracle visitor does not define a bind block, so it falls back to the arel use of ?.

Adding

      BIND_BLOCK = proc { |i| ":a#{i}" }
      private_constant :BIND_BLOCK

      def bind_block; BIND_BLOCK; end

to lib/arel/visitors/oracle.rb fixes the issue for me.

The bind_block is essentially what visit_ActiveModel_Attribute and visit_Arel_Nodes_BindParam are doing, so with the bind block set, those methods are no longer needed.

dlagerro avatar Feb 19 '25 19:02 dlagerro

@dlagerro Thank you!!!! I knew I was papering over a broader issue in the hopes of being able to resolve other issues here first. I had spent more time on the broader issue but not made the breakthrough you appear to have made. I will look at your proposed change and revert the AREL conversion of the contains method. Thank you so much.

andynu avatar Feb 19 '25 20:02 andynu

@dlagerro's suggestion worked great for the bind parameters issue, and was similar to changes found in the postgres adapter.

I've also changed how you invoke the emulate_oracle_adapter feature. You used to have a database config named emulate_oracle_adapter and there was a handler that would dynamically require the OracleAdapter subclass of the OracleEnhancedAdapter class. That dynamic inclusion has moved into the framework itself where it is now easy to register different classes to match with the different adapter keywords. So I've replaced the feature here to use that functionality. Usually, people will want and use adapter: oracle_enhanced which maps to the OracleEnhancedAdapter class. And if you want to get an emulated OracleAdapter class, all you need to do is set your adapter: oracle and you'll get it, no special database config option needed anymore.

I believe this PR is ready to go. I welcome feedback.

andynu avatar Feb 19 '25 23:02 andynu

I have removed ruby 2.7 and 3.0 from the test matrix. They are not compatible with Rails 7.2

andynu avatar Feb 19 '25 23:02 andynu

I have removed ruby 2.7 and 3.0 from the test matrix. They are not compatible with Rails 7.2

Would it be worth adding Ruby 3.4 to the test matrix?

matthewtusker avatar Mar 25 '25 13:03 matthewtusker

Thanks for the work, @andynu!

We have this up and running in one of our projects with no issues.

We will be testing in more projects in the coming weeks.

@joshRpowell says he has the "branch has been running in app ~6mo now. No issues" https://github.com/rsim/oracle-enhanced/issues/2411#issuecomment-2884025571

Is the failing rubocop build the last thing holding up a release?

Anybody know which maintainer is taking lead on the approval of the PR?

dwarner avatar May 15 '25 16:05 dwarner

Thank you!

yahonda avatar Jun 13 '25 08:06 yahonda