Nil variable @version_year is causing error when Rails try to load schema on the test database

[theo-bittencourt]

Issue

Cannot setup test db properly because db:schema:load is raising NoMethodError: undefined method <' for nil:NilClass`.

In the method ConnectionAdapters::SQLServer::SchemaStatements.drop_table, the variable @version_year is not defined yet at this point.

Expected behavior

Should load schema on test db.

Actual behavior

rails db:schema:load RAILS_ENV=test or rails db:test:prepare are raising NoMethodError: undefined method <' for nil:NilClass`.

How to reproduce

Run rails db:schema:load RAILS_ENV=test or rails db:test:prepare.

Details

• Rails version: 6.1.4.1
• SQL Server adapter version: 6.1.2.1
• TinyTDS version: 2.1.5
• FreeTDS details:

Compile-time settings (established with the "configure" script)
                            Version: freetds v1.3.2
             freetds.conf directory: /usr/local/etc
     MS db-lib source compatibility: no
        Sybase binary compatibility: yes
                      Thread safety: yes
                      iconv library: yes
                        TDS version: 7.3
                              iODBC: no
                           unixodbc: yes
              SSPI "trusted" logins: no
                           Kerberos: yes
                            OpenSSL: yes
                             GnuTLS: no
                               MARS: yes

Stacktrace

NoMethodError: undefined method `<' for nil:NilClass
.../ruby/3.0.2/lib/ruby/gems/3.0.0/gems/activerecord-sqlserver-adapter-6.1.2.1/lib/active_record/connection_adapters/sqlserver/schema_statements.rb:29:in `drop_table'
.../ruby/3.0.2/lib/ruby/gems/3.0.0/gems/activerecord-6.1.4.1/lib/active_record/connection_adapters/abstract/schema_statements.rb:317:in `create_table'
.../ruby/3.0.2/lib/ruby/gems/3.0.0/gems/activerecord-sqlserver-adapter-6.1.2.1/lib/active_record/connection_adapters/sqlserver/schema_statements.rb:12:in `create_table'
.../ruby/3.0.2/lib/ruby/gems/3.0.0/gems/activerecord-6.1.4.1/lib/active_record/migration.rb:929:in `block in method_missing'
.../ruby/3.0.2/lib/ruby/gems/3.0.0/gems/activerecord-6.1.4.1/lib/active_record/migration.rb:897:in `block in say_with_time'
.../ruby/3.0.2/lib/ruby/3.0.0/benchmark.rb:293:in `measure'
.../ruby/3.0.2/lib/ruby/gems/3.0.0/gems/activerecord-6.1.4.1/lib/active_record/migration.rb:897:in `say_with_time'
.../ruby/3.0.2/lib/ruby/gems/3.0.0/gems/activerecord-6.1.4.1/lib/active_record/migration.rb:918:in `method_missing'
.../db/schema.rb:15:in `block in <main>'
.../ruby/3.0.2/lib/ruby/gems/3.0.0/gems/activerecord-6.1.4.1/lib/active_record/schema.rb:50:in `instance_eval'
.../ruby/3.0.2/lib/ruby/gems/3.0.0/gems/activerecord-6.1.4.1/lib/active_record/schema.rb:50:in `define'
.../ruby/3.0.2/lib/ruby/gems/3.0.0/gems/activerecord-6.1.4.1/lib/active_record/schema.rb:46:in `define'

[aidanharan]

Unable to recreate the issue. Could you supply a test or sample Rails app that demonstrates the problem?

[wpolicarpo]

@theo-bittencourt do you override the configure_connection method in your application like documentation suggests?

If so, I'm pretty sure doc is wrong and we shouldn't do what is suggested. We should provide another public method as a hook point so users can execute what they need on an initializer.

[theo-bittencourt]

@wpolicarpo Yes, we do!

Removing that I'm facing another error now. The relevant backtrace:

ActiveRecord::StatementInvalid: TinyTds::Error: Incorrect syntax near the keyword 'COLLATE'.
.../activerecord-sqlserver-adapter-6.1.2.1/lib/active_record/connection_adapters/sqlserver/database_statements.rb:361:in `do'
.../activerecord-sqlserver-adapter-6.1.2.1/lib/active_record/connection_adapters/sqlserver/database_statements.rb:361:in `raw_connection_do'
.../activerecord-sqlserver-adapter-6.1.2.1/lib/active_record/connection_adapters/sqlserver/database_statements.rb:298:in `block in do_execute'

[theo-bittencourt]

@aidanharan

As observed by @wpolicarpo, seems like overriding the configure_connection method may affect something. If you wish to try to reproduce again with an initiliazer, our is more or less as below:

# config/initializers/sqlserver_adapter.rb

module ActiveRecord
  module ConnectionAdapters
    class SQLServerAdapter < AbstractAdapter
      def configure_connection
        raw_connection_do "SET TEXTSIZE #{16.megabytes}"
      end
    end
  end
end

[wpolicarpo]

Perfect, I'll look into it.

For now you can just copy the body configure_connection and add anything else you need to your initializer.

Something like this:

ActiveSupport.on_load(:active_record) do
  module ActiveRecord
    module ConnectionAdapters
      class SQLServerAdapter < AbstractAdapter
        def configure_connection
          # Original implementation
          @spid = _raw_select("SELECT @@SPID", fetch: :rows).first.first
          @version_year = version_year
  
          initialize_dateformatter
          use_database

          # Any custom code you may have like README suggests
          raw_connection_do "SET TEXTSIZE #{64.megabytes}"
        end
      end
    end
  end
