rsim / oracle-enhanced

Oracle enhaced adapter for ActiveRecord
MIT License
545 stars 307 forks source link

Oracle12 visitor - Combination of limit and lock is not supported #2237

Open akostadinov opened 2 years ago

akostadinov commented 2 years ago

Oracle12 Arel visitor does not handle sub-queries with LIMIT properly and this makes locking a row impossible. While it works in normal queries, in sub-queries that doesn't seem to be the case.

Steps to reproduce

# just a minimal project `rails new --database=oracle
git clone https://github.com/akostadinov/rails-ora-02014.git
cd rails-ora-02014
vi config/database.yml # set your DB connection details
rails c
> a = Article.create!(title: "test", body: "test body")
> a.with_lock {}

Expected behavior

  Article Load (1.8ms)  SELECT "ARTICLES".* FROM "ARTICLES" WHERE "ARTICLES"."ID" = :a1 AND ROWNUM <= :a2  FOR UPDATE  [["id", 23], ["LIMIT", 1]]
=> nil

Actual behavior

Traceback (most recent call last):
/home/avalon/.asdf/installs/ruby/2.7.3/lib/ruby/gems/2.7.0/gems/activerecord-oracle_enhanced-adapter-7.0.0/lib/arel/visitors/oracle12.rb:10:in `visit_Arel_Nodes_SelectStatement': Combination of limit and lock is not supported. Because generated SQL statements (ArgumentError)
`SELECT FOR UPDATE and FETCH FIRST n ROWS` generates ORA-02014.

Possible workaround - use Arel::Visitors::Oracle

Arel::Visitors::Oracle works fine though because it always translates LIMIT statements to ROWNUM.

The example above properly works, if we add to application.rb:

ActiveSupport.on_load(:active_record) do
  ActiveRecord::ConnectionAdapters::OracleEnhancedAdapter.class_eval do
    # Use old visitor for Oracle 12c database
    self.use_old_oracle_visitor = true
  end
end

System configuration

Oracle Enhanced Adapter version 7.0.0

Rails version: Rails 7.0.1

Ruby version: ruby 2.7.3p183 (2021-04-05 revision 6847ee089d) [x86_64-linux]

Oracle version: 19.3.0

akostadinov commented 2 years ago

Seems to be related also to #1070

ajaya commented 1 year ago

@yahonda Can we get a little clarification on this lock subject. Most of the discussions on it are pretty old, and I can't seem to come up with a definitive approach to get row locking to work.

Most of my errors are ArgumentError (Combination of limit and lock is not supported. Because generated SQL statements) SELECT FOR UPDATE and FETCH FIRST n ROWSgenerates ORA-02014.

Doesn't matter, if I find_by_id, where(id: ) etc etc. Is with_lock just not supported? A little guidance would help. I can volunteer to put in a patch if I know what's the latest state on this and what's the history behind it.

AJ

ajaya commented 1 year ago

@yahonda

I see a possible solution from 2016. https://github.com/rsim/oracle-enhanced/issues/1066

Is this still the current approach?

AJ

joerixaop commented 1 year ago

One possible approach is using a query without limits. Assuming you're using where on a unique value it shouldn't really make a difference in amount of objects returned (and if you're not doing it on a unique value then the setting a limit of 1 is not going to reliably work anyway).

a = Article.create!(title: "test", body: "test body")
a = Article.lock(true).where(id: a.id).to_a[0]
...
a.save! # Make sure to unlock the row (if you are running in a transaction then ending the transaction should work too I think)
akostadinov commented 4 months ago

@joerixaop , thank you for the suggested workaround. It seems possible to workaround this under the hood like (on Rails 6.1.x):

    ActiveRecord::Relation.prepend(Module.new do
      def find_one(id)
        if ActiveRecord::Base === id
          raise ArgumentError, <<-MSG.squish
            You are passing an instance of ActiveRecord::Base to `find`.
            Please pass the id of the object by calling `.id`.
          MSG
        end

        relation = where(primary_key => id)
        record = relation.to_a.first # this is the only change from the original method

        raise_record_not_found_exception!(id, 0, 1) unless record

        record
      end
    end)

I think it is alright to patch all #find_one invocations, because finding by primary key should always find only one element and the database should optimize automatically without needing a hint with FETCH FIRST.

FYI my original workaround to use use_old_oracle_visitor is not viable on Rails 6.1 because some queries are messed up due to inserting ROWNUM field. It appears to be caused by https://github.com/rails/rails/commit/2ad2425fd3b3a58e937f97b1fa05306c4d3166ae#diff-f35ba011d4738058fa067289f7585a0224ef8ab22efe7a7ecdb07c2fed6940eaR419-R420

But since I don't expect anybody to be using an old oracle version anymore, I don't see much sense to file a separate bug about it. Much better would be to fix the new visitor somehow.