From 5480bd5fc20d03d336da6709c1fa6600bcfb5607 Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Tue, 16 Aug 2022 19:31:51 +0200 Subject: [PATCH] Use a nested query to save a rountrip --- lib/fast_page/active_record_methods.rb | 10 +--------- test/fast_page_test.rb | 12 +++++------- test/pagy_test.rb | 5 ++--- 3 files changed, 8 insertions(+), 19 deletions(-) diff --git a/lib/fast_page/active_record_methods.rb b/lib/fast_page/active_record_methods.rb index 37b3753..79607ac 100644 --- a/lib/fast_page/active_record_methods.rb +++ b/lib/fast_page/active_record_methods.rb @@ -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 diff --git a/test/fast_page_test.rb b/test/fast_page_test.rb index ea0afb3..419bab6 100644 --- a/test/fast_page_test.rb +++ b/test/fast_page_test.rb @@ -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 @@ -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 @@ -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" = ?' diff --git a/test/pagy_test.rb b/test/pagy_test.rb index b7235a4..a583387 100644 --- a/test/pagy_test.rb +++ b/test/pagy_test.rb @@ -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