Skip to content

⚡ Bolt: Optimize playlist fetching with innerJoin #53

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Krosebrook wants to merge 1 commit into
base: main
Choose a base branch
from
Open

⚡ Bolt: Optimize playlist fetching with innerJoin #53

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions .jules/bolt.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,11 @@
## 2026-02-10 - Atomic Database Updates
**Learning:** Read-modify-write patterns for counters (like `playCount`) cause race conditions and extra DB round trips.
**Action:** Use atomic SQL updates (e.g., `playCount = playCount + 1`) with `returning()` to ensure data integrity and performance.

## 2026-03-01 - Join Optimization
**Learning:** Fetching related items (like songs in a playlist) by first fetching IDs and then fetching items (N+1-ish) is inefficient and complex to sort manually.
**Action:** Use `innerJoin` with `orderBy` on the join table to fetch related items in a single query with correct ordering, reducing round trips and code complexity.

## 2026-03-01 - Drizzle nullsLast
**Learning:** The `.nullsLast()` method on `desc()` may not be available or cause type errors in some Drizzle versions/configurations.
**Action:** Remove `.nullsLast()` if the column is effectively non-nullable (e.g., `defaultNow()`), or use `sql` operator if strictly needed.
25 changes: 8 additions & 17 deletions server/storage.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {
type Playlist,
type InsertPlaylist,
} from "@shared/schema";
import { eq, desc, and, inArray, sql, getTableColumns } from "drizzle-orm";
import { eq, desc, asc, and, inArray, sql, getTableColumns } from "drizzle-orm";

export interface IStorage {
// Song CRUD
Expand Down Expand Up @@ -140,7 +140,7 @@ export class DatabaseStorage implements IStorage {
.from(songs)
.innerJoin(songLikes, eq(songs.id, songLikes.songId))
.where(eq(songLikes.userId, userId))
.orderBy(desc(songLikes.createdAt).nullsLast());
.orderBy(desc(songLikes.createdAt));
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot Feb 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Keep null timestamps at end of liked-song sorting

Dropping .nullsLast() from the ORDER BY desc(songLikes.createdAt) changes result ordering for any song_likes rows where created_at is NULL (PostgreSQL sorts NULL first on descending order), so affected users will see undated likes pinned above recent likes instead of true recency order. This is user-visible and can happen because song_likes.created_at is not declared NOT NULL in the schema, so legacy/imported rows can legally contain NULL.

Useful? React with 👍 / 👎.

}

// === Playlists ===
Expand All @@ -160,22 +160,13 @@ export class DatabaseStorage implements IStorage {
const playlist = await this.getPlaylist(id);
if (!playlist) return undefined;
Comment on lines 160 to 161
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Feb 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-critical critical

This method is missing an authorization check, creating a security vulnerability where any user could access any playlist if they guess the ID. The query should be scoped to the current user.

I recommend updating the function signature to accept a userId and using it to authorize access to the playlist. This change will also need to be reflected in the IStorage interface.

Suggested change
const playlist = await this.getPlaylist(id);
if (!playlist) return undefined;
// Note: The function signature needs to be updated to accept `userId`.
const [playlist] = await db.select().from(playlists).where(and(eq(playlists.id, id), eq(playlists.userId, userId)));
if (!playlist) return undefined;


const playlistSongRows = await db.select({ songId: playlistSongs.songId })
.from(playlistSongs)
.where(eq(playlistSongs.playlistId, id));

const songIds = playlistSongRows.map(r => r.songId).filter(id => typeof id === 'number' && !isNaN(id));

if (songIds.length === 0) {
return { ...playlist, songs: [] };
}

const songsResult = await db.select()
// Optimized: Single query with innerJoin to fetch songs directly, ordered by added time
// Replaces 3 separate queries (playlist -> IDs -> songs) and manual sorting
const songsList = await db.select(getTableColumns(songs))
.from(songs)
.where(inArray(songs.id, songIds));

const songMap = new Map(songsResult.map(s => [s.id, s]));
const songsList = songIds.map(id => songMap.get(id)).filter((s): s is Song => !!s);
.innerJoin(playlistSongs, eq(songs.id, playlistSongs.songId))
.where(eq(playlistSongs.playlistId, id))
.orderBy(asc(playlistSongs.addedAt));

return { ...playlist, songs: songsList };
}
Expand Down