EugZol / where_exists

Adds the power of SQL Exists to ActiveRecord
MIT License
110 stars 18 forks source link

Association condition seems to be ignored (at least in Rails 7) #25

Open stex opened 2 years ago

stex commented 2 years ago

Hey!

First of all, thanks a lot for this gem and for saving me from having to write pure SQL again in a Rails app :)

I ran into a problem today and it seems like the "Does it take into account default association condition" part of the README isn't fully correct.

An example model structure is fairly easy:

class Tag < ApplicationRecord
  has_many :taggings
end

class Tagging < ApplicationRecord
  belongs_to :tag
  belongs_to :taggable

  enum matching_type: [:mandatory, :optional, :excluded]
end

class Taggable < ApplicationRecord
  has_many :taggings
  has_many :tags, through: :taggings

  has_many :mandatory_tags, -> { merge(Tagging.mandatory) }, through: :taggings, source: :tag
  has_many :optional_tags, -> { merge(Tagging.optional) }, through: :taggings, source: :tag
  has_many :excluded_tags, -> { merge(Tagging.excluded) }, through: :taggings, source: :tag

  scope :without_excluded_tag_names, ->(tag_names) {
    where_not_exists(:excluded_tags, name: tag_names)
  }
end

When calling the helper associations for mandatory, optional and excluded tags directly, everything works as it should:

Taggable.first.excluded_tags
  Taggable Load (0.5ms)  SELECT "taggables".* FROM "taggables" ORDER BY "taggables"."id" ASC LIMIT ?  [["LIMIT", 1]]
  Tag Load (0.2ms)  SELECT "tags".* FROM "tags" INNER JOIN "taggings" ON "tags"."id" = "taggings"."tag_id" WHERE "taggings"."taggable_id" = ? AND "taggings"."matching_type" = ?  [["taggable_id", 1], ["matching_type", 2]]

However, when using where_not_exists, it simply swallows the default condition:

Taggable.without_excluded_tag_names(["A"])
  Taggable Load (0.3ms)  SELECT "taggables".* FROM "taggables" WHERE (NOT (EXISTS (SELECT 1 FROM "taggings" WHERE ("taggings"."taggable_id" = "taggables"."id") AND (EXISTS (SELECT 1 FROM "tags" WHERE ("taggings"."tag_id" = "tags"."id") AND "tags"."name" = 'A')))))

At first I thought that this might be problem with the condition being on the through table, but the same happens when defining a condition on the tags table itself.

I could provide an example Rails 7 application if that helps.

EugZol commented 2 years ago

@Stex I don't seem to reproduce this error.

has_many :mandatory_tags, -> { merge(Tagging.mandatory) }, through: :taggings, source: :tag

I don't think that is correct notation. Tags are extracted with this association, but Tagging scope is merged.

but the same happens when defining a condition on the tags table itself.

Please provide that example.

mehanoid commented 2 years ago

Here's an example

# frozen_string_literal: true

require 'bundler/inline'

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

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

  # Activate the gem you are reporting the issue against.
  gem 'activerecord', '~> 7.0.0'
  gem 'sqlite3'
  gem 'where_exists', '2.0.1'
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 :posts, force: true do |t|
    t.boolean :archived, default: false, null: false
  end

  create_table :comments, force: true do |t|
    t.integer :post_id
    t.integer :commentator_id
  end

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

class Post < ActiveRecord::Base
  has_many :comments
end

class Comment < ActiveRecord::Base
  belongs_to :post
  belongs_to :commentator
end

class Commentator < ActiveRecord::Base
  has_many :comments
  has_many :posts, -> { where(archived: false) }, through: :comments
end

class BugTest < Minitest::Test
  def test_where_exists
    post = Post.create!
    archived_post = Post.create! archived: true

    commentator = Commentator.create! posts: [post]
    commentator2 = Commentator.create! posts: [archived_post]
    commentator3 = Commentator.create!

    assert_equal [commentator], Commentator.where_exists(:posts).to_a # fail: also includes commentator2
    assert_equal [commentator2, commentator3], Commentator.where_not_exists(:posts).to_a # fail: does not include commentator2
  end
end