jashmenn / activeuuid

Binary uuid keys in Rails
MIT License
340 stars 124 forks source link

Doesn't work with add_column/change_column (Rails 4.2.0, mysql2) #56

Open kevin-at-reflexion opened 9 years ago

kevin-at-reflexion commented 9 years ago

The add_column call doesn't work starting with Rails 4.2.0. I am using the mysql2 adapter -- I've not test the issue with other adapters, and using the master branch after the merge of pull request #52 (4.2 support).

The resulting MySQL expression for add_column :table_name, :column_name, :uuid is:

ALTER TABLE `table_name` ADD `column_name` uuid

Similarily, for a change_column

ALTER TABLE `table_name` CHANGE `column_name` `column_name` uuid

This naturally results in a Mysql2::Error: You have an error in your SQL syntax near 'uuid'.

I went hunting through ActiveRecord for the appropriate change, but couldn't find the right place, apologies.

kevin-at-reflexion commented 9 years ago

Clarification, I've just tested this out on a fresh project. The behavior works for a sqlite, but switching a fresh project to mysql2 the project still persists.

kevin-at-reflexion commented 9 years ago

This appears to be caused by Rails occasionally not loading the connection adapter before apply! is called.

I recommend something like the following:

def self.load_adapters(*adapters)
  adapters.each do |adapter|
    begin
      require "active_record/connection_adapters/#{adapter}_adapter"
    rescue LoadError
    end
  end
end

def self.apply!
  self.load_adapters 'mysql2', 'sqlite3', 'postgresql'

  ActiveRecord::ConnectionAdapters::Table.send :include, Migrations if defined? ActiveRecord::ConnectionAdapters::Table
  ActiveRecord::ConnectionAdapters::TableDefinition.send :include, Migrations if defined? ActiveRecord::ConnectionAdapters::TableDefinition

  if ActiveRecord::VERSION::MAJOR == 4 and ActiveRecord::VERSION::MINOR == 2
    ActiveRecord::ConnectionAdapters::Mysql2Adapter.send :include, AbstractAdapter if defined? ActiveRecord::ConnectionAdapters::Mysql2Adapter
    ActiveRecord::ConnectionAdapters::SQLite3Adapter.send :include, AbstractAdapter if defined? ActiveRecord::ConnectionAdapters::SQLite3Adapter
  else
    ActiveRecord::ConnectionAdapters::Column.send :include, Column
    ActiveRecord::ConnectionAdapters::PostgreSQLColumn.send :include, PostgreSQLColumn if defined? ActiveRecord::ConnectionAdapters::PostgreSQLColumn
  end

  ActiveRecord::ConnectionAdapters::Mysql2Adapter.send :include, Quoting if defined? ActiveRecord::ConnectionAdapters::Mysql2Adapter
  ActiveRecord::ConnectionAdapters::SQLite3Adapter.send :include, Quoting if defined? ActiveRecord::ConnectionAdapters::SQLite3Adapter
  ActiveRecord::ConnectionAdapters::PostgreSQLAdapter.send :include, PostgreSQLQuoting if defined? ActiveRecord::ConnectionAdapters::PostgreSQLAdapter
end

In this case, the require will throw LoadError if the appropriate gem (mysql2, sqlite3, postgresql) is not also required (typically via Bundler.require in a rails config/application.rb)

tony-currentuser commented 9 years ago

I have a similar problem but in context of rails server (while @kevin-at-reflexion encountered above the problem in the context of a rake task).

The application is here: https://github.com/currentuser/currentuser-example-blog-rails

The problem arise when trying to create a new Post record, using this code (in the highlighted lines, currentuser_id is an uuid).

I used the following workaround to make the application work:

# config/application.rb

# Require mysql2_adapter just after rails is loaded to fix
# https://github.com/jashmenn/activeuuid/issues/56
require 'rails/all'
require 'active_record/connection_adapters/mysql2_adapter'

You can see the workaround here.

Confusion commented 8 years ago

@tony-currentuser Thanks for that solution. After upgrading from Rails 4.1 to 4.2, I ran into errors saying

Mysql2::Error: Field 'id' doesn't have a default value

I traced the cause to uuid.rb#generate_uuids_if_needed, where the column type was returned as being :binary instead of :uuid, so no uuid was being generated.

The solution of requiring the myqsl2_adapter immediately after rails/all solved the issue.

inbeom commented 8 years ago

@tony-currentuser @Confusion Would you try using PR #85 instead? It does patch applications immediately after connections are established, so the load order won't be the problem.