Bug when joining two tables with a common column name

Hi had an issue and was about to report it on GitHub and see the reference to the here and I’m grateful because I’ve created an issue and a fix. Although since I’m not familiar with the codebase it’s probably a good option to start here a discussion.

The issue happens, when we have a query and a join between tables with a common column name. I will post here the example that I hope could bring clarity to the issue:

require "bundler/inline"

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

  git_source(:github) { |repo| "https://github.com/#{repo}.git" }

  gem "rails", github: "rails/rails", branch: "main"
  gem "sqlite3"
end

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

# This connection will do for database-independent bug reports.
ActiveRecord::Base.establish_connection(adapter: "sqlite3", database: ":memory:")
ActiveRecord::Base.logger = Logger.new(STDOUT)

ActiveRecord::Schema.define do

  create_table :events, force: true do |t|
  end

  create_table :tickets, force: true do |t|
    t.integer :event_id
    t.decimal :price, default: 0
    t.boolean :refunded, default: false
  end

  create_table :registrations, force: true do |t|
    t.integer :event_id
    t.integer :ticket_id
    t.decimal :price, default: 0
    t.string :source
  end
end


class Event < ActiveRecord::Base
  has_many :tickets, inverse_of: :event, dependent: :destroy

  def free_registrations
    tickets.free.joins(:registrations).count
  end


  def paid_registrations
    tickets.paid.joins(:registrations).count
  end
end

class Ticket < ActiveRecord::Base
  has_many :registrations
  belongs_to :event

  scope :paid, -> { where('price > 0') }
  scope :free, -> { where('price= 0') }
end

class Registration < ActiveRecord::Base
  belongs_to :event
  belongs_to :ticket 
end

class BugTest < Minitest::Test
  def test_association_stuff
    event = Event.create!
    free = Ticket.create(event: event)
    vip = Ticket.create(event: event, price: 1000)
    Registration.create(event: event, ticket: free)
    Registration.create(event: event, ticket: vip)

    assert_equal 1,event.paid_registrations 
    assert_equal 1,event.free_registrations
  end
end

This will led to ActiveRecord::StatementInvalid: SQLite3::SQLException: ambiguous column name: price

Given that error, I will need to simply add the table name to the query.

I’ve been digging into the code and come with a solution:

diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb
index c9da474824..7b401fe019 100644
--- a/activerecord/lib/active_record/relation/query_methods.rb
+++ b/activerecord/lib/active_record/relation/query_methods.rb
@@ -1185,6 +1185,7 @@ def build_subquery(subquery_alias, select_value) # :nodoc:
 
       def build_where_clause(opts, rest = []) # :nodoc:
         opts = sanitize_forbidden_attributes(opts)
+        opts = transform_comparison_clause(opts)
 
         case opts
         when String, Array
@@ -1211,6 +1212,30 @@ def build_where_clause(opts, rest = []) # :nodoc:
       alias :build_having_clause :build_where_clause
 
     private
+      def transform_comparison_clause(clause)
+        return clause unless clause.kind_of?(String)
+
+        # TODO handle cases with or/and/not
+        return clause if clause.match?(/and|or|not/i)
+        case clause
+        when />=/
+          attribute, value = clause.split(/>=/).each(&:strip!)
+          table[attribute].gteq(value)
+        when /<=/
+          attribute, value = clause.split(/<=/).each(&:strip!)
+          table[attribute].lteq(value)
+        when /=/
+          attribute, value = clause.split(/=/).each(&:strip!)
+          table[attribute].eq(value)
+        when />/
+          attribute, value = clause.split(/>/).each(&:strip!)
+          table[attribute].gt(value)
+        when /</
+          attribute, value = clause.split(/</).each(&:strip!)
+          table[attribute].lt(value)
+        end
+      end
+
       def lookup_table_klass_from_join_dependencies(table_name)
         each_join_dependencies do |join|
           return join.base_klass if table_name == join.table_name

This proposed solution works, but I doubt it’s the best implementation or the right way to implement the desired behaviour, so this thread is open to collect feedback.

I wouldn’t say that this is a bug – when you write the where conditions in a string like you did, it gets inserted directly into the generated SQL statement as is. Try this:

scope :paid, -> { where(price: 0..) }
scope :free, -> { where(price: 0) }

and AR will take care of the rest for you.

1 Like