Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Address ORA-01795: maximum number of expressions in a list is 1000 #35838

Merged
merged 1 commit into from
Apr 11, 2019

Conversation

yahonda
Copy link
Member

@yahonda yahonda commented Apr 3, 2019

Summary

This WIP pull request adresses ORA-01795: maximum number of expressions in a list is 1000

$ ARCONN=oracle bin/test test/cases/bind_parameter_test.rb -n test_too_many_binds

... snip ...

Error:
ActiveRecord::BindParameterTest#test_too_many_binds:
ActiveRecord::StatementInvalid: OCIError: ORA-01795: maximum number of expressions in a list is 1000
    stmt.c:267:in oci8lib_260.so
    /home/yahonda/.rbenv/versions/2.6.2/lib/ruby/gems/2.6.0/gems/ruby-oci8-2.2.7/lib/oci8/cursor.rb:131:in `exec'
    /home/yahonda/git/oracle-enhanced/lib/active_record/connection_adapters/oracle_enhanced/oci_connection.rb:142:in `exec'
    /home/yahonda/git/oracle-enhanced/lib/active_record/connection_adapters/oracle_enhanced/database_statements.rb:41:in `block in exec_query'
    /home/yahonda/git/rails/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb:676:in `block (2 levels) in log'
    /home/yahonda/.rbenv/versions/2.6.2/lib/ruby/2.6.0/monitor.rb:230:in `mon_synchronize'
    /home/yahonda/git/rails/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb:675:in `block in log'
    /home/yahonda/git/rails/activesupport/lib/active_support/notifications/instrumenter.rb:24:in `instrument'
    /home/yahonda/git/rails/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb:666:in `log'
    /home/yahonda/git/oracle-enhanced/lib/active_record/connection_adapters/oracle_enhanced/dbms_output.rb:36:in `log'
    /home/yahonda/git/oracle-enhanced/lib/active_record/connection_adapters/oracle_enhanced/database_statements.rb:24:in `exec_query'
    /home/yahonda/git/rails/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb:484:in `select'
    /home/yahonda/git/rails/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb:70:in `select_all'
    /home/yahonda/git/rails/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb:106:in `select_all'
    /home/yahonda/git/rails/activerecord/lib/active_record/relation/calculations.rb:299:in `block in execute_simple_calculation'
    /home/yahonda/git/rails/activerecord/lib/active_record/relation.rb:755:in `skip_query_cache_if_necessary'
    /home/yahonda/git/rails/activerecord/lib/active_record/relation/calculations.rb:299:in `execute_simple_calculation'
    /home/yahonda/git/rails/activerecord/lib/active_record/relation/calculations.rb:251:in `perform_calculation'
    /home/yahonda/git/rails/activerecord/lib/active_record/relation/calculations.rb:141:in `calculate'
    /home/yahonda/git/rails/activerecord/lib/active_record/relation/calculations.rb:49:in `count'
    /home/yahonda/git/rails/activerecord/test/cases/bind_parameter_test.rb:113:in `test_too_many_binds'

bin/test test/cases/bind_parameter_test.rb:109

.............................F

Two remaining things to remove [WIP].

  • Duplicate in_clause_length methods
  • Support Oracle visitor for 11g or lower

collector << "1=0"
else
consumed = 0
collector = visit o.left, collector
Copy link
Member

Choose a reason for hiding this comment

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

I think this (and the line below) should move inside the loop... #{quote_column_name(o.left.name)} seems overly presumptuous about o.left's type?

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the comment. I did not have time to respond to your comment because I've been investigating ORA-00913 errors. Let me have some more look at if these lines can be moved inside the loop.

Copy link
Member Author

Choose a reason for hiding this comment

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

If I understand the first part of the comment correctly, It generates incorrect and redundant IN clause like below.

