Skip to content

db: drop duplicate team_api_keys hash index - #3407

Merged
ValentaTomas merged 1 commit into
mainfrom
drop-dup-team-api-keys-hash-index
Jul 27, 2026
Merged

ValentaTomas merged 1 commit into
mainfrom
drop-dup-team-api-keys-hash-index

Conversation

@ValentaTomas

@ValentaTomas ValentaTomas commented Jul 27, 2026

Copy link
Copy Markdown
Member

team_api_keys.api_key_hash carries two identical unique indexes: the UNIQUE constraint's own and a standalone copy. Identical indexes are interchangeable to the planner, so lookups continue on the constraint's index and uniqueness is untouched; every key write stops maintaining the second copy. The standalone twin is the droppable one — the constraint-backed index refuses DROP INDEX, making a mixed-up drop fail loud instead of silently.

Schema proof, both definitions in this repo: the standalone unique index (20250825102440 L5-L6) and the column's UNIQUE constraint that already carries its own index (20250211160814 L6) — identical key (api_key_hash). Background: PostgreSQL wiki — duplicate indexes.

@cla-bot cla-bot Bot added the cla-signed label Jul 27, 2026
@cursor

cursor Bot commented Jul 27, 2026

Copy link
Copy Markdown

PR Summary

Low Risk
Index-only change on API key storage; uniqueness stays on the constraint index, with concurrent drop and explicit timeouts to limit migration risk.

Overview
Adds a goose migration that removes the redundant standalone unique index idx_team_api_keys_api_key_hash on team_api_keys.api_key_hash, leaving the UNIQUE constraint’s backing index in place so lookups and uniqueness behave the same while writes no longer maintain a duplicate index.

The Up path uses DROP INDEX CONCURRENTLY with a 1h statement_timeout, then restores the migrator’s 3h default. The Down path drops any leftover index (including INVALID ones from interrupted builds) before recreating the standalone unique index concurrently.

Reviewed by Cursor Bugbot for commit ff183dd. Bugbot is set up for automated code reviews on this repo. Configure here.

@codecov

codecov Bot commented Jul 27, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

📢 Thoughts on this report? Let us know!

@ValentaTomas
ValentaTomas marked this pull request as ready for review July 27, 2026 04:13

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM. Small, well-formed goose migration that drops a genuinely redundant duplicate unique index.

Extended reasoning...

Overview

Single-file migration dropping idx_team_api_keys_api_key_hash, a standalone unique index that duplicates the index backing the UNIQUE constraint on team_api_keys.api_key_hash. I confirmed the duplication is real: the column was declared UNIQUE in migration 20250211160814_add_token_hashes.sql (creating the constraint's own backing index), and a later migration 20250825102440_add_hash_indexes.sql added the second, standalone unique index on the same column. This PR removes that second copy, leaving uniqueness enforcement on the constraint's index.

Security risks

None. This is a schema-only change to an index; it doesn't touch auth/permission logic, data exposure, or query behavior beyond dropping a redundant index. Uniqueness enforcement for api_key_hash is preserved via the constraint's own index.

Level of scrutiny

Low. This is a routine index-maintenance migration using DROP INDEX CONCURRENTLY (non-locking) with explicit statement_timeout bounds and a matching Down migration that recreates the index (with a defensive DROP ... IF EXISTS first to clean up any INVALID index left by an interrupted concurrent build). The 1h/3h statement_timeout pattern exactly mirrors the immediately preceding migration 20260725100500_env_builds_status_group_statistics.sql, and the 3h restore matches the migrator's per-connection default in scripts/migrator.go.

Other factors

The bug-hunting system found no issues, and the one candidate concern raised (statement_timeout scoping/restoration) was checked and ruled out — it follows the same established pattern already used elsewhere in this migrations directory. No outstanding reviewer comments need addressing; the two automated comments (Cursor, Codecov) are informational only.

@ValentaTomas
ValentaTomas merged commit be387e6 into main Jul 27, 2026
43 checks passed
@ValentaTomas
ValentaTomas deleted the drop-dup-team-api-keys-hash-index branch July 27, 2026 07:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants