DatabaseStatements#affected_rows returning nil raising exception in ActiveRecord when comparing with 0

[taylorthurlow]

I upgraded to Rails 8.0 and activerecord-sqlserver-adapter 8.0.2 (w/ tiny_tds 3.2.0), and started getting a test failure:

NoMethodError:
       undefined method `>' for nil

The test fails at a point where I am deleting a single record from the database.

The exception is being raised from within ActiveRecord: https://github.com/rails/rails/blob/800976975253be2912d09a80757ee70a2bb1e984/activerecord/lib/active_record/counter_cache.rb#L236-L238

The return value of super which when you tug on the method call chain long enough you find is the return value of ActiveRecord::ConnectionAdapters::SQLServer::DatabaseStatements#affected_rows:

activerecord-sqlserver-adapter/lib/active_record/connection_adapters/sqlserver/database_statements.rb

Lines 41 to 43 in 8afbccc

	def affected_rows(raw_result)
	raw_result.first['AffectedRows']
	end

Dropping a debugger in here and inspecting raw_result:

irb:rdbg(#<ActiveRecord::ConnectionAda...):006> raw_result
#<ActiveRecord::Result:0x0000000157dd2c70
 @column_indexes={"affectedrows"=>0},
 @column_types={},
 @columns=["affectedrows"],
 @hash_rows=[{"affectedrows"=>1}],
 @rows=[[1]]>

The issue is the capitalization (or lack thereof) the AffectedRows key. Because the query result key is lower case, and this gem is looking under the key AffectedRows, the return value is nil and then that bubbles up into ActiveRecord internally where it chokes.

There are recent changes in #1268 but it does look like it wasn't necessarily unexpected for this method to return nil before, though after this change it does seem that the intent is to allow nil. I haven't speculated any further on what the cause of the issue is. (@aidanharan hope you don't mind the ping here, you are the author on that PR and would probably be the person to ask about this specific issue).

[aidanharan]

Would you be able to recreate the issue using a bug script? See https://github.com/rails-sqlserver/activerecord-sqlserver-adapter/wiki/How-to-report-a-bug

[aidanharan]

@taylorthurlow What is the collation of your database? You can find it by running:

SELECT collation_name
FROM sys.databases
WHERE name = 'your_database_name';

Also, what column names do you get if you run the following:

SELECT 1 AS AffectedRows;

My output is the following btw.

[taylorthurlow]

@aidanharan Thanks for looking into this! My database's collation is also SQL_Latin1_General_CP1_CI_AS.

Sorry that I didn't see the report template. I'll put one together today.

EDIT: Ok first pass at a reproduction script and I'm not having the issue. Spending some time trying to figure out what's causing the difference.

[taylorthurlow]

Ok I found the issue. The active record wrapper gem we use for this specific database has this additional configuration:

ActiveRecord::ConnectionAdapters::SQLServerAdapter.lowercase_schema_reflection = true

Here's the repro which replicates the issue:

require "bundler/inline"

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

  gem "rails", "8.0.1"
  gem "activerecord", "8.0.1"
  gem "activerecord-sqlserver-adapter", "8.0.2"
end

require "active_record"
require "minitest/autorun"
require "logger"

adapter = ActiveRecord::ConnectionAdapters::SQLServerAdapter
adapter.lowercase_schema_reflection = true

database_name = "sqlserver_adapter_bug_repro"
ActiveRecord::Base.establish_connection(
  adapter:  "sqlserver",
  encoding: "utf8",
  username: "SA",
  password: "password",
  host:     "localhost",
  port:     1433,
)
ActiveRecord::Base.logger = Logger.new(STDOUT)

begin
  ActiveRecord::Base.connection.drop_database(database_name)
rescue ActiveRecord::StatementInvalid
  puts "Database does not exist, skipping drop"
end

ActiveRecord::Base.connection.create_database(database_name)

ActiveRecord::Base.establish_connection(
  adapter:  "sqlserver",
  encoding: "utf8",
  database: database_name,
  username: "SA",
  password: "password",
  host:     "localhost",
  port:     1433,
)

ActiveRecord::Schema.define do
  drop_table :bug_tests rescue nil

  create_table :bug_tests, force: true do |t|
    t.bigint :external_id
  end
end

class BugTest < ActiveRecord::Base
end

class TestBugTest < Minitest::Test
  def test_count
    bug_test = BugTest.create!(external_id: 2_032_070_100_001)

    assert_equal 1, BugTest.count

    bug_test.destroy!

    assert_equal 0, BugTest.count
  end
end

Disabling the lowercase schema reflection setting causes the test to pass.

[aidanharan]

Fixed by #1306