From e918e5bd1da17cbba06a70d6ca946d11a87bffd2 Mon Sep 17 00:00:00 2001 From: codegirl-007 Date: Thu, 27 Aug 2026 00:22:07 -0700 Subject: [PATCH] Simplify post listing queries. Use named sqlc arguments, scope vote aggregation to selected roots, join viewer votes directly, and add the indexes and drift migration required by the resulting access paths. --- db/queries/posts.sql | 67 ++++++++++++---------- internal/store/migrate.go | 51 +++++++++++++++++ internal/store/migrate_posts_test.go | 84 ++++++++++++++++++++++++++++ internal/store/sqlc/posts.sql.go | 65 ++++++++++++--------- schema.sql | 3 + 5 files changed, 215 insertions(+), 55 deletions(-) diff --git a/db/queries/posts.sql b/db/queries/posts.sql index aa22b07..626e109 100644 --- a/db/queries/posts.sql +++ b/db/queries/posts.sql @@ -2,7 +2,18 @@ INSERT INTO posts ( id, parent_id, author_id, title, body, city, post_date, hidden, created_at, updated_at ) -VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10); +VALUES ( + sqlc.arg(id), + sqlc.arg(parent_id), + sqlc.arg(author_id), + sqlc.arg(title), + sqlc.arg(body), + sqlc.arg(city), + sqlc.arg(post_date), + sqlc.arg(hidden), + sqlc.arg(created_at), + sqlc.arg(updated_at) +); -- name: GetPost :one SELECT @@ -10,13 +21,13 @@ SELECT p.title, p.body, p.city, p.post_date, p.hidden, p.created_at, p.updated_at FROM posts p JOIN users u ON u.id = p.author_id -WHERE p.id = $1; +WHERE p.id = sqlc.arg(id); -- name: ListPostThread :many WITH RECURSIVE thread AS ( SELECT p.* FROM posts p - WHERE p.id = $1 AND p.parent_id IS NULL + WHERE p.id = sqlc.arg(root_id) AND p.parent_id IS NULL UNION ALL @@ -35,8 +46,10 @@ ORDER BY thread.created_at, thread.id; -- name: UpdatePost :execrows UPDATE posts -SET body = $2, updated_at = $3 -WHERE id = $1; +SET + body = sqlc.arg(body), + updated_at = sqlc.arg(updated_at) +WHERE id = sqlc.arg(id); -- name: ListRootPosts :many WITH RECURSIVE roots AS ( @@ -47,8 +60,9 @@ WITH RECURSIVE roots AS ( AND p.hidden = 0 ), thread AS ( - SELECT roots.id AS root_id, roots.id AS post_id, roots.author_id + SELECT roots.id AS root_id, child.id AS post_id, child.author_id FROM roots + JOIN posts child ON child.parent_id = roots.id UNION ALL @@ -57,15 +71,16 @@ thread AS ( JOIN posts child ON child.parent_id = thread.post_id ), answered AS ( - SELECT thread.root_id, bool_or(u.role = 'admin' AND thread.post_id <> thread.root_id) AS answered + SELECT DISTINCT thread.root_id FROM thread JOIN users u ON u.id = thread.author_id - GROUP BY thread.root_id + WHERE u.role = 'admin' ), scores AS ( - SELECT post_id, COALESCE(SUM(value), 0)::bigint AS score - FROM post_votes - GROUP BY post_id + SELECT votes.post_id, SUM(votes.value)::bigint AS score + FROM roots + JOIN post_votes votes ON votes.post_id = roots.id + GROUP BY votes.post_id ) SELECT roots.id, roots.parent_id, roots.author_id, @@ -73,17 +88,15 @@ SELECT roots.title, roots.body, roots.city, roots.post_date, roots.hidden, roots.created_at, roots.updated_at, COALESCE(scores.score, 0)::bigint AS score, - COALESCE(answered.answered, false)::bool AS answered, - COALESCE(( - SELECT post_votes.value - FROM post_votes - WHERE post_votes.user_id = sqlc.arg(viewer_id) - AND post_votes.post_id = roots.id - ), 0)::bigint AS user_vote + (answered.root_id IS NOT NULL)::bool AS answered, + COALESCE(viewer_vote.value, 0)::bigint AS user_vote FROM roots JOIN users u ON u.id = roots.author_id LEFT JOIN scores ON scores.post_id = roots.id LEFT JOIN answered ON answered.root_id = roots.id +LEFT JOIN post_votes viewer_vote + ON viewer_vote.user_id = sqlc.arg(viewer_id) + AND viewer_vote.post_id = roots.id ORDER BY score DESC, roots.created_at, roots.id LIMIT sqlc.arg(row_limit); @@ -91,22 +104,20 @@ LIMIT sqlc.arg(row_limit); SELECT EXISTS( SELECT 1 FROM posts - WHERE id = $1 AND parent_id IS NULL AND hidden = 0 + WHERE id = sqlc.arg(id) AND parent_id IS NULL AND hidden = 0 )::bool; -- name: DeletePostVote :exec DELETE FROM post_votes -WHERE user_id = $1 AND post_id = $2; +WHERE user_id = sqlc.arg(user_id) + AND post_id = sqlc.arg(post_id); -- name: UpsertPostVoteOnVisibleRoot :execrows INSERT INTO post_votes (user_id, post_id, value) -SELECT $1, $2, $3 +SELECT sqlc.arg(user_id), sqlc.arg(post_id), sqlc.arg(value) FROM posts p -WHERE p.id = $2 AND p.parent_id IS NULL AND p.hidden = 0 +WHERE p.id = sqlc.arg(post_id) + AND p.parent_id IS NULL + AND p.hidden = 0 ON CONFLICT (user_id, post_id) DO UPDATE -SET value = excluded.value -WHERE EXISTS ( - SELECT 1 - FROM posts p2 - WHERE p2.id = excluded.post_id AND p2.parent_id IS NULL AND p2.hidden = 0 -); +SET value = excluded.value; diff --git a/internal/store/migrate.go b/internal/store/migrate.go index fd5c10b..cc80253 100644 --- a/internal/store/migrate.go +++ b/internal/store/migrate.go @@ -104,6 +104,55 @@ ON CONFLICT (user_id, post_id) DO NOTHING`}, return nil } +func migratePostVoteIndex(ctx context.Context, exec execContext) error { + if _, err := exec.ExecContext(ctx, ` +CREATE INDEX IF NOT EXISTS idx_post_votes_post_id + ON post_votes(post_id)`); err != nil { + return fmt.Errorf("idx_post_votes_post_id: %w", err) + } + return nil +} + +func migratePostDate(ctx context.Context, exec execContext) error { + steps := []struct { + name string + sql string + }{ + {"rename post date", ` +DO $migration$ +BEGIN + IF EXISTS ( + SELECT 1 + FROM information_schema.columns + WHERE table_schema = current_schema() + AND table_name = 'posts' + AND column_name = 'hunt_date' + ) AND NOT EXISTS ( + SELECT 1 + FROM information_schema.columns + WHERE table_schema = current_schema() + AND table_name = 'posts' + AND column_name = 'post_date' + ) THEN + ALTER TABLE posts RENAME COLUMN hunt_date TO post_date; + END IF; +END +$migration$`}, + {"drop legacy root date index", ` +DROP INDEX IF EXISTS idx_posts_root_hunt`}, + {"create root date index", ` +CREATE INDEX IF NOT EXISTS idx_posts_root_date + ON posts(post_date, hidden) + WHERE parent_id IS NULL`}, + } + for _, step := range steps { + if _, err := exec.ExecContext(ctx, step.sql); err != nil { + return fmt.Errorf("%s: %w", step.name, err) + } + } + return nil +} + type execContext interface { ExecContext(ctx context.Context, query string, args ...any) (sql.Result, error) QueryContext(ctx context.Context, query string, args ...any) (*sql.Rows, error) @@ -151,6 +200,8 @@ CREATE TABLE IF NOT EXISTS schema_migrations ( {"002_user_profile_columns", migrateUserProfileColumns}, {"003_user_email", migrateUserEmail}, {"004_posts", migratePosts}, + {"005_post_vote_post_id_index", migratePostVoteIndex}, + {"006_post_date", migratePostDate}, } for _, m := range migrations { if applied[m.version] { diff --git a/internal/store/migrate_posts_test.go b/internal/store/migrate_posts_test.go index b4975d2..ff23031 100644 --- a/internal/store/migrate_posts_test.go +++ b/internal/store/migrate_posts_test.go @@ -97,6 +97,12 @@ VALUES ('homeowner', 'question-1', 1);`); err != nil { if err := migratePosts(ctx, conn); err != nil { t.Fatalf("migration is not idempotent: %v", err) } + if err := migratePostVoteIndex(ctx, conn); err != nil { + t.Fatal(err) + } + if err := migratePostVoteIndex(ctx, conn); err != nil { + t.Fatalf("post vote index migration is not idempotent: %v", err) + } var postCount, voteCount, legacyQuestionCount, legacyAnswerCount int if err := conn.QueryRowContext(ctx, "SELECT count(*) FROM posts").Scan(&postCount); err != nil { @@ -120,6 +126,23 @@ VALUES ('homeowner', 'question-1', 1);`); err != nil { legacyAnswerCount, ) } + var postVoteIndexCount int + if err := conn.QueryRowContext(ctx, ` +SELECT count(*) +FROM pg_indexes +WHERE schemaname = current_schema() + AND tablename = 'post_votes' + AND indexname = 'idx_post_votes_post_id'`).Scan(&postVoteIndexCount); err != nil { + t.Fatal(err) + } + if postVoteIndexCount != 1 { + t.Fatalf("post vote index count = %d, want 1", postVoteIndexCount) + } + if _, err := conn.ExecContext(ctx, ` +INSERT INTO post_votes (user_id, post_id, value) +VALUES ('homeowner', 'question-1', -1)`); err == nil { + t.Fatal("duplicate user/post vote unexpectedly succeeded") + } var rootParent sql.NullString var rootAuthor, title, rootBody, city, postDate, rootCreated, rootUpdated string @@ -210,6 +233,13 @@ INSERT INTO posts ( thread[2].ID != "follow-up" { t.Fatalf("recursive thread = %+v", thread) } + nonRootThread, err := queries.ListPostThread(ctx, "answer:question-1") + if err != nil { + t.Fatal(err) + } + if len(nonRootThread) != 0 { + t.Fatalf("non-root thread lookup returned %+v", nonRootThread) + } if n, err := queries.UpdatePost(ctx, sqlc.UpdatePostParams{ ID: "follow-up", Body: "The drip continues.", @@ -238,6 +268,60 @@ INSERT INTO posts ( roots[0].UserVote != 1 { t.Fatalf("root annotations = %+v", roots) } + + if _, err := conn.ExecContext(ctx, ` +DROP INDEX idx_posts_root_date; +ALTER TABLE posts RENAME COLUMN post_date TO hunt_date; +CREATE INDEX idx_posts_root_hunt + ON posts(hunt_date, hidden) + WHERE parent_id IS NULL;`); err != nil { + t.Fatal(err) + } + if err := migratePostDate(ctx, conn); err != nil { + t.Fatal(err) + } + if err := migratePostDate(ctx, conn); err != nil { + t.Fatalf("post date migration is not idempotent: %v", err) + } + + var postDateColumnCount, huntDateColumnCount, rootDateIndexCount, legacyIndexCount int + if err := conn.QueryRowContext(ctx, ` +SELECT + count(*) FILTER (WHERE column_name = 'post_date'), + count(*) FILTER (WHERE column_name = 'hunt_date') +FROM information_schema.columns +WHERE table_schema = current_schema() + AND table_name = 'posts'`).Scan(&postDateColumnCount, &huntDateColumnCount); err != nil { + t.Fatal(err) + } + if err := conn.QueryRowContext(ctx, ` +SELECT + count(*) FILTER (WHERE indexname = 'idx_posts_root_date'), + count(*) FILTER (WHERE indexname = 'idx_posts_root_hunt') +FROM pg_indexes +WHERE schemaname = current_schema() + AND tablename = 'posts'`).Scan(&rootDateIndexCount, &legacyIndexCount); err != nil { + t.Fatal(err) + } + var migratedPostDate string + if err := conn.QueryRowContext(ctx, ` +SELECT post_date FROM posts WHERE id = 'question-1'`).Scan(&migratedPostDate); err != nil { + t.Fatal(err) + } + if postDateColumnCount != 1 || + huntDateColumnCount != 0 || + rootDateIndexCount != 1 || + legacyIndexCount != 0 || + migratedPostDate != "2026-08-26" { + t.Fatalf( + "post date migration columns=%d legacy_columns=%d indexes=%d legacy_indexes=%d date=%q", + postDateColumnCount, + huntDateColumnCount, + rootDateIndexCount, + legacyIndexCount, + migratedPostDate, + ) + } } func TestMigratePostsReportsStep(t *testing.T) { diff --git a/internal/store/sqlc/posts.sql.go b/internal/store/sqlc/posts.sql.go index c0f647a..eaf7bca 100644 --- a/internal/store/sqlc/posts.sql.go +++ b/internal/store/sqlc/posts.sql.go @@ -14,7 +14,18 @@ const createPost = `-- name: CreatePost :exec INSERT INTO posts ( id, parent_id, author_id, title, body, city, post_date, hidden, created_at, updated_at ) -VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10) +VALUES ( + $1, + $2, + $3, + $4, + $5, + $6, + $7, + $8, + $9, + $10 +) ` type CreatePostParams struct { @@ -48,7 +59,8 @@ func (q *Queries) CreatePost(ctx context.Context, arg CreatePostParams) error { const deletePostVote = `-- name: DeletePostVote :exec DELETE FROM post_votes -WHERE user_id = $1 AND post_id = $2 +WHERE user_id = $1 + AND post_id = $2 ` type DeletePostVoteParams struct { @@ -142,8 +154,8 @@ type ListPostThreadRow struct { UpdatedAt string } -func (q *Queries) ListPostThread(ctx context.Context, id string) ([]ListPostThreadRow, error) { - rows, err := q.db.QueryContext(ctx, listPostThread, id) +func (q *Queries) ListPostThread(ctx context.Context, rootID string) ([]ListPostThreadRow, error) { + rows, err := q.db.QueryContext(ctx, listPostThread, rootID) if err != nil { return nil, err } @@ -187,8 +199,9 @@ WITH RECURSIVE roots AS ( AND p.hidden = 0 ), thread AS ( - SELECT roots.id AS root_id, roots.id AS post_id, roots.author_id + SELECT roots.id AS root_id, child.id AS post_id, child.author_id FROM roots + JOIN posts child ON child.parent_id = roots.id UNION ALL @@ -197,15 +210,16 @@ thread AS ( JOIN posts child ON child.parent_id = thread.post_id ), answered AS ( - SELECT thread.root_id, bool_or(u.role = 'admin' AND thread.post_id <> thread.root_id) AS answered + SELECT DISTINCT thread.root_id FROM thread JOIN users u ON u.id = thread.author_id - GROUP BY thread.root_id + WHERE u.role = 'admin' ), scores AS ( - SELECT post_id, COALESCE(SUM(value), 0)::bigint AS score - FROM post_votes - GROUP BY post_id + SELECT votes.post_id, SUM(votes.value)::bigint AS score + FROM roots + JOIN post_votes votes ON votes.post_id = roots.id + GROUP BY votes.post_id ) SELECT roots.id, roots.parent_id, roots.author_id, @@ -213,17 +227,15 @@ SELECT roots.title, roots.body, roots.city, roots.post_date, roots.hidden, roots.created_at, roots.updated_at, COALESCE(scores.score, 0)::bigint AS score, - COALESCE(answered.answered, false)::bool AS answered, - COALESCE(( - SELECT post_votes.value - FROM post_votes - WHERE post_votes.user_id = $1 - AND post_votes.post_id = roots.id - ), 0)::bigint AS user_vote + (answered.root_id IS NOT NULL)::bool AS answered, + COALESCE(viewer_vote.value, 0)::bigint AS user_vote FROM roots JOIN users u ON u.id = roots.author_id LEFT JOIN scores ON scores.post_id = roots.id LEFT JOIN answered ON answered.root_id = roots.id +LEFT JOIN post_votes viewer_vote + ON viewer_vote.user_id = $1 + AND viewer_vote.post_id = roots.id ORDER BY score DESC, roots.created_at, roots.id LIMIT $2 ` @@ -308,18 +320,20 @@ func (q *Queries) PostIsVisibleRoot(ctx context.Context, id string) (bool, error const updatePost = `-- name: UpdatePost :execrows UPDATE posts -SET body = $2, updated_at = $3 -WHERE id = $1 +SET + body = $1, + updated_at = $2 +WHERE id = $3 ` type UpdatePostParams struct { - ID string Body string UpdatedAt string + ID string } func (q *Queries) UpdatePost(ctx context.Context, arg UpdatePostParams) (int64, error) { - result, err := q.db.ExecContext(ctx, updatePost, arg.ID, arg.Body, arg.UpdatedAt) + result, err := q.db.ExecContext(ctx, updatePost, arg.Body, arg.UpdatedAt, arg.ID) if err != nil { return 0, err } @@ -330,14 +344,11 @@ const upsertPostVoteOnVisibleRoot = `-- name: UpsertPostVoteOnVisibleRoot :execr INSERT INTO post_votes (user_id, post_id, value) SELECT $1, $2, $3 FROM posts p -WHERE p.id = $2 AND p.parent_id IS NULL AND p.hidden = 0 +WHERE p.id = $2 + AND p.parent_id IS NULL + AND p.hidden = 0 ON CONFLICT (user_id, post_id) DO UPDATE SET value = excluded.value -WHERE EXISTS ( - SELECT 1 - FROM posts p2 - WHERE p2.id = excluded.post_id AND p2.parent_id IS NULL AND p2.hidden = 0 -) ` type UpsertPostVoteOnVisibleRootParams struct { diff --git a/schema.sql b/schema.sql index 5571cad..ff92e94 100644 --- a/schema.sql +++ b/schema.sql @@ -74,6 +74,9 @@ CREATE TABLE IF NOT EXISTS post_votes ( PRIMARY KEY (user_id, post_id) ); +CREATE INDEX IF NOT EXISTS idx_post_votes_post_id + ON post_votes(post_id); + CREATE TABLE IF NOT EXISTS sessions ( token TEXT PRIMARY KEY, data BYTEA NOT NULL,