Skip to content

Commit

Permalink
WIP
Browse files Browse the repository at this point in the history
  • Loading branch information
MaxLap committed Dec 4, 2024
1 parent 11f3e09 commit 014e5ee
Show file tree
Hide file tree
Showing 4 changed files with 171 additions and 8 deletions.
12 changes: 6 additions & 6 deletions ALTERNATIVES_PROBLEMS.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,10 @@ This is a list of some of those alternatives, explaining what issues they have o

**Use this gem, you will avoid problems and save time**

* No more having to choose, case by case, which way has the less problems.
* No more having to choose, case by case, which way has the less problems.
Just use `#where_assoc_*` each time and avoid every problems.
* Need less raw SQL, which means less code, more clarity and less maintenance.
* Generates a single `#where`. No weird side-effects things like `#eager_load` or `#join`
* Generates a single `#where`. No weird side-effects things like `#eager_load` or `#join`
This makes well-behaved scopes, you can even have multiple conditions on the same association
* Handles recursive associations correctly.
* Handles has_one correctly (Except [MySQL has a limitation](README.md#mysql-doesnt-support-sub-limit)).
Expand Down Expand Up @@ -169,7 +169,7 @@ Post.joins(:comments).where(comments: {is_spam: true})

* If the association maps to multiple records (such as with a has_many), then the the relation will return one
record for each matching association record. In this example, you would get the same post twice if it has 2
comments that are marked as spam.
comments that are marked as spam.
Using `uniq` can solve this issue, but if you do that in a scope, then that scope unexpectedly adds a DISTINCT
to your query, which can lead to unexpected results if you actually wanted duplicates for a different reason.

Expand Down Expand Up @@ -220,7 +220,7 @@ Post.eager_load(:comments).where(comments: {is_spam: true})
scope like `have_reported_comments` to trigger eager loading. This is a performance degradation.

* The eager loaded records of the association are actually also filtered by the conditions. All of the posts
returned will only have the comments that are spam.
returned will only have the comments that are spam.
This means if you iterate on `Post.have_reported_comments` to display each of the comments of the posts that
have at least one reported comment, you are actually only going to display the reported comments. This may
be what you wanted to do, but it clearly isn't intuitive.
Expand Down Expand Up @@ -276,14 +276,14 @@ User.where_exists(:posts) { |posts| posts.where_exists(comments) }
* Has no equivalent to `#where_assoc_count`
```ruby
# There is no equivalent for this (posts with more than 5 comments)
Post.where_assoc_count(:comments, :>, 5)
Post.where_assoc_count(5, :<, :comments)
```

* [Treats has_one like a has_many](#treating-has_one-like-has_many)

* [Can't handle recursive associations](#unable-to-handle-recursive-associations)

* `#where_exists` is shorter than `#where_assoc_exists`, but it is also less obvious about what it does.
* `#where_exists` is shorter than `#where_assoc_exists`, but it is also less obvious about what it does.
In any case, it is trivial to alias one name to the other one.

* where_exists supports Rails 4.2 and up, while activerecord_where_assoc supports Rails 4.1 and up.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# Unreleased

* Optimize `has_one` handling when `#where_assoc_exists` with a `has_one` as last association + without any conditions or offset.

# 1.2.0 - 2024-08-31

* Add support for composite primary keys in Rails 7.2
Expand Down
14 changes: 12 additions & 2 deletions lib/active_record_where_assoc/core_logic.rb
Original file line number Diff line number Diff line change
Expand Up @@ -155,9 +155,17 @@ def self.relations_on_one_association(record_class, association_name, given_cond

init_scopes = initial_scopes_from_reflection(record_class, reflection_chain[i..-1], constraints_chain[i], options)
current_scopes = init_scopes.map do |alias_scope, current_scope, klass_scope|
current_scope = process_association_step_limits(current_scope, reflection, record_class, options)

if i.zero?
if given_conditions || klass_scope || last_assoc_block || current_scope.offset_value || nest_assocs_block == NestWithSumBlock
# In the deepest layer, the limit & offset complexities only matter when:
# * There is a condition to apply
# * There is an offset (which is a form a filtering)
# * We are counting the total matches
# Since last_assoc_block is always set except for the deepest association, and is only unset for the deepest layer if
# there is no condition given, using it as part of the condition does a lot of work here.
current_scope = process_association_step_limits(current_scope, reflection, record_class, options)
end

current_scope = current_scope.where(given_conditions) if given_conditions
if klass_scope
if klass_scope.respond_to?(:call)
Expand All @@ -167,6 +175,8 @@ def self.relations_on_one_association(record_class, association_name, given_cond
end
end
current_scope = apply_proc_scope(current_scope, last_assoc_block) if last_assoc_block
else
current_scope = process_association_step_limits(current_scope, reflection, record_class, options)
end

# Those make no sense since at this point, we are only limiting the value that would match using conditions
Expand Down
151 changes: 151 additions & 0 deletions test/tests/wa_has_one_optimization_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,151 @@
# frozen_string_literal: true

require_relative "../test_helper"

# has_one requires doing sub-queries with SELECT IN / applying limits to
# handle conditions / counts properly. However, when there is no conditions and counts,
# the deepest layer can avoid those sub-queries, which is faster.

describe "wa_exists(has_one)" do
# MySQL doesn't support has_one
next if Test::SelectedDBHelper == Test::MySQL

it "with a single has_one and no condition, doesn't do a sub-query" do
sql = S0.where_assoc_exists(:o1).to_sql
assert sql.include?('SELECT 1 FROM "s1s"')
assert !sql.include?('SELECT 1 FROM (SELECT "s1s".*')
end

it "with a single has_one and a block, does a sub-query" do
sql = S0.where_assoc_exists(:o1) {}.to_sql
assert !sql.include?('SELECT 1 FROM "s1s"')
assert sql.include?('SELECT 1 FROM (SELECT "s1s".*')
end

it "with a single has_one and a condition, does a sub-query" do
sql = S0.where_assoc_exists(:o1, id: 1).to_sql
assert !sql.include?('SELECT 1 FROM "s1s"')
assert sql.include?('SELECT 1 FROM (SELECT "s1s".*')
end


it "with a two has_one and no condition, does a sub-query only once" do
sql = S0.where_assoc_exists([:o1, :o2]).to_sql
assert !sql.include?('SELECT 1 FROM "s1s"')
assert sql.include?('SELECT 1 FROM (SELECT "s1s".*')

assert sql.include?('SELECT 1 FROM "s2s"')
assert !sql.include?('SELECT 1 FROM (SELECT "s2s".*')
end

it "with a two has_one and a block, does two sub-queries" do
sql = S0.where_assoc_exists([:o1, :o2]) {}.to_sql
assert !sql.include?('SELECT 1 FROM "s1s"')
assert sql.include?('SELECT 1 FROM (SELECT "s1s".*')

assert !sql.include?('SELECT 1 FROM "s2s"')
assert sql.include?('SELECT 1 FROM (SELECT "s2s".*')
end

it "with a two has_one and a condition, does two sub-queries" do
sql = S0.where_assoc_exists([:o1, :o2], id: 1).to_sql
assert !sql.include?('SELECT 1 FROM "s1s"')
assert sql.include?('SELECT 1 FROM (SELECT "s1s".*')

assert !sql.include?('SELECT 1 FROM "s2s"')
assert sql.include?('SELECT 1 FROM (SELECT "s2s".*')
end


it "with a a has_one through has_one and no condition, does a sub-query only once" do
sql = S0.where_assoc_exists(:o2o1).to_sql
assert !sql.include?('SELECT 1 FROM "s1s"')
assert sql.include?('SELECT 1 FROM (SELECT "s1s".*')

assert sql.include?('SELECT 1 FROM "s2s"')
assert !sql.include?('SELECT 1 FROM (SELECT "s2s".*')
end

it "with a a has_one through has_one and a block, does two sub-queries" do
sql = S0.where_assoc_exists(:o2o1) {}.to_sql
assert !sql.include?('SELECT 1 FROM "s1s"')
assert sql.include?('SELECT 1 FROM (SELECT "s1s".*')

assert !sql.include?('SELECT 1 FROM "s2s"')
assert sql.include?('SELECT 1 FROM (SELECT "s2s".*')
end

it "with a a has_one through has_one and a condition, does two sub-queries" do
sql = S0.where_assoc_exists(:o2o1, id: 1).to_sql
assert !sql.include?('SELECT 1 FROM "s1s"')
assert sql.include?('SELECT 1 FROM (SELECT "s1s".*')

assert !sql.include?('SELECT 1 FROM "s2s"')
assert sql.include?('SELECT 1 FROM (SELECT "s2s".*')
end
end

describe "wa_exists(schema has_one)" do
next if Test::SelectedDBHelper == Test::SQLite3

it "with a single schema has_one and no condition, doesn't do a sub-query" do
sql = S0.where_assoc_exists(:schema_o1).to_sql
puts sql
assert sql.include?('SELECT 1 FROM "bar_schema"."s1s"')
assert !sql.include?('SELECT 1 FROM (SELECT "bar_schema"."s1s".*')
assert !sql.include?(' IN (SELECT "bar_schema"."schema_s1s"')
end

it "with a single schema has_one and a condition, does a ...IN..." do
sql = S0.where_assoc_exists(:schema_o1, id: 1).to_sql
assert !sql.include?('SELECT 1 FROM "bar_schema"."s1s"')
assert !sql.include?('SELECT 1 FROM (SELECT "bar_schema"."s1s".*')
assert sql.include?(' IN (SELECT "bar_schema"."schema_s1s"')
end


it "with two single schema has_one and no condition, does a single sub-query with ...IN..." do
sql = S0.where_assoc_exists([:schema_o1, :schema_o2]).to_sql
puts sql
assert !sql.include?('SELECT 1 FROM "bar_schema"."s1s"')
assert !sql.include?('SELECT 1 FROM (SELECT "bar_schema"."s1s".*')
assert sql.include?(' IN (SELECT "bar_schema"."schema_s1s"')

assert sql.include?('SELECT 1 FROM "bar_schema"."s2s"')
assert !sql.include?('SELECT 1 FROM (SELECT "bar_schema"."s2s".*')
assert !sql.include?(' IN (SELECT "bar_schema"."schema_s2s"')
end

it "with two single schema has_one and a condition, does two ...IN..." do
sql = S0.where_assoc_exists([:schema_o1, :schema_o2], id: 1).to_sql
assert !sql.include?('SELECT 1 FROM "bar_schema"."s1s"')
assert !sql.include?('SELECT 1 FROM (SELECT "bar_schema"."s1s".*')
assert sql.include?(' IN (SELECT "bar_schema"."schema_s1s"')

assert !sql.include?('SELECT 1 FROM "bar_schema"."s2s"')
assert !sql.include?('SELECT 1 FROM (SELECT "bar_schema"."s2s".*')
assert sql.include?(' IN (SELECT "bar_schema"."schema_s2s"')
end
end

describe "wa_count(has_one)" do
# MySQL doesn't support has_one
next if Test::SelectedDBHelper == Test::MySQL

it "with a single has_one, does a sub-query (because the count needs it)" do
sql = S0.where_assoc_count(1, :<, :o1).to_sql
assert sql.include?('SELECT COUNT(*) FROM (SELECT "s1s".*')
end

it "with a two has_one and a condition, does two sub-queries (because the count needs it)" do
sql = S0.where_assoc_count(1, :<, [:o1, :o2]).to_sql
assert sql.include?('FROM (SELECT "s1s".*')
assert sql.include?('SELECT COUNT(*) FROM (SELECT "s2s".*')
end

it "with a a has_one through has_one and a condition, does two sub-queries (because the count needs it)" do
sql = S0.where_assoc_count(1, :<, :o2o1).to_sql
assert sql.include?('FROM (SELECT "s1s".*')
assert sql.include?('SELECT COUNT(*) FROM (SELECT "s2s".*')
end
end

0 comments on commit 014e5ee

Please sign in to comment.