Always set id from database when creating new record

masa331 · August 21, 2015, 9:07am

Hi guys,

this is code from ActiveRecord::Persistance:

def _create_record(attribute_names = self.attribute_names) attributes_values = arel_attributes_with_values_for_create(attribute_names)

new_id = self.class.unscoped.insert attributes_values self.id ||= new_id if self.class.primary_key

@new_record = false id end

``

I’m curious about the rational behind line no. 5. Why we set self.id to value returned from db only if it was previously empty? I couldn’t find any test nor some discussion. Also the code dates back to 2011.

I work with some legacy db where primary key is string which is modified in some database trigger. Because the self.id is set before save it isn’t assigned to proper value returned from db after trigger modification.

I can’t think of any problem if we always assign id to value returned from database.

I would love to create PR if it’s ok

thanks!

Sledujte náš blog.uol.cz, facebook.com/uol.cz, gplus.to/uolcz

Kevin_Deisz · August 21, 2015, 3:31pm

Wouldn’t doing so disallow you from manually setting the ID? I know it’s a weird case but we do this in our product in a table with string IDs.

masa331 · August 21, 2015, 6:46pm

I don’t think it would. You set the id in model, it’s passed to database, and then database returns it back again

Dne pátek 21. srpna 2015 17:26:49 UTC+2 masa331 napsal(a):

Matt_Jones · August 24, 2015, 8:23pm

I don't think it would. You set the id in model, it's passed to database, and then database returns it back again

Some of the database adapters require an additional round-trip to retrieve the newly-inserted ID; this doesn't happen if an id was supplied at record creation. Others (like MySQL) will fall back on the DB's built in "last inserted ID" mechanism, which doesn't work if you are generating IDs manually (`mysql_insert_id`, for instance, only gets a value if the id column was AUTO_INCREMENT).

For more detail, keep tracing from where you quoted. `_create_record` calls `ActiveRecord::Relation#insert`, here:

github.com

rails/rails/blob/337684fa28a3e8d55874d5740585710d0fa99ead/activerecord/lib/active_record/relation.rb#L41


      
            @predicate_builder = predicate_builder
          end
          
          def initialize_copy(other)
            # This method is a hot spot, so for now, use Hash[] to dup the hash.
            #   https://bugs.ruby-lang.org/issues/7166
            @values        = Hash[@values]
            reset
          end
          
          def insert(values) # :nodoc:
            primary_key_value = nil
          
            if primary_key && Hash === values
              primary_key_value = values[values.keys.find { |k|
                k.name == primary_key
              }]
          
              if !primary_key_value && connection.prefetch_primary_key?(klass.table_name)
                primary_key_value = connection.next_sequence_value(klass.sequence_name)
                values[klass.arel_table[klass.primary_key]] = primary_key_value

which does two things:

* if the primary key is empty and the adapter wants to use prefetch, it generates an ID

* explicitly selects the supplied (or generated) primary key value and passes it as a separate argument to the `insert` method in the connection adapter.

The default implementation of the connection adapter's `insert` method:

github.com

rails/rails/blob/337684fa28a3e8d55874d5740585710d0fa99ead/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb#L107


      
          end
          
          # Returns the last auto-generated ID from the affected table.
          #
          # +id_value+ will be returned unless the value is nil, in
          # which case the database will attempt to calculate the last inserted
          # id and return that value.
          #
          # If the next id was calculated in advance (as in Oracle), it should be
          # passed in as +id_value+.
          def insert(arel, name = nil, pk = nil, id_value = nil, sequence_name = nil, binds = [])
            sql, binds = sql_for_insert(to_sql(arel, binds), pk, id_value, sequence_name, binds)
            value      = exec_insert(sql, name, binds, pk, sequence_name)
            id_value || last_inserted_id(value)
          end
          
          # Executes the update statement and returns the number of rows affected.
          def update(arel, name = nil, binds = [])
            exec_update(to_sql(arel, binds), name, binds)
          end

will automatically return the value passed in as `id_value`, if present. The MySQL and Postgres versions appear to do this as well.

A workaround for your case would be database-specific; for instance, on MySQL I'd recommend that you ensure the trigger sets `LAST_INSERT_ID` and then extract the value from the DB directly (via `SELECT LAST_INSERT_ID()`) in an `after_create`. Unfortunately, the ID is the one field that the standard, DB-independent approach to dealing with trigger-supplied data (calling `reload` after saving) won't work for.

--Matt Jones

Dne pátek 21. srpna 2015 17:26:49 UTC+2 masa331 napsal(a):