Schema dumps turn primary keys that are non-auto-increment or string into auto-increment integers

[neongrau]

I just recently upgraded to Rails 4.2 and using activerecord-sqlserver-adapter 4.2.2 (updated to 4.2.3 today but no changes here).
One issue is with a couple of Models that are using string based primary keys. The primary key column is named "guid" and is holding a UID22 string.
old schema.rb from Rails 3.2 had it this way:

  create_table "connector_feeds", :id => false, :force => true do |t|
    t.string   "guid",        :limit => 22,                    :null => false
    t.string   "name",                                         :null => false
    t.string   "url",                                          :null => false
    t.string   "path"
    t.boolean  "deactivated",               :default => false, :null => false
    t.datetime "created_at",                                   :null => false
    t.datetime "updated_at",                                   :null => false
    t.text     "actors"
  end

while the current version dumps this:

  create_table "connector_feeds", primary_key: "guid", force: :cascade do |t|
    t.string   "name",        limit: 255,                        null: false
    t.string   "url",         limit: 255,                        null: false
    t.string   "path",        limit: 255
    t.boolean  "deactivated",                    default: false, null: false
    t.datetime "created_at",                                     null: false
    t.datetime "updated_at",                                     null: false
    t.text     "actors",      limit: 2147483647
  end

When i try to load fixtures that each have a guid defined it'll end up trying to put a zero as primary key into the db which of course fails on the second record with a identity insert problem. Which happens because the guid column isn't a string but auto-increment integer which at the same time prevents me from inserting the string guid.

sample fixture:

connector_feeds_001:
  guid: aj1c6K_LDFgyNqVq4SGeUi
  name: sz
  url: http://rss.sueddeutsche.de/rss/Topthemen
  path: rss/channel/item
  deactivated: false
  created_at: 2012-06-14 10:46:00.937000000 +02:00
  updated_at: 2012-06-14 10:46:00.937000000 +02:00
  actors: 

[neongrau]

Also with a legacy table that wasn't created by a migration is now being dumped wrong.

Old dump (correnct) was:

create_table "kractorformright", :id => false, :force => true do |t|
  t.integer "lactorid", :null => false
  t.integer "lformid",  :null => false
end

Current dump (wrong) as none of these fields ever was an auto-increment but now the first one was made into one:

create_table "kractorformright", primary_key: "lactorid", force: :cascade do |t|
  t.integer "lformid", limit: 4, null: false
end

[neongrau]

after reading this sound alot like the problem in issue #373 #373 which should be fixed.
But doens't look like it is fixed. Or is this a different problem?

[metaskills]

My first glance is that the schema is now correct when taking into the context that Rails and hence the adapter now supports unique identifiers as primary keys. This leaves 1 of 2 problems. Either your method for inserting a PK guid is not supported and/or your legacy database is not representable via schema Rails schema dumping. There are solutions to each of these problems.

But first, can you tell me what is the SQL type for your guid column?

[neongrau]

Well it was correct with all the adapters for Rails up to 3.2. All fixtures can be loaded into the db that gets created from the old schema.rb.
The guid column is nvarchar(22) when created from the migration:

class CreateConnectorFeeds < ActiveRecord::Migration
  def self.up
    create_table :connector_feeds, { :id => false, :force => true } do |t|
      t.column :guid, :string, { :limit => 22, :null => false }
      t.column "name",        :string,                     :null => false
      t.column "url",         :string,                     :null => false
      t.column "path",        :string
      t.column "deactivated", :boolean, :default => false, :null => false
      t.timestamps
    end
    execute "ALTER TABLE connector_feeds ADD PRIMARY KEY (guid)"
  end

  def self.down
    drop_table :connector_feeds
  end
end

[metaskills]

Well it was correct with all the adapters for Rails up to 3.2.

That depends. Things are different in 4.2 and what was right for 3.2 does not mean the same for 4.2. Especially when we are dealing with behavior that is now standardized by Rails core.

The guid column is nvarchar(22) when created from the migration

Interesting. So this adapter complies with Rails UUID primary keys via SQL Server's uniqueidentifier data type. This means that your connector_feeds table as far as ActiveRecord is concerned just has a normal string primary key. I just wanted to clarify that the name threw me and this is not a true Rails :uuid column type. Which means this should indeed work just like older Rails versions. A few questions/thoughts to try and help:

How are you populating that column via the Model? Using a DB default function or model callback?

[metaskills]

OK, so I did some digging into Rails core tests. They have a test table called movies and another called string_key_objects which looks like this.

create_table :string_key_objects, id: false, primary_key: :id, force: true do |t|
  t.string     :id
  t.string     :name
  t.integer    :lock_version, null: false, default: 0
end

When I test the output of this in our schema dumper test, it looks like this:

create_table "string_key_objects", id: false, force: :cascade do |t|
  t.string  "id",           limit: 4000
  t.string  "name",         limit: 4000
  t.integer "lock_version", limit: 4,    default: 0, null: false
