zulip
zulip copied to clipboard
search operators: Add is-muted search operator.
Add the is:muted search operator.
-is:muted is an alias for the in:home operator.
Fix the existing logic for excluding muted messages. Existing logic does not take into account followed topics and unmuted topics in muted channels.
Existing logic did not work when including messages,
it only worked for excluding messages. For the is:muted operator,
we need to include messages that are muted.
Add tests for the different cases of the is:muted operator.
Fixes: #16943
Explain Analyze
The most complicated query is the regular is:muted operator (without channel narrow) query.
(SELECT message_id, flags
FROM zerver_usermessage JOIN zerver_message ON zerver_usermessage.message_id = zerver_message.id
WHERE user_profile_id = 72 AND NOT (((recipient_id NOT IN (165)) OR recipient_id = 165 AND upper(subject) = upper('bridge')) AND NOT (recipient_id = 157 AND upper(subject) = upper('little WEB APP was compiling quickly'))) ORDER BY message_id DESC);
Note: recipient_id NOT IN (165) will have all the muted channels ids, recipient_id = 165 AND upper(subject) = upper('bridge') will exist for all followed topics and similarly in the NOT part of the query it will exist for all muted topics.
These channel ids and topic data is fetched separately using Django's ORM (just as it was being fetched before). Not sure how that impacts the overall performance of the query.
Explain Analyze
QUERY PLAN
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Sort (cost=1095.31..1095.67 rows=144 width=12) (actual time=2.213..2.227 rows=324 loops=1)
Sort Key: zerver_usermessage.message_id DESC
Sort Method: quicksort Memory: 40kB
-> Hash Join (cost=616.83..1090.15 rows=144 width=12) (actual time=0.922..2.156 rows=324 loops=1)
Hash Cond: (zerver_usermessage.message_id = zerver_message.id)
-> Bitmap Heap Scan on zerver_usermessage (cost=42.99..508.60 rows=2929 width=12) (actual time=0.096..1.117 rows=2908 loops=1)
Recheck Cond: (user_profile_id = 72)
Heap Blocks: exact=203
-> Bitmap Index Scan on zerver_usermessage_user_profile_id_e901f3b7 (cost=0.00..42.26 rows=2929 width=0) (actual time=0.072..0.073 rows=2908 loops=1)
Index Cond: (user_profile_id = 72)
-> Hash (cost=570.78..570.78 rows=245 width=4) (actual time=0.814..0.814 rows=324 loops=1)
Buckets: 1024 Batches: 1 Memory Usage: 20kB
-> Index Only Scan using zerver_message_realm_recipient_subject on zerver_message (cost=0.28..570.78 rows=245 width=4) (actual time=0.213..0.773 rows=324 loops=1)
Filter: (((recipient_id = 165) AND ((recipient_id <> 165) OR (upper((subject)::text) <> 'BRIDGE'::text))) OR ((recipient_id = 157) AND (upper((subject)::text) = 'LITTLE WEB APP WAS COMPILING QUICKLY'::text)))
Rows Removed by Filter: 4676
Heap Fetches: 0
Planning Time: 0.503 ms
Execution Time: 2.407 ms
(18 rows)
The less complicated query is the regular is:muted channel:cname operator query.
(SELECT message_id, flags
FROM zerver_usermessage JOIN zerver_message ON zerver_usermessage.message_id = zerver_message.id
WHERE user_profile_id = 72 AND NOT (recipient_id = 165 AND upper(subject) = upper('bridge')) AND recipient_id = 165 ORDER BY message_id DESC
)
Explain Analyze
QUERY PLAN
------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Sort (cost=974.42..974.78 rows=142 width=12) (actual time=1.536..1.548 rows=227 loops=1)
Sort Key: zerver_usermessage.message_id DESC
Sort Method: quicksort Memory: 35kB
-> Hash Join (cost=496.03..969.34 rows=142 width=12) (actual time=0.379..1.485 rows=227 loops=1)
Hash Cond: (zerver_usermessage.message_id = zerver_message.id)
-> Bitmap Heap Scan on zerver_usermessage (cost=42.99..508.60 rows=2929 width=12) (actual time=0.086..0.918 rows=2908 loops=1)
Recheck Cond: (user_profile_id = 72)
Heap Blocks: exact=203
-> Bitmap Index Scan on zerver_usermessage_user_profile_id_e901f3b7 (cost=0.00..42.26 rows=2929 width=0) (actual time=0.063..0.063 rows=2908 loops=1)
Index Cond: (user_profile_id = 72)
-> Hash (cost=450.02..450.02 rows=242 width=4) (actual time=0.282..0.283 rows=227 loops=1)
Buckets: 1024 Batches: 1 Memory Usage: 16kB
-> Index Only Scan using zerver_message_realm_recipient_subject on zerver_message (cost=0.28..450.02 rows=242 width=4) (actual time=0.081..0.255 rows=227 loops=1)
Index Cond: (recipient_id = 165)
Filter: ((recipient_id <> 165) OR (upper((subject)::text) <> 'BRIDGE'::text))
Rows Removed by Filter: 15
Heap Fetches: 0
Planning Time: 0.515 ms
Execution Time: 1.598 ms
(19 rows)
Hello @zulip/server-search members, this pull request was labeled with the "area: search" label, so you may want to check it out!
@PieterCK please review this PR.
@zulipbot add "buddy review"
@kennethnrk - Thanks for all this work! However, it seems like there is an issue when querying just Muted Messages in the search:
@kennethnrk - Thanks for all this work! However, it seems like there is an issue when querying just Muted Messages in the search:
I'm sorry, it was a simple error made while getting tests to pass. I've fixed it.
Note for next reviewers: I'm not too familiar with the code around our search/narrowing feature, so besides manual testing, I focus on code style and overall logic. I find the changes readable enough.
Issue #16943 plans to gradually deprecate the in:home search keyword after this PR is implemented. Should we add code comments in the relevant areas to help track this follow-up plan?
Issue #16943 plans to gradually deprecate the
in:homesearch keyword after this PR is implemented. Should we add code comments in the relevant areas to help track this follow-up plan?
Sure I can add some comments indicating the same.
Thanks for the detailed review, I'll address the comments.
@alya I'll mark this as ready for the next round of review!
Heads up @kennethnrk, we just merged some commits that conflict with the changes you made in this pull request! You can review this repository's recent commits to see where the conflicts occur. Please rebase your feature branch against the upstream/main branch and resolve your pull request's merge conflicts accordingly.
Attempted a rebase and it seems most of the conflicts are with #32579 which fixes some of the issues with the muting logic. It will require significant manual testing to identify which issues persist and the changes needed to fix them. @opmkumar will you be down to identify if the muting logic is still incorrect and which cases it fails for, since I'm not sure how soon I'll be able to do a deep run through?
Closing in favor of #33480, which completes this work. Thanks @kennethnrk!