From 98ee6c31c1011aa57e22af6383c441b8c4a1a4e7 Mon Sep 17 00:00:00 2001 From: evazion Date: Sun, 3 Jan 2021 19:01:44 -0600 Subject: [PATCH] favorites: refactor fav:/ordfav: searches to not use fav_string. Refactor fav: and ordfav: searches to use the favorites table instead of the posts.fav_string. This may be slower for fav: searches. The fav_string effectively treats favorites like secret tags on the post, so fav: searches were effectively the same as tag searches. Now they do a subquery on the favorites table, which may not perform as well for things like multiple fav: metatags or negated fav: metatags. For ordfav: searches, this may be faster. ordfav: searches had a tag match clause (`tag_index @@ 'fav:123'`) in addition to a join on the favs table. This was redundant, and in some cases it inhibited the query planner from choosing a more optimal plan. Partially addresses #4652 by eliminating another place where we depended on the fav_string. --- app/logical/post_query_builder.rb | 7 ++++--- test/unit/post_query_builder_test.rb | 6 ++++++ 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/app/logical/post_query_builder.rb b/app/logical/post_query_builder.rb index 5a5b11fed..8e6fc980f 100644 --- a/app/logical/post_query_builder.rb +++ b/app/logical/post_query_builder.rb @@ -393,7 +393,8 @@ class PostQueryBuilder favuser = User.find_by_name(username) if favuser.present? && Pundit.policy!([current_user, nil], favuser).can_see_favorites? - tags_include("fav:#{favuser.id}") + favorites = Favorite.from("favorites_#{favuser.id % 100} AS favorites").where(user: favuser) + Post.where(id: favorites.select(:post_id)) else Post.none end @@ -402,8 +403,8 @@ class PostQueryBuilder def ordfav_matches(username) user = User.find_by_name(username) - if user.present? - favorites_include(username).joins(:favorites).merge(Favorite.for_user(user.id)).order("favorites.id DESC") + if user.present? && Pundit.policy!([current_user, nil], user).can_see_favorites? + Post.joins(:favorites).merge(Favorite.for_user(user.id)).order("favorites.id DESC") else Post.none end diff --git a/test/unit/post_query_builder_test.rb b/test/unit/post_query_builder_test.rb index f4882c287..b38ce79e8 100644 --- a/test/unit/post_query_builder_test.rb +++ b/test/unit/post_query_builder_test.rb @@ -191,9 +191,15 @@ class PostQueryBuilderTest < ActiveSupport::TestCase assert_tag_match([post1], "fav:#{user1.name}") assert_tag_match([post2], "fav:#{user2.name}") assert_tag_match([], "fav:#{user3.name}") + + assert_tag_match([], "fav:#{user1.name} fav:#{user2.name}") + assert_tag_match([post1], "fav:#{user1.name} -fav:#{user2.name}") + assert_tag_match([post3], "-fav:#{user1.name} -fav:#{user2.name}") + assert_tag_match([], "fav:dne") assert_tag_match([post3, post2], "-fav:#{user1.name}") + assert_tag_match([post3], "-fav:#{user1.name} -fav:#{user2.name}") assert_tag_match([post3, post2, post1], "-fav:dne") as(user3) do