Avoid extra BindParam allocation to generate placeholder in queries

[koic]

Resolves #2152 and follow rails/rails#41577.

The tests that fail below have not yet been resolved.
rails/rails@6ee96a8.

[yahonda]

ORA-01722: invalid number error occurred at this statement, likely at INTO :returning_id.

ActiveRecord::InternalMetadata Create (19506.5ms)  INSERT INTO "AR_INTERNAL_METADATA" ("KEY", "VALUE", "CREATED_AT", "UPDATED_AT") VALUES (:a1, :a2, :a3, :a4) RETURNING "KEY" INTO :returning_id  [["key", "environment"], ["value", "default_env"], ["created_at", "2021-03-29 12:47:19.056199"], ["updated_at", "2021-03-29 12:47:19.056199"], ["returning_id", nil]]

Compared to this statement with that of Oracle enhanced adapter release61 branch, it does not have INTO :returning_id.

ctiveRecord::InternalMetadata Create (6.8ms) INSERT INTO "AR_INTERNAL_METADATA" ("KEY", "VALUE", "CREATED_AT", "UPDATED_AT") VALUES (:a1, :a2, :a3, :a4) [["key", "environment"], ["value", "default_env"], ["created_at", "2021-03-29 23:06:55.059484"], ["updated_at", "2021-03-29 23:06:55.059484"]]

Then just calling super at sql_for_insert method workarounds ORA-1722 error.

$ git diff 4f03593176b220f48e565bdf4df13e439f821e64...b19e8f4022768541605628bf376164ed1ccd4f78
diff --git a/lib/active_record/connection_adapters/oracle_enhanced/database_statements.rb b/lib/active_record/connection_adapters/oracle_enhanced/database_statements.rb
index 54144633..814c6b9b 100644
--- a/lib/active_record/connection_adapters/oracle_enhanced/database_statements.rb
+++ b/lib/active_record/connection_adapters/oracle_enhanced/database_statements.rb
@@ -79,10 +79,6 @@ module ActiveRecord
         # New method in ActiveRecord 3.1
         # Will add RETURNING clause in case of trigger generated primary keys
         def sql_for_insert(sql, pk, binds)
-          unless pk == false || pk.nil? || pk.is_a?(Array)
-            sql = "#{sql} RETURNING #{quote_column_name(pk)} INTO :returning_id"
-            (binds = binds.dup) << ActiveRecord::Relation::QueryAttribute.new("returning_id", nil, Type::OracleEnhanced::Integer.new)
-          end
           super
         end

$

[yahonda]

@koic Would you add this change to this pull request? then CI should be green.

$ git diff
diff --git a/lib/active_record/connection_adapters/oracle_enhanced/database_statements.rb b/lib/active_record/connection_adapters/oracle_enhanced/database_statements.rb
index 54144633..631dcffa 100644
--- a/lib/active_record/connection_adapters/oracle_enhanced/database_statements.rb
+++ b/lib/active_record/connection_adapters/oracle_enhanced/database_statements.rb
@@ -79,7 +79,7 @@ module ActiveRecord
         # New method in ActiveRecord 3.1
         # Will add RETURNING clause in case of trigger generated primary keys
         def sql_for_insert(sql, pk, binds)
-          unless pk == false || pk.nil? || pk.is_a?(Array)
+          unless pk == false || pk.nil? || pk.is_a?(Array) || pk.is_a?(String)
             sql = "#{sql} RETURNING #{quote_column_name(pk)} INTO :returning_id"
             (binds = binds.dup) << ActiveRecord::Relation::QueryAttribute.new("returning_id", nil, Type::OracleEnhanced::Integer.new)
           end
$

[yahonda]

The RuboCop offense reported since RuboCop 1.12.0. 1.11.0 does not report this offense then it is likely due to rubocop/rubocop#9601

[koic]

@yahonda Awesome! Thank you for your investigation and resolution. I have updated this PR.

[yahonda]

Failure against Oracle database 11g at https://travis-ci.com/github/rsim/oracle-enhanced/jobs/494935830 will be taken care of by another pull request.

[yahonda]

I meant to say I'm going to merge this pull request when CI runs successfully against Oracle Database 18c at GitHub Actions.

[yahonda]

Bug report templates failures are expected since it has not merged to master yet.

[yahonda]

Thanks for addressing the showstopper.

[koic]

Thank you too!