Bug: `ActiveRecord::Base.increment_counter` within a multi-tenant context, the counter column is set to `NULL` instead of being incremented

[pkmuldoon]

Environment

• Rails version: 7.2.x
• activerecord-multi-tenant version: Commit 0ac43aa98334a29ed3e5b33e0bcc5ee281f97254 (broken)
• activerecord-multi-tenant version: v2.4.0 release (works correctly)
• Ruby version: 3.4.2
• PostgreSQL version: 14+

Description

When using ActiveRecord::Base.increment_counter within a multi-tenant context, the counter column is set to NULL instead of being incremented.

IMPORTANT: This is a regression introduced in commit 0ac43aa98334a29ed3e5b33e0bcc5ee281f97254 ("Support rails 7.2"). The released version 2.4.0 works correctly.

Testing Results

✅ v2.4.0 (release) with Rails 7.1: WORKS

UPDATE "users" SET "failed_attempts" = COALESCE("failed_attempts", 0) + 1
WHERE "users"."tenant_id" = 1 AND "users"."id" = 1

❌ Commit 0ac43aa98334a29ed3e5b33e0bcc5ee281f97254 with Rails 7.2: BROKEN (same behavior)

UPDATE "users" SET "failed_attempts" = NULL
WHERE "users"."id" IN (
  SELECT "users"."id" FROM "users"
  WHERE "users"."tenant_id" = 1 AND "users"."id" = 1)
AND "users"."tenant_id" = 1

