Merge pull request rsim#2735 from yahonda/mirror-pk-fallback-in-sql-for-insert

Mirror AbstractAdapter PK inference fallback in sql_for_insert

Author: yahonda

lib/active_record/connection_adapters/oracle_enhanced/database_statements.rb

@@ -275,15 +275,20 @@ def affected_rows(result)
           end
 
           def sql_for_insert(sql, pk, binds, returning) # :nodoc:
+            table_ref = extract_table_ref_from_insert_sql(sql)
+            # Mirror AbstractAdapter#sql_for_insert: when caller passes pk: nil,
+            # infer the primary key from the SQL via the schema cache so that
+            # generic `connection.exec_insert(sql, name, binds)` callers also
+            # benefit from RETURNING auto-fetch.
+            pk = schema_cache.primary_keys(table_ref) if pk.nil? && table_ref
             cols = columns_for_returning_clause(sql, pk, binds, returning)
             unless cols.empty?
-              table_name = extract_table_ref_from_insert_sql(sql)
               quoted_cols = cols.map { |c| quote_column_name(c) }.join(", ")
               placeholders = cols.map { |c| ":returning_#{c}" }.join(", ")
               sql = "#{sql} RETURNING #{quoted_cols} INTO #{placeholders}"
               binds = binds.dup
               cols.each do |col|
-                column = table_name ? columns(table_name).find { |c| c.name == col } : nil
+                column = table_ref ? columns(table_ref).find { |c| c.name == col } : nil
                 type = column&.cast_type || Type::OracleEnhanced::Integer.new
                 binds << ActiveRecord::Relation::QueryAttribute.new("returning_#{col}", nil, type)
               end

spec/active_record/connection_adapters/oracle_enhanced_adapter_spec.rb

@@ -165,6 +165,19 @@ class ::TestReturningIdentityItem < ActiveRecord::Base
         expect(insert_log).to match(/RETURNING\s+"ID"\s+INTO\s+:returning_id/i)
         expect(insert_log).not_to match(/RETURNING\s+"ID"\s*\)?\s*\z/i)
       end
+
+      # AbstractAdapter#sql_for_insert infers the primary key from the SQL when
+      # the caller passes pk: nil. Generic callers that go through this path
+      # without an explicit `pk` used to miss out on RETURNING auto-fetch on
+      # Oracle because the fallback was not mirrored locally; this spec locks
+      # in the parity introduced for issue #2732.
+      it "infers the primary key from the SQL when pk: nil (parity with AbstractAdapter)" do
+        conn = ActiveRecord::Base.lease_connection
+        insert_sql = "INSERT INTO #{conn.quote_table_name('test_returning_identity_items')} (#{conn.quote_column_name('name')}) VALUES ('direct-call')"
+        out_sql, out_binds = conn.send(:sql_for_insert, insert_sql, nil, [], nil)
+        expect(out_sql).to match(/RETURNING\s+"ID"\s+INTO\s+:returning_id/i)
+        expect(out_binds.last.name).to eq("returning_id")
+      end
     end
 
     # Sequence-prefetched path: oracle-enhanced's default for `create_table` (no