Oracle visitor needs two more `add_bind` argument fix

[yahonda]

Refer https://github.com/rails/arel/commit/53521a9e39b9d8af4165d7703c36dc905f1f8f67

This pull request addresses this failure at Oracle enhanced unit test.

$ bundle exec rspec ./spec/active_record/connection_adapters/oracle_enhanced_adapter_spec.rb:628
==> Loading config from ENV or use default
==> Running specs with MRI version 2.5.0
/home/yahonda/git/rails/activesupport/lib/active_support/core_ext/hash/keys.rb:14: warning: method redefined; discarding old transform_keys
/home/yahonda/git/rails/activesupport/lib/active_support/core_ext/hash/keys.rb:25: warning: method redefined; discarding old transform_keys!
==> Effective ActiveRecord version 5.2.0.alpha
Run options: include {:locations=>{"./spec/active_record/connection_adapters/oracle_enhanced_adapter_spec.rb"=>[628]}}
F

Failures:

  1) OracleEnhancedAdapter using offset and limit should return the records starting from offset n with offset(n)
     Failure/Error: type_casted_binds = type_casted_binds(binds)

     NoMethodError:
       undefined method `value_for_database' for #<Arel::Nodes::BindParam:0x000055852a3e64d0>
     # /home/yahonda/git/rails/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb:161:in `block in type_casted_binds'
     # /home/yahonda/git/rails/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb:161:in `map'
     # /home/yahonda/git/rails/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb:161:in `type_casted_binds'
     # ./lib/active_record/connection_adapters/oracle_enhanced/database_statements.rb:22:in `exec_query'
     # /home/yahonda/git/rails/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb:427:in `select_prepared'
     # /home/yahonda/git/rails/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb:49:in `select_all'
     # /home/yahonda/git/rails/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb:99: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:664:in `block in exec_queries'
     # /home/yahonda/git/rails/activerecord/lib/active_record/relation.rb:687:in `skip_query_cache_if_necessary'
     # /home/yahonda/git/rails/activerecord/lib/active_record/relation.rb:663:in `exec_queries'
     # /home/yahonda/git/rails/activerecord/lib/active_record/relation.rb:542:in `load'
     # /home/yahonda/git/rails/activerecord/lib/active_record/relation.rb:251:in `records'
     # /home/yahonda/git/rails/activerecord/lib/active_record/relation.rb:247:in `to_a'
     # /home/yahonda/git/rails/activerecord/lib/active_record/relation/finder_methods.rb:541:in `find_nth_with_limit'
     # /home/yahonda/git/rails/activerecord/lib/active_record/relation/finder_methods.rb:530:in `find_nth'
     # /home/yahonda/git/rails/activerecord/lib/active_record/relation/finder_methods.rb:124:in `first'
     # ./spec/active_record/connection_adapters/oracle_enhanced_adapter_spec.rb:630:in `block (3 levels) in <top (required)>'
     # /home/yahonda/.rbenv/versions/2.5.0-dev/lib/ruby/gems/2.5.0/gems/rspec-core-3.6.0/lib/rspec/core/example.rb:254:in `instance_exec'
     # /home/yahonda/.rbenv/versions/2.5.0-dev/lib/ruby/gems/2.5.0/gems/rspec-core-3.6.0/lib/rspec/core/example.rb:254:in `block in run'
     # /home/yahonda/.rbenv/versions/2.5.0-dev/lib/ruby/gems/2.5.0/gems/rspec-core-3.6.0/lib/rspec/core/example.rb:500:in `block in with_around_and_singleton_context_hooks'
     # /home/yahonda/.rbenv/versions/2.5.0-dev/lib/ruby/gems/2.5.0/gems/rspec-core-3.6.0/lib/rspec/core/example.rb:457:in `block in with_around_example_hooks'
     # /home/yahonda/.rbenv/versions/2.5.0-dev/lib/ruby/gems/2.5.0/gems/rspec-core-3.6.0/lib/rspec/core/hooks.rb:464:in `block in run'
     # /home/yahonda/.rbenv/versions/2.5.0-dev/lib/ruby/gems/2.5.0/gems/rspec-core-3.6.0/lib/rspec/core/hooks.rb:602:in `run_around_example_hooks_for'
     # /home/yahonda/.rbenv/versions/2.5.0-dev/lib/ruby/gems/2.5.0/gems/rspec-core-3.6.0/lib/rspec/core/hooks.rb:464:in `run'
     # /home/yahonda/.rbenv/versions/2.5.0-dev/lib/ruby/gems/2.5.0/gems/rspec-core-3.6.0/lib/rspec/core/example.rb:457:in `with_around_example_hooks'
     # /home/yahonda/.rbenv/versions/2.5.0-dev/lib/ruby/gems/2.5.0/gems/rspec-core-3.6.0/lib/rspec/core/example.rb:500:in `with_around_and_singleton_context_hooks'
     # /home/yahonda/.rbenv/versions/2.5.0-dev/lib/ruby/gems/2.5.0/gems/rspec-core-3.6.0/lib/rspec/core/example.rb:251:in `run'
     # /home/yahonda/.rbenv/versions/2.5.0-dev/lib/ruby/gems/2.5.0/gems/rspec-core-3.6.0/lib/rspec/core/example_group.rb:627:in `block in run_examples'
     # /home/yahonda/.rbenv/versions/2.5.0-dev/lib/ruby/gems/2.5.0/gems/rspec-core-3.6.0/lib/rspec/core/example_group.rb:623:in `map'
     # /home/yahonda/.rbenv/versions/2.5.0-dev/lib/ruby/gems/2.5.0/gems/rspec-core-3.6.0/lib/rspec/core/example_group.rb:623:in `run_examples'
     # /home/yahonda/.rbenv/versions/2.5.0-dev/lib/ruby/gems/2.5.0/gems/rspec-core-3.6.0/lib/rspec/core/example_group.rb:589:in `run'
     # /home/yahonda/.rbenv/versions/2.5.0-dev/lib/ruby/gems/2.5.0/gems/rspec-core-3.6.0/lib/rspec/core/example_group.rb:590:in `block in run'
     # /home/yahonda/.rbenv/versions/2.5.0-dev/lib/ruby/gems/2.5.0/gems/rspec-core-3.6.0/lib/rspec/core/example_group.rb:590:in `map'
     # /home/yahonda/.rbenv/versions/2.5.0-dev/lib/ruby/gems/2.5.0/gems/rspec-core-3.6.0/lib/rspec/core/example_group.rb:590:in `run'
     # /home/yahonda/.rbenv/versions/2.5.0-dev/lib/ruby/gems/2.5.0/gems/rspec-core-3.6.0/lib/rspec/core/runner.rb:118:in `block (3 levels) in run_specs'
     # /home/yahonda/.rbenv/versions/2.5.0-dev/lib/ruby/gems/2.5.0/gems/rspec-core-3.6.0/lib/rspec/core/runner.rb:118:in `map'
     # /home/yahonda/.rbenv/versions/2.5.0-dev/lib/ruby/gems/2.5.0/gems/rspec-core-3.6.0/lib/rspec/core/runner.rb:118:in `block (2 levels) in run_specs'
     # /home/yahonda/.rbenv/versions/2.5.0-dev/lib/ruby/gems/2.5.0/gems/rspec-core-3.6.0/lib/rspec/core/configuration.rb:1894:in `with_suite_hooks'
     # /home/yahonda/.rbenv/versions/2.5.0-dev/lib/ruby/gems/2.5.0/gems/rspec-core-3.6.0/lib/rspec/core/runner.rb:113:in `block in run_specs'
     # /home/yahonda/.rbenv/versions/2.5.0-dev/lib/ruby/gems/2.5.0/gems/rspec-core-3.6.0/lib/rspec/core/reporter.rb:79:in `report'
     # /home/yahonda/.rbenv/versions/2.5.0-dev/lib/ruby/gems/2.5.0/gems/rspec-core-3.6.0/lib/rspec/core/runner.rb:112:in `run_specs'
     # /home/yahonda/.rbenv/versions/2.5.0-dev/lib/ruby/gems/2.5.0/gems/rspec-core-3.6.0/lib/rspec/core/runner.rb:87:in `run'
     # /home/yahonda/.rbenv/versions/2.5.0-dev/lib/ruby/gems/2.5.0/gems/rspec-core-3.6.0/lib/rspec/core/runner.rb:71:in `run'
     # /home/yahonda/.rbenv/versions/2.5.0-dev/lib/ruby/gems/2.5.0/gems/rspec-core-3.6.0/lib/rspec/core/runner.rb:45:in `invoke'
     # /home/yahonda/.rbenv/versions/2.5.0-dev/lib/ruby/gems/2.5.0/gems/rspec-core-3.6.0/exe/rspec:4:in `<top (required)>'
     # /home/yahonda/.rbenv/versions/2.5.0-dev/bin/rspec:23:in `load'
     # /home/yahonda/.rbenv/versions/2.5.0-dev/bin/rspec:23:in `<top (required)>'
     # /home/yahonda/.rbenv/versions/2.5.0-dev/lib/ruby/gems/2.5.0/gems/bundler-1.15.3/lib/bundler/cli/exec.rb:74:in `load'
     # /home/yahonda/.rbenv/versions/2.5.0-dev/lib/ruby/gems/2.5.0/gems/bundler-1.15.3/lib/bundler/cli/exec.rb:74:in `kernel_load'
     # /home/yahonda/.rbenv/versions/2.5.0-dev/lib/ruby/gems/2.5.0/gems/bundler-1.15.3/lib/bundler/cli/exec.rb:27:in `run'
     # /home/yahonda/.rbenv/versions/2.5.0-dev/lib/ruby/gems/2.5.0/gems/bundler-1.15.3/lib/bundler/cli.rb:365:in `exec'
     # /home/yahonda/.rbenv/versions/2.5.0-dev/lib/ruby/gems/2.5.0/gems/bundler-1.15.3/lib/bundler/vendor/thor/lib/thor/command.rb:27:in `run'
     # /home/yahonda/.rbenv/versions/2.5.0-dev/lib/ruby/gems/2.5.0/gems/bundler-1.15.3/lib/bundler/vendor/thor/lib/thor/invocation.rb:126:in `invoke_command'
     # /home/yahonda/.rbenv/versions/2.5.0-dev/lib/ruby/gems/2.5.0/gems/bundler-1.15.3/lib/bundler/vendor/thor/lib/thor.rb:369:in `dispatch'
     # /home/yahonda/.rbenv/versions/2.5.0-dev/lib/ruby/gems/2.5.0/gems/bundler-1.15.3/lib/bundler/cli.rb:22:in `dispatch'
     # /home/yahonda/.rbenv/versions/2.5.0-dev/lib/ruby/gems/2.5.0/gems/bundler-1.15.3/lib/bundler/vendor/thor/lib/thor/base.rb:444:in `start'
     # /home/yahonda/.rbenv/versions/2.5.0-dev/lib/ruby/gems/2.5.0/gems/bundler-1.15.3/lib/bundler/cli.rb:13:in `start'
     # /home/yahonda/.rbenv/versions/2.5.0-dev/lib/ruby/gems/2.5.0/gems/bundler-1.15.3/exe/bundle:30:in `block in <top (required)>'
     # /home/yahonda/.rbenv/versions/2.5.0-dev/lib/ruby/gems/2.5.0/gems/bundler-1.15.3/lib/bundler/friendly_errors.rb:121:in `with_friendly_errors'
     # /home/yahonda/.rbenv/versions/2.5.0-dev/lib/ruby/gems/2.5.0/gems/bundler-1.15.3/exe/bundle:22:in `<top (required)>'
     # /home/yahonda/.rbenv/versions/2.5.0-dev/bin/bundle:23:in `load'
     # /home/yahonda/.rbenv/versions/2.5.0-dev/bin/bundle:23:in `<main>'