This indicates that changes made in the Support rails 7.22 PR (#239) introduced a regression.

Expected Behavior

User.increment_counter(:failed_attempts, user.id)
user.reload
# expected: failed_attempts should be 1

Actual Behavior

User.increment_counter(:failed_attempts, user.id)
user.reload
# actual: failed_attempts is NULL

SQL Generated

The generated SQL shows the problem:

UPDATE "users" SET "failed_attempts" = NULL
WHERE "users"."id" IN (
  SELECT "users"."id" FROM "users"
  WHERE "users"."tenant_id" = 1 AND "users"."id" = 1
)
AND "users"."tenant_id" = 1

Instead of the expected:

UPDATE "users" SET "failed_attempts" = COALESCE("failed_attempts", 0) + 1
WHERE "users"."id" = 1 AND "users"."tenant_id" = 1

Root Cause

I think, like in issue 278, this lies within lib/activerecord-multi-tenant/relation_extension.rb, specifically the update_all override:

def updates
  return super if MultiTenant.current_tenant_is_id? || MultiTenant.current_tenant.nil?

  stmt = Arel::UpdateManager.new
  stmt.table(table)
  stmt.set Arel.sql(klass.send(:sanitize_sql_for_assignment, updates))
  stmt.wheres = [generate_in_condition_subquery]

  klass.connection.update(stmt, "#{klass} Update All").tap { reset }
end

When Rails 7.2's increment_counter calls update_all, the sanitize_sql_for_assignment method appears to be mishandling the increment expression, resulting in NULL assignment.

Reproducer

I've created a minimal, self-contained reproducer script that demonstrates this issue. The script:

• Uses bundler/inline so no Gemfile is needed
• Creates its own database tables
• Demonstrates the bug
• Cleans up after itself

See attached reproduce_increment_counter_bug.rb. Please note, depending on your database setup, you may need to set the environment variables:

PGHOST, PGUSER, PGPASSWORD.

### Running the reproducer:

```bash
# Create test database
createdb multi_tenant_test

# Run the script
ruby reproduce_increment_counter_bug.rb

# Cleanup (optional)
dropdb multi_tenant_test

Expected output showing the bug

=== Setting up test data ===
Created user with failed_attempts: 0

=== Attempting to increment counter with increment_counter ===
UPDATE "users" SET "failed_attempts" = NULL WHERE ...
After increment_counter, failed_attempts: nil

❌ BUG REPRODUCED: failed_attempts is NULL instead of 1

I've included within the gemfile do a version showing how it used to work. To reproduce a working version, remove the comments from lines 32-34 and comment out lines 36-38. It sets the reproducer to Rails 7.1 and activerecord_multi_tenant v2.4.0 to show how it should work.

Impact

This breaks critical functionality that relies on counter increments:

• Devise's lockable module (failed login attempts)
• Any counter caches
• Custom counter implementations

Regression Analysis

The regression was introduced between:

• Working: v2.4.0 (released version)
• Broken: Commit 0ac43aa98334a29ed3e5b33e0bcc5ee281f97254 (PR #239: "Support rails 7.22")

Related PR

PR #239 The changes made for Rails 7.2 support appear to have broken increment_counter behavior on Rails 7.1 and 7.2.

Impact

This regression blocks users from:

• Using the Rails 7.2 support added in PR #239
• Using commit 0ac43aa98334a29ed3e5b33e0bcc5ee281f97254 or any commits after it
• Applications using Devise's lockable module will see authentication failures
• Any application using counter caches will experience data corruption (NULL values)

[pkmuldoon]

Reproducer:

# Reproducer for activerecord-multi-tenant increment_counter regression
#
# This demonstrates a REGRESSION introduced in commit 0ac43aa9 (PR #239: Rails 7.2 support)
# where increment_counter sets columns to NULL instead of incrementing them.
#
# REGRESSION ANALYSIS:
#   ✅ v2.4.0 (release) + Rails 7.1: WORKS
#   ❌ commit 0ac43aa98334a29ed3e5b33e0bcc5ee281f97254 + Rails 7.2: BROKEN
#
# Setup:
#   createdb multi_tenant_test
#
# To run:
#   ruby reproduce_increment_counter_bug.rb
#
# To test with the WORKING version (v2.4.0):
#   Uncomment line 15-17 nd comment out lines 19-21
#
# Environment:
#   Rails 7.2.x
#   activerecord-multi-tenant commit 0ac43aa9 (BROKEN)
#   PostgreSQL 10+

require "bundler/inline"

gemfile do
  source "https://rubygems.org"
  # gem "rails", "~> 7.1.0"
  # gem "pg"
  # gem "activerecord-multi-tenant", "~> 2.4.0", require: false

  gem "rails", "~> 7.2.0"
  gem "pg"
  gem "activerecord-multi-tenant", github: "citusdata/activerecord-multi-tenant", ref: "0ac43aa98334a29ed3e5b33e0bcc5ee281f97254", require: false
end

require "active_record"
require "active_record/connection_adapters/postgresql_adapter"
require "logger"

# Set up database connection BEFORE loading activerecord-multi-tenant
# Configure these environment variables if needed:
#   PGUSER - your PostgreSQL username (default: current user)
#   PGPASSWORD - your PostgreSQL password (default: none)
#   PGHOST - PostgreSQL host (default: localhost)
ActiveRecord::Base.logger = Logger.new(STDOUT)
ActiveRecord::Base.establish_connection(
  adapter: "postgresql",
  database: "multi_tenant_test",
  host: ENV["PGHOST"] || "localhost",
  username: ENV["PGUSER"] || ENV["USER"],
  password: ENV["PGPASSWORD"],
  pool: 5
)

# Now load activerecord-multi-tenant after ActiveRecord is fully initialized
require "activerecord-multi-tenant"

# Create schema
ActiveRecord::Schema.define do
  create_table :tenants, force: true do |t|
    t.string :name
  end

  create_table :users, force: true do |t|
    t.string :email
    t.integer :tenant_id
    t.integer :failed_attempts, default: 0
    t.timestamps
  end
end

# Define models
class Tenant < ActiveRecord::Base
end

class User < ActiveRecord::Base
  multi_tenant :tenant
end

# Reproduce the bug
puts "\n=== Setting up test data ==="
tenant = Tenant.create!(name: "Test Tenant")
user = nil

MultiTenant.with(tenant) do
  user = User.create!(email: "[email protected]", tenant: tenant, failed_attempts: 0)
  puts "Created user with failed_attempts: #{user.failed_attempts}"
end

puts "\n=== Attempting to increment counter with increment_counter ==="
MultiTenant.with(tenant) do
  User.increment_counter(:failed_attempts, user.id)
  user.reload
  puts "After increment_counter, failed_attempts: #{user.failed_attempts.inspect}"

  if user.failed_attempts.nil?
    puts "\n❌ BUG REPRODUCED: failed_attempts is NULL instead of 1"
    puts "Expected: failed_attempts should be 1"
    puts "Actual: failed_attempts is NULL"
  else
    puts "\n✅ No bug: failed_attempts correctly incremented to #{user.failed_attempts}"
  end
end

# Clean up
ActiveRecord::Schema.define do
  drop_table :users, if_exists: true
  drop_table :tenants, if_exists: true
end

puts "\n=== Test complete ==="