Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Use a nested query to save a rountrip #4

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Use a nested query to save a rountrip #4

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 1 addition & 9 deletions lib/fast_page/active_record_methods.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,7 @@ def deferred_join_load

id_scope = dup
id_scope = id_scope.except(:includes) unless references_eager_loaded_tables?
ids = id_scope.pluck(:id)

if ids.empty?
@records = []
@loaded = true
return self
end

@records = where(id: ids).unscope(:limit).unscope(:offset).load
@records = where(id: id_scope).unscope(:limit).unscope(:offset).load
@loaded = true

self
Expand Down
12 changes: 5 additions & 7 deletions test/fast_page_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ def test_executes_extra_id_query

User.all.limit(5).fast_page

assert_equal 2, count
assert_equal 1, count

ActiveSupport::Notifications.unsubscribe("sql.active_record")
end
Expand All @@ -40,9 +40,8 @@ def test_correct_sql

User.all.limit(5).fast_page

assert_equal 2, queries.size
assert_includes queries, 'SELECT "users"."id" FROM "users" LIMIT ?'
assert_includes queries, 'SELECT "users".* FROM "users" WHERE "users"."id" IN (?, ?, ?, ?, ?)'
assert_equal 1, queries.size
assert_includes queries, 'SELECT "users".* FROM "users" WHERE "users"."id" IN (SELECT "users"."id" FROM "users" LIMIT ?)'

ActiveSupport::Notifications.unsubscribe("sql.active_record")
end
Expand All @@ -56,11 +55,10 @@ def test_removes_includes_id_query

User.all.includes(:organization).limit(50).fast_page

assert_equal 3, queries.size
assert_equal 2, queries.size

# Organizations are not included on the ID query (not needed)
assert_includes queries, 'SELECT "users"."id" FROM "users" LIMIT ?'
assert_includes queries, 'SELECT "users".* FROM "users" WHERE "users"."id" IN (?, ?, ?, ?, ?, ?, ?, ?, ?, ?)'
assert_includes queries, 'SELECT "users".* FROM "users" WHERE "users"."id" IN (SELECT "users"."id" FROM "users" LIMIT ?)'
# Includes are still loaded
assert_includes queries, 'SELECT "organizations".* FROM "organizations" WHERE "organizations"."id" = ?'

Expand Down
5 changes: 2 additions & 3 deletions test/pagy_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,10 @@ def test_pagy_works
assert_equal 1, pagy.page
assert_equal 2, pagy.next
assert_equal 5, records.size
assert_equal 3, queries.size
assert_equal 2, queries.size

assert_includes queries, 'SELECT COUNT(*) FROM "users"'
assert_includes queries, 'SELECT "users"."id" FROM "users" LIMIT ? OFFSET ?'
assert_includes queries, 'SELECT "users".* FROM "users" WHERE "users"."id" IN (?, ?, ?, ?, ?)'
assert_includes queries, 'SELECT "users".* FROM "users" WHERE "users"."id" IN (SELECT "users"."id" FROM "users" LIMIT ? OFFSET ?)'

ActiveSupport::Notifications.unsubscribe("sql.active_record")
end
Expand Down