diff --git a/api/dbv1/get_playlists.sql.go b/api/dbv1/get_playlists.sql.go index 8e9c5984..8b0cdae9 100644 --- a/api/dbv1/get_playlists.sql.go +++ b/api/dbv1/get_playlists.sql.go @@ -14,7 +14,7 @@ import ( ) const getPlaylists = `-- name: GetPlaylists :many -WITH my_follows AS ( +WITH my_follows AS MATERIALIZED ( SELECT followee_user_id as user_id, follower_count @@ -182,6 +182,7 @@ type GetPlaylistsRow struct { FolloweeFavorites json.RawMessage `json:"followee_favorites"` } +// See get_tracks.sql for why my_follows is MATERIALIZED. func (q *Queries) GetPlaylists(ctx context.Context, arg GetPlaylistsParams) ([]GetPlaylistsRow, error) { rows, err := q.db.Query(ctx, getPlaylists, arg.MyID, arg.Ids) if err != nil { diff --git a/api/dbv1/get_tracks.sql.go b/api/dbv1/get_tracks.sql.go index f5ee3142..1dd6981d 100644 --- a/api/dbv1/get_tracks.sql.go +++ b/api/dbv1/get_tracks.sql.go @@ -14,7 +14,7 @@ import ( ) const getTracks = `-- name: GetTracks :many -WITH my_follows AS ( +WITH my_follows AS MATERIALIZED ( SELECT followee_user_id as user_id, follower_count @@ -325,6 +325,10 @@ type GetTracksRow struct { AccessAuthorities []string `json:"access_authorities"` } +// MATERIALIZED forces this CTE to compute once per call. Without it the +// planner inlines and re-evaluates the follows JOIN aggregate_user + sort +// once per row in the followee_reposts/favorites SubPlans, which dominates +// runtime for users with many follows. func (q *Queries) GetTracks(ctx context.Context, arg GetTracksParams) ([]GetTracksRow, error) { rows, err := q.db.Query(ctx, getTracks, arg.MyID, diff --git a/api/dbv1/queries/get_playlists.sql b/api/dbv1/queries/get_playlists.sql index 4fe07d44..ab5c0b10 100644 --- a/api/dbv1/queries/get_playlists.sql +++ b/api/dbv1/queries/get_playlists.sql @@ -1,5 +1,6 @@ -- name: GetPlaylists :many -WITH my_follows AS ( +-- See get_tracks.sql for why my_follows is MATERIALIZED. +WITH my_follows AS MATERIALIZED ( SELECT followee_user_id as user_id, follower_count diff --git a/api/dbv1/queries/get_tracks.sql b/api/dbv1/queries/get_tracks.sql index 4702daaa..43b567cd 100644 --- a/api/dbv1/queries/get_tracks.sql +++ b/api/dbv1/queries/get_tracks.sql @@ -1,5 +1,9 @@ -- name: GetTracks :many -WITH my_follows AS ( +-- MATERIALIZED forces this CTE to compute once per call. Without it the +-- planner inlines and re-evaluates the follows JOIN aggregate_user + sort +-- once per row in the followee_reposts/favorites SubPlans, which dominates +-- runtime for users with many follows. +WITH my_follows AS MATERIALIZED ( SELECT followee_user_id as user_id, follower_count diff --git a/api/v1_track_test.go b/api/v1_track_test.go index 54da4699..e9aa1422 100644 --- a/api/v1_track_test.go +++ b/api/v1_track_test.go @@ -24,6 +24,48 @@ func TestGetTrack(t *testing.T) { }) } +// Regression coverage for the my_follows CTE in get_tracks.sql. The CTE +// powers `has_current_user_*`, `followee_reposts`, and `followee_favorites` +// and was changed to MATERIALIZED for performance — these assertions guard +// against future refactors silently breaking the personalization shape. +func TestGetTrackPersonalization(t *testing.T) { + app := testAppWithFixtures(t) + app.skipAuthCheck = true + + var resp struct{ Data dbv1.Track } + + // Track 200 (eYJyn) is reposted by user 1, who is followed by user 2 (ML51L). + // Track 100 (eYZmn) is saved by user 1. + // Querying as user 2 should populate followee_* with user 1, and current-user + // repost/save flags should be false. + _, body := testGet(t, app, "/v1/full/tracks/eYJyn?user_id=ML51L", &resp) + jsonAssert(t, body, map[string]any{ + "data.id": "eYJyn", + "data.has_current_user_reposted": false, + "data.has_current_user_saved": false, + "data.followee_reposts.0.user_id": trashid.MustEncodeHashID(1), + }) + + _, body = testGet(t, app, "/v1/full/tracks/eYZmn?user_id=ML51L", &resp) + jsonAssert(t, body, map[string]any{ + "data.id": "eYZmn", + "data.has_current_user_reposted": false, + "data.has_current_user_saved": false, + "data.followee_favorites.0.user_id": trashid.MustEncodeHashID(1), + }) + + // Querying as user 1 themselves: their own repost/save shows on the flags; + // followee_* should not include themselves. + _, body = testGet(t, app, "/v1/full/tracks/eYJyn?user_id="+trashid.MustEncodeHashID(1), &resp) + jsonAssert(t, body, map[string]any{ + "data.has_current_user_reposted": true, + }) + _, body = testGet(t, app, "/v1/full/tracks/eYZmn?user_id="+trashid.MustEncodeHashID(1), &resp) + jsonAssert(t, body, map[string]any{ + "data.has_current_user_saved": true, + }) +} + func TestGetTrackFollowDownloadAcess(t *testing.T) { app := testAppWithFixtures(t) var trackResponse struct {