end

If I were to examine id column.

class StringKeyObject < ActiveRecord::Base ; self.table_name = :string_key_objects ; end
StringKeyObject.columns_hash['id']
=> #<ActiveRecord::ConnectionAdapters::SQLServerColumn:0x007ff1153ab5c0
 @cast_type=#<ActiveRecord::ConnectionAdapters::SQLServer::Type::UnicodeVarchar:0x007ff115f801e8 @limit=4000, @precision=nil, @scale=nil>,
 @default=nil,
 @default_function=nil,
 @name="id",
 @null=true,
 @sql_type="nvarchar(4000)",
 @sqlserver_options={:ordinal_position=>1, :is_primary=>false, :is_identity=>false, :default_function=>nil, :table_name=>"string_key_objects"}>

This shows me that Rails or the adapter are not putting the PRIMARY KEY on the end or even if that is right. Then on the flip side, if it were there (as yours is) it would work at all.

[metaskills]

Another note, the default visit_ColumnDefinition and add_column_options! does not add PRIMARY KEY, but I notice the PostgreSQL adapter does via if o.primary_key? && o.type != :primary_key. Maybe we should too and I wonder if all this can test right if we do.

We have this model and schema to make the tests too.

execute "IF EXISTS(SELECT * FROM INFORMATION_SCHEMA.TABLES WHERE TABLE_NAME = 'sst_natural_pk_data') DROP TABLE sst_natural_pk_data"
execute <<-NATURALPKTABLESQL
  CREATE TABLE sst_natural_pk_data(
    parent_id int,
    name nvarchar(255),
    description nvarchar(1000),
    legacy_id nvarchar(10) NOT NULL PRIMARY KEY
  )
NATURALPKTABLESQL

class SSTestNaturalPkData < ActiveRecord::Base
  self.table_name = 'sst_natural_pk_data'
  self.primary_key = 'legacy_id'
end

[neongrau]

THe model is a few years old from times whene there was no builtin support for UID columns on either Rails or the DB.

I'm using this little extension:
# from Demetrio Nunes
# Modified by Andy Singleton to use different GUID generator
# Further modified by Brian Morearty to:
# 1. optionally use the MySQL GUID generator
# 2. respect the "column" option
# 3. set the id before create instead of after initialize
#
# MIT License

#require 'uuid22'
#require 'uuid_mysql'

module Usesguid
  module ActiveRecordExtensions

    #def self.append_features( base )
    def self.included( base )
      super
      base.extend( ClassMethods )
    end


    module ClassMethods

      # guid_generator can be :timestamp or :mysql
      def guid_generator=(generator); class_eval { @guid_generator = generator } end
      def guid_generator; class_eval { @guid_generator || :timestamp } end

      def usesguid(options = {})

        class_eval do
          self.primary_key = options[:column] if options[:column]

          before_create :assign_guid

          # Give this record a guid id.  Public method so people can call it before save if necessary.
          def assign_guid
            self[self.class.primary_key] ||= case ActiveRecord::Base.guid_generator
              when :mysql then UUID.mysql_create(self.connection)
              when :timestamp then UUID.sha1_create(UUID_DNS_NAMESPACE, Time.now.to_f.to_s) #UUID.timestamp_create()
              else raise "Unrecognized guid generator '#{ActiveRecord::Base.guid_generator.to_s}'"
            end.to_s22
          end

        end

      end

    end

  end
end

[metaskills]

OK, this is a bug with Rails. https://github.com/rails/rails/blob/4-2-stable/activerecord/lib/active_record/schema_dumper.rb#L113-L119

# first dump primary key column
pk = @connection.primary_key(table)

tbl.print "  create_table #{remove_prefix_and_suffix(table).inspect}"
pkcol = columns.detect { |c| c.name == pk }
if pkcol
  if pk != 'id'

So it find the pk column. And that column is != or named id.

[metaskills]

One solution would be to remove the PRIMARY KEY at the DB level. But we should fix this in Rails.

[neongrau]

I don't really understand what the problem is with the code part you referenced. I thought the fact that it made it primary key is correct, just that it automatically assumes the type to be auto-increment integer instead of string or whatever the column really is in the db or was created with (regardless of being legacy or created by a migration).

[metaskills]

@neongrau

Right, that setting :primary_key "options" of the create_table method is only for integer types. There is no type information. The only way I know of is when you set primary_key: :uuid as a symbol which infers the name of id and the type of uniqueidentifier. The options to create table to not support natural or legacy primary_key options. What is supposed to happen is the legacy_id in my example is likely supposed to be in the create table block where both the name and type can be explicitly set. We are going to have to check with Rails core on this. cc @sgrif

[sgrif]

Can you check on master? I'm pretty sure we added support for non-standard primary keys in the last month or so.

[Manifold0]

Any progress on this? Just ran into it yesterday in Rails 4.2.1. It isn't a blocking issue, but it does mean we have to run migrations in the test environment instead of loading the schema.