diff --git a/backend/app/feedquery.py b/backend/app/feedquery.py index 75b09b2a..1c6020c2 100644 --- a/backend/app/feedquery.py +++ b/backend/app/feedquery.py @@ -131,7 +131,6 @@ def filtered_query( either mode via the VideoState outer join.""" state = aliased(VideoState) status_expr = func.coalesce(state.status, "new") - position_expr = func.coalesce(state.position_seconds, 0) query = select(*feed_columns(user, state)).join( Channel, Channel.id == Video.channel_id @@ -163,7 +162,14 @@ def filtered_query( and_(SearchFind.video_id == Video.id, SearchFind.user_id == user.id), ) - query = query.outerjoin( + # The state-gated views (watched/hidden/in_progress) only ever return videos that HAVE a + # video_states row, so an INNER join lets the planner drive from the user's own small video_states + # set (~100 rows) instead of scanning all videos and outer-joining (PERF-16). status is NOT NULL on + # a present row (server_default 'new'), so the raw-column filters below match the coalesce form + # exactly. 'unwatched'/'all' include row-less 'new' videos, so they keep the outer join. + state_required = show in ("watched", "hidden", "in_progress") + join_state = query.join if state_required else query.outerjoin + query = join_state( state, and_(state.video_id == Video.id, state.user_id == user.id) ) @@ -246,15 +252,25 @@ def filtered_query( # channel searches work. `rank_expr` (weight-aware ts_rank) drives the "relevance" sort. rank_expr = None if q: + # Resolve the channels whose NAME matches up front (there are only a few hundred channels, so + # this is a tiny standalone scan) and match videos by channel_id. ORing an unindexed + # `unaccent(channels.title) ILIKE '%q%'` straight into the per-video predicate defeated the + # search_vector GIN — the planner detoasted every non-matching tsvector to test the `@@` + # (~2.6s/search on the prod-sized set); channel_id.in_(...) keeps the GIN as a BitmapOr branch + # (PERF-12). Empty match -> false() so the OR collapses to the text match alone. + like = func.unaccent(f"%{q}%") + matching_channel_ids = ( + db.execute(select(Channel.id).where(func.unaccent(Channel.title).ilike(like))) + .scalars() + .all() + ) + channel_match = ( + Video.channel_id.in_(matching_channel_ids) if matching_channel_ids else false() + ) ts_str = to_tsquery_str(q) if ts_str: tsq = func.to_tsquery("public.unaccent_simple", ts_str) - query = query.where( - or_( - Video.search_vector.op("@@")(tsq), - func.unaccent(Channel.title).ilike(func.unaccent(f"%{q}%")), - ) - ) + query = query.where(or_(Video.search_vector.op("@@")(tsq), channel_match)) # Scale ts_rank (a float4) to an integer score so the keyset cursor compares it # EXACTLY — a raw float4 vs the float8 cursor value mismatches on round-trip and # breaks paging (the same page repeats). 1e6 granularity is ample; ties break on @@ -262,13 +278,7 @@ def filtered_query( rank_expr = cast(func.ts_rank(Video.search_vector, tsq) * 1000000, BigInteger) else: # No usable tokens (e.g. only punctuation): fall back to a plain substring match. - like = func.unaccent(f"%{q}%") - query = query.where( - or_( - func.unaccent(Video.title).ilike(like), - func.unaccent(Channel.title).ilike(like), - ) - ) + query = query.where(or_(func.unaccent(Video.title).ilike(like), channel_match)) if tags: # AND across tag categories (e.g. language AND topic narrows), OR within a @@ -315,14 +325,15 @@ def filtered_query( if show == "unwatched": query = query.where(status_expr.notin_(("watched", "hidden"))) elif show == "in_progress": - # Started but not finished: a stored resume position, not yet watched/hidden. + # Started but not finished: a stored resume position, not yet watched/hidden. INNER-joined + # (state_required) so the raw state columns are safe and the planner can use video_states. query = query.where( - and_(position_expr > 0, status_expr.notin_(("watched", "hidden"))) + and_(state.position_seconds > 0, state.status.notin_(("watched", "hidden"))) ) elif show == "watched": - query = query.where(status_expr == "watched") + query = query.where(state.status == "watched") elif show == "hidden": - query = query.where(status_expr == "hidden") + query = query.where(state.status == "hidden") else: # all query = query.where(status_expr != "hidden") diff --git a/backend/tests/test_feed_views.py b/backend/tests/test_feed_views.py new file mode 100644 index 00000000..161a29e2 --- /dev/null +++ b/backend/tests/test_feed_views.py @@ -0,0 +1,84 @@ +"""Feed search + watch-state views (E2 S3), DB-lane. + +Regression nets for two query rewrites that must stay behaviour-equivalent: +- PERF-12: the search channel-name branch moved from an ORed `unaccent(channels.title) ILIKE` (which + defeated the search_vector GIN) to a pre-resolved `channel_id IN (...)`. A search must STILL return + both videos whose text matches AND videos whose CHANNEL name matches. +- PERF-16: the watched/hidden/in_progress views moved from an outer join + `coalesce(state.status,'new')` + to an inner join on video_states + the raw column. Each view must return exactly the same set. +""" +from datetime import datetime, timezone + +from app.models import Channel, Subscription, Video, VideoState + + +def _dt(day: int) -> datetime: + return datetime(2024, 1, day, tzinfo=timezone.utc) + + +def _video(vid: str, ch: str, title: str) -> Video: + return Video( + id=vid, channel_id=ch, published_at=_dt(1), title=title, + is_short=False, live_status="none", via_search=False, via_explore=False, + ) + + +def _feed_ids(client, query: str) -> set[str]: + resp = client.get(f"/api/feed?{query}&limit=50") + assert resp.status_code == 200, resp.text + return {it["id"] for it in resp.json()["items"]} + + +def test_search_matches_both_video_text_and_channel_name(db, as_user, make_user): + # Two subscribed channels: one whose NAME matches the query (its video's text does not), and one + # whose video TEXT matches (its name does not). The pre-resolved channel_id branch must still OR in + # the name match, so both videos come back — and an unrelated video on a matching-name-less channel + # stays out. + user = make_user(email="search@example.com") + db.add_all([ + Channel(id="UCkube", title="Kubernetes Weekly", subscriber_count=10), + Channel(id="UCother", title="Some Other Channel", subscriber_count=10), + ]) + db.add_all([ + Subscription(user_id=user.id, channel_id="UCkube", priority=0, hidden=False), + Subscription(user_id=user.id, channel_id="UCother", priority=0, hidden=False), + ]) + db.add_all([ + _video("vidChan", "UCkube", "Totally unrelated content"), # channel NAME matches + _video("vidText", "UCother", "kubernetes deep dive"), # video TEXT matches + _video("vidNone", "UCother", "cooking pasta at home"), # neither matches + ]) + db.commit() + + ids = _feed_ids(as_user(user), "q=kubernetes&show=all") + assert ids == {"vidChan", "vidText"} + + +def test_watch_state_views_return_the_right_sets(db, as_user, make_user): + user = make_user(email="views@example.com") + db.add(Channel(id="UCv", title="Channel V", subscriber_count=10)) + db.add(Subscription(user_id=user.id, channel_id="UCv", priority=0, hidden=False)) + db.add_all([ + _video("vWatch", "UCv", "watched one"), + _video("vHide", "UCv", "hidden one"), + _video("vProg", "UCv", "in progress one"), + _video("vNew", "UCv", "brand new one"), + ]) + db.flush() + db.add_all([ + VideoState(user_id=user.id, video_id="vWatch", status="watched"), + VideoState(user_id=user.id, video_id="vHide", status="hidden"), + # in progress = a stored resume position while still "new" (not watched/hidden) + VideoState(user_id=user.id, video_id="vProg", status="new", position_seconds=30), + # vNew has NO state row -> "new"/unwatched + ]) + db.commit() + + client = as_user(user) + assert _feed_ids(client, "show=watched") == {"vWatch"} + assert _feed_ids(client, "show=hidden") == {"vHide"} + assert _feed_ids(client, "show=in_progress") == {"vProg"} + # unwatched excludes watched + hidden, keeps new AND in-progress + assert _feed_ids(client, "show=unwatched") == {"vProg", "vNew"} + # "all" shows everything except hidden + assert _feed_ids(client, "show=all") == {"vWatch", "vProg", "vNew"}