Handle Oracle IN conditions with more than 1000 items

cdinger commented 6 years ago

Oracle has a limit of 1000 item in IN conditions. Supplying an IN/NOT IN condition with more than 1000 items results in an ORA-01795 error. This change splits IN/NOT IN condition values into 1000-item chunks for the Oracle and Oracle12 visitors.

I think the introduction of the Arel::Visitors::OracleInCondition module is a little weird when it sits next to the other visitor classes, but it seemed better than duplicating the code.

Thanks for considering!

rails-bot commented 6 years ago

r? @tenderlove

(@rails-bot has picked a reviewer for you, use r? to override)

koic commented 6 years ago

This approach is interesting. @yahonda I'd like to hear your opinion about this PR since you have better background knowledge for this kind of changes.

yahonda commented 6 years ago

I like the idea since there are some requests and issues opened to address ORA-01795 maximum number of expressions in a list is 1000 like https://github.com/rsim/oracle-enhanced/issues/1160 . This pull request actually addresses the test case provided at https://github.com/rsim/oracle-enhanced/issues/1160

Then I have run Active Record unit tests using this pull request and found there are more than 20 errors introduced. Fortunately, all of error messages are identical.

Arel::Visitors::UnsupportedVisitError: Unsupported argument type: String. Construct an Arel node instead.

Here is the step to reproduce Arel::Visitors::UnsupportedVisitError:

Environment

ruby 2.5.1p57 (2018-03-29 revision 63029) [x86_64-linux]
Oracle Database 12c Enterprise Edition Release 12.2.0.1.0 - 64bit Production
Rails master branch with the latest commit is c9b606dfe799ba93494f3e21c913f9505d97888a
Arel with this commit 1bf86cb652ec7653841af93d4c5b21121718f082

$ cd rails/activerecord
$ ARCONN=oracle bundle exec ruby -Itest test/cases/relations_test.rb -n test_finding_with_subquery_with_eager_loading_in_where
Using oracle
Run options: -n test_finding_with_subquery_with_eager_loading_in_where --seed 15013

# Running:

E

