rsim / oracle-enhanced

Oracle enhaced adapter for ActiveRecord
MIT License
544 stars 308 forks source link

"Assign auto populated columns on Active Record object creation", breaks the returning of id on insert #2380

Open andynu opened 3 months ago

andynu commented 3 months ago

Overview

The rails commit [1] changes how values are returned from insert statements. The worthy goal is to use the RETURNING statement in postgres to set all the database generated attribute values as a direct result of the insert statement. Oracle supports the RETURNING statement too and may ultimately benefit from this change! As part of this change, the ActiveRecord::Persistence#_create_record now looks for a set of returning_columns instead of assuming that the primary key is being returned.

The challenge with this in the oracle adapter is the way this returning_columns list is built via ActiveRecord::ConnectionAdapters::Column#auto_incremented_by_db? which defaults to a literal false. (or slightly higher up the callstack connection.return_value_after_insert?(c) where c is a Column)

The oracle enhanced adapter does not appear to have knowledge about if a column is a primary key or not on the Column record itself (at least not on that record, or within the sql_type_metadata). It is unclear to me how best to identify which columns are the primary key, and thus should be included in the returning_columns collection.

[1] https://github.com/rails/rails/commit/3421e892afa532a254e54379ac2ce9bef138cf3f

Steps to reproduce

You can see the barest implementation of the parameters for the rails change, and the problem in the tests here: https://github.com/andynu/oracle-enhanced/commit/336d04b588f62f97814142528f8b99d44e32ecf4

Expected behavior

insert should return the id of the new record

Actual behavior

insert returns nil

System configuration

Rails version: not quite 7.1.0 - 3421e892afa532a254e54379ac2ce9bef138cf3f

Oracle enhanced adapter version: a little ways off master, 4707dfa45fcaa73df5399626f752204165016f7b

Ruby version: 3.2.2

Oracle Database version: 23.0.0

andynu commented 3 months ago

This may have been corrected on the rails side.

https://github.com/rails/rails/pull/50783

This fix has not been released yet, and will probably be part of Rails 7.2.0.

But it was back ported to 7.1.0 here: https://github.com/rails/rails/commit/221e609ee8cb1dfb686e43481f3e9496b549e974 :tada: