Fixtures for Secondbase DB error in >= Rails 5.2

[llhhaa]

On upgrade our app to Rails 5.2, all tests started failing on the following error:

Error:
ModelTest::scenario#test_0002_returns true:
Mysql2::Error: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '"TABLE" ("ID", "COL1", "COL2", "COL3", "COL4", "COL5", "CO' at line 1

To isolate the issue away from the app (monkey-patches, bad fixtures), I was able to recreate the issue in a pristine Rails 5.2 app running SecondBase with MySQL and PostgreSQL.

The issue is the same as the one encountered with multiverse: https://github.com/ankane/multiverse/issues/34

As described in the above issue, Rails 5.2 switches to loading fixtures using ActiveRecord::ConnectionAdapters::DatabaseStatements#insert_fixtures_set, which in turn relies on ActiveRecord::ConnectionAdapters::DatabaseStatements#build_fixture_sql. This directly builds a bulk insert statement using Arel::Table, which defaults to using ActiveRecord::Base. The result in our application is that the generated SQL is for Oracle (primary DB) instead of MySQL (Secondbase DB).

I tried to create a Secondbase-owned inheritor of Arel::Table with its engine set to SecondBase::Base, but this critical block of code still produced Oracle SQL, even with this new class:

table = SecondBase::ArelTable.new(table_name) # originally Arel::Table.new(table_name)
manager = Arel::InsertManager.new
manager.into(table)

The only apparent solution then would be to patch build_fixture_sql to dynamically change the engine of Arel::Table depending on which table it's building for. That seems like a suboptimal solution in many ways: high effort, complicated and brittle patch, and potential for unpredictable/subtle bugs. All even less appealing considering Rails 6.0 adds multi-database support.

Instead, I think the solution we'll move forward with was the "temporary" adapter patch I devised to clean up the SQL before insert:

require "active_record/connection_adapters/abstract_mysql_adapter"
require "active_record/connection_adapters/mysql/database_statements"

module ActiveRecord
  module ConnectionAdapters
    module MySQL
      module DatabaseStatements
        # Executes the SQL statement in the context of this connection.
        def execute(sql, name = nil)

          sql = sql.gsub("\"", "`").downcase # THIS IS THE FIX LINE

          # make sure we carry over any changes to ActiveRecord::Base.default_timezone that have been
          # made since we established the connection
          @connection.query_options[:database_timezone] = ActiveRecord::Base.default_timezone

          super
        end
      end
    end

    class Mysql2Adapter < AbstractMysqlAdapter
      include MySQL::DatabaseStatements
    end
  end
end

This cleans up the SQL just before insert. This works for Oracle -> MySQL, but would need to be adapted for other database pairings. In any case, the patch will be cleaned up, restricted to the test environment, etc.

[metaskills]

The only apparent solution then would be to patch build_fixture_sql to dynamically change the engine of Arel::Table depending on which table it's building for...

I actually like this option depending on a few conditions.

1. If the table name check the base tables first. Missing would trigger Secondbase hack.
2. If we could use a Module.prepend to be able to super up to the orig implementation.
3. That there was some singleton we could set for the duration of a block when calling super.

If those were true, then the patch would be minimally invasive. Otherwise agreed, hacking the internals of build_fixture_sql seems very brittle if we had to maintain that across Rails versions >= 5.2.

we'll move forward with was the "temporary" adapter patch

I like the idea of the adapter patch if it were only applied for the dev/test environments. Seems it should not run in staging/prod?

[llhhaa]

After looking at it with @drinks, we ended up with a single-line patch to build_fixture_sql. This line is the only the changed. Below is the patch:

raise "Remove this patch if Secondbase is no longer being used!" if Rails::VERSION::MAJOR > 5

if Rails.version =~ /^5.2/ && (Rails.env.development? || Rails.env.test?)
  module ActiveRecord
    module ConnectionAdapters
      class AbstractAdapter
        def build_fixture_sql(fixtures, table_name)
          # [insert rest of unchanged method here]
          # ...
          #
          # last line:
          # manager.to_sql                                # original
          manager.to_sql OpenStruct.new(connection: self) # fix
        end
      end
    end
  end
end

For now we are committing this patch directly to the affected app, may add it Secondbase for legacy support at a later date if there's a need for it.