end

[theo-bittencourt]

Receiving now the same error from the early comment:

ActiveRecord::StatementInvalid: TinyTds::Error: Incorrect syntax near the keyword 'COLLATE'.

Raising from database_statements.rb:361:in `do'

[wpolicarpo]

Do you have multiple databases? Can you maybe post your initializer or create a rails application that simulates the error?

[theo-bittencourt]

I could execute db:test:prepare without errors by removing the option collation from schema.rb .

from
t.string "foo", limit: 100, collation: "Latin1_General_CI_AI"

to t.string "foo", limit: 100

[theo-bittencourt]

@wpolicarpo

We're not connecting with two different database at the same time. But we did a migration from one legacy database, which collation is defined as Latin1_General_CI_AI.

So, we end up changing the database.yml to define the default collation as Latin1_General_CI_AI instead SQL_Latin1_General_CP1_CI_AS (used by this gem).

This fixed our problem, although the TinyTds::Error will continue affecting other cases.

Thanks for the support guys! Feel free to close the issue.

[owst]

@aidanharan can this be reopened please?

Unless I'm mistaken the underlying issue hasn't been resolved - as per @wpolicarpo https://github.com/rails-sqlserver/activerecord-sqlserver-adapter/issues/950#issuecomment-943453365 , specifically:

I'm pretty sure doc is wrong and we shouldn't do what is suggested. We should provide another public method as a hook point so users can execute what they need on an initializer.

It looks like #917 introduced the issue, whereby overriding configure_connection (as suggested in the README) causes some of the default configuration to no longer be performed (e.g. initialize_dateformatter)

We resolved this issue by prepending a custom module that also called super, along the lines of:

module CustomSqlServerConfig
  def configure_connection
    # custom config here...
    super
  end
end

ActiveRecord::ConnectionAdapters::SQLServerAdapter.prepend(CustomSqlServerConfig)

but I think @wpolicarpo's suggestion would be better.

An example reproduction, place in repro.rb and run with

DB_USERNAME=... DB_PASSWORD=... DB_NAME=... DB_HOST=... ruby repro.rb

begin
  require "bundler/inline"
rescue LoadError => e
  $stderr.puts "Bundler version 1.10 or later is required. Please update your Bundler"
  raise e
end

gemfile(true) do
  source "https://rubygems.org"

  # Working versions
  # gem "activerecord", "6.0.6.1"
  # gem "activerecord-sqlserver-adapter", "6.0.2"

  gem "activerecord", "6.1.7.2"
  gem "activerecord-sqlserver-adapter", "6.1.2.1"
end

require "active_record"
require "logger"

module ActiveRecord
  module ConnectionAdapters
    class SQLServerAdapter < AbstractAdapter
      def configure_connection
        raw_connection_do "SET LANGUAGE British;"
      end
    end
  end
end

ActiveRecord::Base.establish_connection(
  adapter: "sqlserver",
  database: ENV["DB_NAME"],
  host: ENV["DB_HOST"],
  username: ENV["DB_USERNAME"],
  password: ENV["DB_PASSWORD"]
)
ActiveRecord::Base.logger = Logger.new(STDOUT)

ActiveRecord::Schema.define do
  drop_table :things if ActiveRecord::Base.connection.table_exists? :things

  create_table :things do |t|
    t.datetime :happened_at
  end
end

class ApplicationRecord < ActiveRecord::Base
  self.abstract_class = true
end

class Thing < ApplicationRecord
end

Time.zone = 'Europe/London'
now = Time.zone.parse("2023-03-06 16:30:40.123456 +00:00")

Thing.create!(happened_at: now)

p Thing.first

with 6.0.2 no exceptions are raised but with 6.1.2.1 an exception is raised:

...
	 4: from /Users/owenstephens/.rbenv/versions/2.7.6/lib/ruby/gems/2.7.0/gems/activerecord-sqlserver-adapter-6.1.2.1/lib/active_record/connection_adapters/sqlserver/database_statements.rb:412:in `block in raw_select'
	 3: from /Users/owenstephens/.rbenv/versions/2.7.6/lib/ruby/gems/2.7.0/gems/activerecord-sqlserver-adapter-6.1.2.1/lib/active_record/connection_adapters/sqlserver/database_statements.rb:417:in `_raw_select'
	 2: from /Users/owenstephens/.rbenv/versions/2.7.6/lib/ruby/gems/2.7.0/gems/activerecord-sqlserver-adapter-6.1.2.1/lib/active_record/connection_adapters/sqlserver/database_statements.rb:438:in `handle_to_names_and_values'
	 1: from /Users/owenstephens/.rbenv/versions/2.7.6/lib/ruby/gems/2.7.0/gems/activerecord-sqlserver-adapter-6.1.2.1/lib/active_record/connection_adapters/sqlserver/database_statements.rb:447:in `handle_to_names_and_values_dblib'
/Users/owenstephens/.rbenv/versions/2.7.6/lib/ruby/gems/2.7.0/gems/activerecord-sqlserver-adapter-6.1.2.1/lib/active_record/connection_adapters/sqlserver/database_statements.rb:447:in `each': TinyTds::Error: Error converting data type varchar to datetime. (ActiveRecord::StatementInvalid)

which is due to initialize_dateformatter not having been called

[aidanharan]

Fixed in release v6.1.3.0