zulip icon indicating copy to clipboard operation
zulip copied to clipboard

search operators: Add is-muted search operator.

Open kennethnrk opened this issue 1 year ago • 9 comments

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)

kennethnrk avatar Jul 24 '24 09:07 kennethnrk

Hello @zulip/server-search members, this pull request was labeled with the "area: search" label, so you may want to check it out!

zulipbot avatar Jul 24 '24 09:07 zulipbot

@PieterCK please review this PR.

kennethnrk avatar Jul 27 '24 12:07 kennethnrk

@zulipbot add "buddy review"

kennethnrk avatar Jul 27 '24 12:07 kennethnrk

@kennethnrk - Thanks for all this work! However, it seems like there is an issue when querying just Muted Messages in the search:

Screencast from 07-29-2024 10:37:44 AM.webm

PieterCK avatar Jul 29 '24 03:07 PieterCK

@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.

kennethnrk avatar Jul 29 '24 04:07 kennethnrk

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?

PieterCK avatar Jul 29 '24 09:07 PieterCK

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?

Sure I can add some comments indicating the same.

kennethnrk avatar Jul 29 '24 12:07 kennethnrk

Thanks for the detailed review, I'll address the comments.

kennethnrk avatar Jul 29 '24 12:07 kennethnrk

@alya I'll mark this as ready for the next round of review!

PieterCK avatar Jul 30 '24 08:07 PieterCK

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.

zulipbot avatar Dec 03 '24 02:12 zulipbot

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?

kennethnrk avatar Feb 02 '25 18:02 kennethnrk

Closing in favor of #33480, which completes this work. Thanks @kennethnrk!

timabbott avatar Mar 12 '25 01:03 timabbott