-
Notifications
You must be signed in to change notification settings - Fork 21.8k
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
Conversation
collector << "1=0" | ||
else | ||
consumed = 0 | ||
collector = visit o.left, collector |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
6e5495d
to
41fac4c
Compare
ORA-01795: maximum number of expressions in a list is 1000
ORA-01795: maximum number of expressions in a list is 1000
41fac4c
to
aea26d2
Compare
Let me address CI errors. |
visit(sliced_o_right, collector) | ||
collector << ")" | ||
end | ||
end |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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 ```
aea26d2
to
029d05f
Compare
Opened #35941 for the CI failure of this pull request https://buildkite.com/rails/rails/builds/60325#c0a7fa75-4232-4aea-8435-d821b1ea504b . |
Follow up of rails#35838. And also this refactors `in_clause_length` handling is entirely integrated in Arel visitor.
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.
`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).
Summary
This WIP pull request adresses
ORA-01795: maximum number of expressions in a list is 1000
Two remaining things to remove [WIP].
in_clause_length
methods