Error:
RelationTest#test_finding_with_subquery_with_eager_loading_in_where:
Arel::Visitors::UnsupportedVisitError: Unsupported argument type: String. Construct an Arel node instead.
    /home/yahonda/git/arel/lib/arel/visitors/to_sql.rb:754:in `unsupported'
    /home/yahonda/git/arel/lib/arel/visitors/visitor.rb:29:in `visit'
    /home/yahonda/git/arel/lib/arel/visitors/to_sql.rb:820:in `block in inject_join'
    /home/yahonda/git/arel/lib/arel/visitors/to_sql.rb:816:in `each'
    /home/yahonda/git/arel/lib/arel/visitors/to_sql.rb:816:in `each_with_index'
    /home/yahonda/git/arel/lib/arel/visitors/to_sql.rb:816:in `each'
    /home/yahonda/git/arel/lib/arel/visitors/to_sql.rb:816:in `inject'
    /home/yahonda/git/arel/lib/arel/visitors/to_sql.rb:816:in `inject_join'
    /home/yahonda/git/arel/lib/arel/visitors/to_sql.rb:789:in `visit_Array'
    /home/yahonda/git/arel/lib/arel/visitors/visitor.rb:29:in `visit'
    /home/yahonda/git/arel/lib/arel/visitors/oracle_in_condition.rb:23:in `block in visit_Arel_Nodes_In'
    /home/yahonda/git/arel/lib/arel/visitors/oracle_in_condition.rb:15:in `each_slice'
    /home/yahonda/git/arel/lib/arel/visitors/oracle_in_condition.rb:15:in `visit_Arel_Nodes_In'
    /home/yahonda/git/rails/activerecord/lib/active_record/connection_adapters/determine_if_preparable_visitor.rb:15:in `visit_Arel_Nodes_In'
    /home/yahonda/git/arel/lib/arel/visitors/visitor.rb:29:in `visit'
    /home/yahonda/git/arel/lib/arel/visitors/to_sql.rb:818:in `block in inject_join'
    /home/yahonda/git/arel/lib/arel/visitors/to_sql.rb:816:in `each'
    /home/yahonda/git/arel/lib/arel/visitors/to_sql.rb:816:in `each_with_index'
    /home/yahonda/git/arel/lib/arel/visitors/to_sql.rb:816:in `each'
    /home/yahonda/git/arel/lib/arel/visitors/to_sql.rb:816:in `inject'
    /home/yahonda/git/arel/lib/arel/visitors/to_sql.rb:816:in `inject_join'
    /home/yahonda/git/arel/lib/arel/visitors/to_sql.rb:635:in `visit_Arel_Nodes_And'
    /home/yahonda/git/arel/lib/arel/visitors/visitor.rb:29:in `visit'
    /home/yahonda/git/arel/lib/arel/visitors/to_sql.rb:266:in `block in collect_nodes_for'
    /home/yahonda/git/arel/lib/arel/visitors/to_sql.rb:265:in `each'
    /home/yahonda/git/arel/lib/arel/visitors/to_sql.rb:265:in `each_with_index'
    /home/yahonda/git/arel/lib/arel/visitors/to_sql.rb:265:in `collect_nodes_for'
    /home/yahonda/git/arel/lib/arel/visitors/to_sql.rb:250:in `visit_Arel_Nodes_SelectCore'
    /home/yahonda/git/arel/lib/arel/visitors/to_sql.rb:213:in `block in visit_Arel_Nodes_SelectStatement'
    /home/yahonda/git/arel/lib/arel/visitors/to_sql.rb:212:in `each'
    /home/yahonda/git/arel/lib/arel/visitors/to_sql.rb:212:in `inject'
    /home/yahonda/git/arel/lib/arel/visitors/to_sql.rb:212:in `visit_Arel_Nodes_SelectStatement'
    /home/yahonda/git/arel/lib/arel/visitors/oracle12.rb:21:in `visit_Arel_Nodes_SelectStatement'
    /home/yahonda/git/arel/lib/arel/visitors/visitor.rb:29:in `visit'
    /home/yahonda/git/arel/lib/arel/visitors/visitor.rb:10:in `accept'
    /home/yahonda/git/rails/activerecord/lib/active_record/connection_adapters/determine_if_preparable_visitor.rb:10:in `accept'
    /home/yahonda/git/rails/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb:23:in `to_sql_and_binds'
    /home/yahonda/git/rails/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb:48:in `select_all'
    /home/yahonda/git/rails/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb:101:in `select_all'
    /home/yahonda/git/rails/activerecord/lib/active_record/querying.rb:41:in `find_by_sql'
    /home/yahonda/git/rails/activerecord/lib/active_record/relation.rb:586:in `block in exec_queries'
    /home/yahonda/git/rails/activerecord/lib/active_record/relation.rb:604:in `skip_query_cache_if_necessary'
    /home/yahonda/git/rails/activerecord/lib/active_record/relation.rb:574:in `exec_queries'
    /home/yahonda/git/rails/activerecord/lib/active_record/relation.rb:437:in `load'
    /home/yahonda/git/rails/activerecord/lib/active_record/relation.rb:230:in `records'
    /home/yahonda/git/rails/activerecord/lib/active_record/relation/delegation.rb:41:in `each'
    test/cases/relations_test.rb:208:in `sort_by'
    test/cases/relations_test.rb:208:in `test_finding_with_subquery_with_eager_loading_in_where'

bin/rails test test/cases/relations_test.rb:206

Finished in 29.216781s, 0.0342 runs/s, 0.0000 assertions/s.
1 runs, 0 assertions, 0 failures, 1 errors, 0 skips
$

I have not done some more investigation of this error but wanted to show the information what I have now so far.

Thanks again for your great work. I wil take a look at this error and will investigate some detail.

matthewd commented 6 years ago

Per #523, Arel development is moving to rails/rails.

If this PR is still relevant, please consider reopening it over there.

rails / arel

Handle Oracle IN conditions with more than 1000 items #527

Environment