Finished in 0.42775 seconds (files took 0.77656 seconds to load)
1 example, 1 failure

Failed examples:

rspec ./spec/active_record/connection_adapters/oracle_enhanced_adapter_spec.rb:628 # OracleEnhancedAdapter using offset and limit should return the records starting from offset n with offset(n)

Coverage report generated for RSpec to /home/yahonda/git/oracle-enhanced/coverage. 1027 / 2229 LOC (46.07%) covered.
$

[rails-bot]

r? @rafaelfranca

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

[yahonda]

Updated pull request to reflect reviews.

Note: this method needs to bind offset value twice. The bind value name for the second offset changed from :b1 to :b3 by using visitor pattern here and confirmed the first offset :b1 and the second offset :b3 have the same value.

• Current behavior with this pull request

  D, [2017-07-25T22:16:51.986248 #27602] DEBUG -- :    Load (4.1ms)
            SELECT * FROM (
              SELECT raw_sql_.*, rownum raw_rnum_
              FROM (SELECT  "TEST_EMPLOYEES".* FROM "TEST_EMPLOYEES" ORDER BY "TEST_EMPLOYEES"."SORT_ORDER" ASC ) raw_sql_ WHERE rownum <= (:a1 + :a2) ) WHERE raw_rnum_ > :a3  [["OFFSET", 0], ["LIMIT", 1], ["OFFSET", 0]]
  D, [2017-07-25T22:16:51.989567 #27602] DEBUG -- :    Load (1.0ms)
            SELECT * FROM (
              SELECT raw_sql_.*, rownum raw_rnum_
              FROM (SELECT  "TEST_EMPLOYEES".* FROM "TEST_EMPLOYEES" ORDER BY "TEST_EMPLOYEES"."SORT_ORDER" ASC ) raw_sql_ WHERE rownum <= (:a1 + :a2) ) WHERE raw_rnum_ > :a3  [["OFFSET", 1], ["LIMIT", 1], ["OFFSET", 1]]
  D, [2017-07-25T22:16:51.991870 #27602] DEBUG -- :    Load (0.6ms)
            SELECT * FROM (
              SELECT raw_sql_.*, rownum raw_rnum_
              FROM (SELECT  "TEST_EMPLOYEES".* FROM "TEST_EMPLOYEES" ORDER BY "TEST_EMPLOYEES"."SORT_ORDER" ASC ) raw_sql_ WHERE rownum <= (:a1 + :a2) ) WHERE raw_rnum_ > :a3  [["OFFSET", 4], ["LIMIT", 1], ["OFFSET", 4]]

• Previous behavior

  D, [2017-07-25T22:18:01.061554 #27751] DEBUG -- :    Load (4.8ms)
            SELECT * FROM (
              SELECT raw_sql_.*, rownum raw_rnum_
              FROM (SELECT  "TEST_EMPLOYEES".* FROM "TEST_EMPLOYEES" ORDER BY "TEST_EMPLOYEES"."SORT_ORDER" ASC ) raw_sql_ WHERE rownum <= (:a1 + :a2) ) WHERE raw_rnum_ > :a1  [["OFFSET", 0], ["LIMIT", 1]]
  D, [2017-07-25T22:18:01.065624 #27751] DEBUG -- :    Load (1.1ms)
            SELECT * FROM (
              SELECT raw_sql_.*, rownum raw_rnum_
              FROM (SELECT  "TEST_EMPLOYEES".* FROM "TEST_EMPLOYEES" ORDER BY "TEST_EMPLOYEES"."SORT_ORDER" ASC ) raw_sql_ WHERE rownum <= (:a1 + :a2) ) WHERE raw_rnum_ > :a1  [["OFFSET", 1], ["LIMIT", 1]]
  D, [2017-07-25T22:18:01.068899 #27751] DEBUG -- :    Load (1.0ms)
            SELECT * FROM (
              SELECT raw_sql_.*, rownum raw_rnum_
              FROM (SELECT  "TEST_EMPLOYEES".* FROM "TEST_EMPLOYEES" ORDER BY "TEST_EMPLOYEES"."SORT_ORDER" ASC ) raw_sql_ WHERE rownum <= (:a1 + :a2) ) WHERE raw_rnum_ > :a1  [["OFFSET", 4], ["LIMIT", 1]]

[sgrif]

Ah I see. In that case your original fix was the right thing to do.

[yahonda]

Pushed the original fix again.

[yahonda]

I've found the original fix did not resolve this error when tested with JRuby and JDBC driver since bind value naming convention differences between OCI(for CRuby) and JDBC(for JRuby).

       Java::JavaSql::SQLException: Missing IN or OUT parameter at index:: 3: 

https://travis-ci.org/rsim/oracle-enhanced/jobs/257787629

Going back to visitor pattern again and it works with both CRuby and JRuby.

[rafaelfranca]

r? @sgrif