:a1000) OR "POSTS"."ID" IN ("POSTS"."ID" IN (:a1001, :a1002, :a1003, :a1004, :a1005, :a1006, :a1007, :a1008, :a1009, :a1010)  
$ git diff
diff --git a/activerecord/lib/arel/visitors/oracle12.rb b/activerecord/lib/arel/visitors/oracle12.rb
index 35fbfddfe5..3ea646e4bf 100644
--- a/activerecord/lib/arel/visitors/oracle12.rb
+++ b/activerecord/lib/arel/visitors/oracle12.rb
@@ -50,9 +50,9 @@ def visit_Arel_Nodes_In(o, collector)
             collector << "1=0"
           else
             consumed = 0
-            collector = visit o.left, collector
-            collector << " IN ("
             o.right.each_slice(in_clause_length) do |sliced_o_right|
+              collector = visit o.left, collector
+              collector << " IN ("
               consumed = consumed + sliced_o_right.size
               visit(sliced_o_right, collector)
               if consumed != o.right.size
$

Let me confirm if I understand your comment corrrectly.

Copy link
Member Author

Choose a reason for hiding this comment

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

#{quote_column_name(o.left.name)} seems overly presumptuous about o.left's type?

Right. I expect o.left.name here returns column name. Would you let me know how can I "guarantee" and/or validate o.oeft.name is always a column name? becaue I have no good idea yet.

Copy link
Member

Choose a reason for hiding this comment

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

I was thinking of something like this:

        def visit_Arel_Nodes_In(o, collector)
          if Array === o.right && !o.right.empty?
            o.right.delete_if { |value| unboundable?(value) }
          end

          if Array === o.right && o.right.empty?
            collector << "1=0"
          else
            first = true
            o.right.each_slice(in_clause_length) do |sliced_o_right|
              collector << " OR " unless first
              first = false

              collector = visit o.left, collector
              collector << " IN ("
              collector = visit sliced_o_right, collector
              collector << ")"
            end
          end
        end

@yahonda yahonda force-pushed the more_than_1000_inlist branch 3 times, most recently from 6e5495d to 41fac4c Compare April 9, 2019 14:50
@yahonda yahonda changed the title [WIP] Address ORA-01795: maximum number of expressions in a list is 1000 Address ORA-01795: maximum number of expressions in a list is 1000 Apr 10, 2019
@yahonda
Copy link
Member Author

yahonda commented Apr 11, 2019

Let me address CI errors.

visit(sliced_o_right, collector)
collector << ")"
end
end
Copy link
Member

Choose a reason for hiding this comment

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

I think I missed a collector return value here

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the update. Right, I should have been aware of that. I appreciate your kind review and I learned some more visitor patterns.

* To address this error, this commit splits expressions by slices of 1000 elements.

* "Oracle Database Error Messages 18c"
https://docs.oracle.com/en/database/oracle/oracle-database/18/errmg/

```
ORA-01795: maximum number of expressions in a list is 1000
Cause: Number of expressions in the query exceeded than 1000. Note that unused column/expressions are also counted Maximum number of expressions that are allowed are 1000.
```

* This commit addresses this ORA-01795 error

Note: Actually addressing this error raises another "ORA-00913: too many values"
Number of values Oracle database allows is 65535 regardless bind values or literal values.

```ruby
$ ARCONN=oracle bin/test test/cases/bind_parameter_test.rb -n test_too_many_binds

... snip ...

Error:
ActiveRecord::BindParameterTest#test_too_many_binds:
ActiveRecord::StatementInvalid: OCIError: ORA-01795: maximum number of expressions in a list is 1000
    stmt.c:267:in oci8lib_260.so
    /home/yahonda/.rbenv/versions/2.6.2/lib/ruby/gems/2.6.0/gems/ruby-oci8-2.2.7/lib/oci8/cursor.rb:131:in `exec'
    /home/yahonda/git/oracle-enhanced/lib/active_record/connection_adapters/oracle_enhanced/oci_connection.rb:142:in `exec'
    /home/yahonda/git/oracle-enhanced/lib/active_record/connection_adapters/oracle_enhanced/database_statements.rb:41:in `block in exec_query'
    /home/yahonda/git/rails/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb:676:in `block (2 levels) in log'
    /home/yahonda/.rbenv/versions/2.6.2/lib/ruby/2.6.0/monitor.rb:230:in `mon_synchronize'
    /home/yahonda/git/rails/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb:675:in `block in log'
    /home/yahonda/git/rails/activesupport/lib/active_support/notifications/instrumenter.rb:24:in `instrument'
    /home/yahonda/git/rails/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb:666:in `log'
    /home/yahonda/git/oracle-enhanced/lib/active_record/connection_adapters/oracle_enhanced/dbms_output.rb:36:in `log'
    /home/yahonda/git/oracle-enhanced/lib/active_record/connection_adapters/oracle_enhanced/database_statements.rb:24:in `exec_query'
    /home/yahonda/git/rails/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb:484:in `select'
    /home/yahonda/git/rails/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb:70:in `select_all'
    /home/yahonda/git/rails/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb:106:in `select_all'
    /home/yahonda/git/rails/activerecord/lib/active_record/relation/calculations.rb:299:in `block in execute_simple_calculation'
    /home/yahonda/git/rails/activerecord/lib/active_record/relation.rb:755:in `skip_query_cache_if_necessary'
    /home/yahonda/git/rails/activerecord/lib/active_record/relation/calculations.rb:299:in `execute_simple_calculation'
    /home/yahonda/git/rails/activerecord/lib/active_record/relation/calculations.rb:251:in `perform_calculation'
    /home/yahonda/git/rails/activerecord/lib/active_record/relation/calculations.rb:141:in `calculate'
    /home/yahonda/git/rails/activerecord/lib/active_record/relation/calculations.rb:49:in `count'
    /home/yahonda/git/rails/activerecord/test/cases/bind_parameter_test.rb:113:in `test_too_many_binds'

bin/test test/cases/bind_parameter_test.rb:109

.............................F

```
@yahonda
Copy link
Member Author

yahonda commented Apr 11, 2019

Opened #35941 for the CI failure of this pull request https://buildkite.com/rails/rails/builds/60325#c0a7fa75-4232-4aea-8435-d821b1ea504b .

@rafaelfranca rafaelfranca merged commit 032d4ad into rails:master Apr 11, 2019
@yahonda yahonda deleted the more_than_1000_inlist branch April 18, 2019 13:55
kamipo added a commit to kamipo/rails that referenced this pull request Apr 24, 2019
Follow up of rails#35838.

And also this refactors `in_clause_length` handling is entirely
integrated in Arel visitor.
kamipo added a commit to kamipo/rails that referenced this pull request Apr 15, 2020
This removes ibm_db, informix, mssql, oracle, and oracle12 Arel visitors
which are not used in the code base.

Actually oracle and oracle12 visitors are used at oracle-enhanced
adapter, but now I think that those visitors should be in the adapter's
repo like sqlserver adapter and the dedicated Arel visitor
(https://github.com/rails-sqlserver/activerecord-sqlserver-adapter/blob/master/lib/arel/visitors/sqlserver.rb),
otherwise it is hard to find a bug and review PRs for the oracle
visitors (e.g. rails#35838, rails#37646), since we don't have knowledge and
environment enough for Oracle.
kamipo added a commit to kamipo/rails that referenced this pull request Apr 26, 2020
`in_clause_length` was added at c5a284f to address to Oracle IN clause
length limitation.

Now `in_clause_length` is entirely integrated in Arel visitor since
rails#35838 and rails#36074.

Since Oracle visitors are the only code that rely on `in_clause_length`.
so I'd like to remove that from Rails code base, like has removed Oracle
visitors (rails#38946).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants