Bug when joining two tables with a common column name

rubyonrails-core
1

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.

2

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.

3 Likes