Add map_version.folder_name column, derive filename#332
Conversation
Make the map folder name a real, indexable column. Previously only `filename` (e.g. `maps/scmp_001.v0001.zip`) was stored and the API derived the folder name from it in-memory, so `versions.folderName == X` filters could not be pushed to SQL and forced a full table scan on every map lookup. `folder_name` becomes the source of truth; `filename` is kept as a VIRTUAL generated column so faf-server, the replay server and existing API clients keep seeing the identical `maps/<folder>.zip` value. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 56 minutes and 11 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. 📝 WalkthroughWalkthroughA new SQL migration refactors the Changesmap_version Schema Refactor
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
filename is now a generated column, so test-data.sql must populate the new NOT NULL folder_name instead. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@migrations/V144__map_version_folder_name.sql`:
- Line 13: The folder_name column is set to varchar(200) but the generated
filename concatenates it with fixed characters (maps/ prefix and .zip suffix
totaling 9 characters), which can exceed the filename column's varchar(200)
limit if folder_name exceeds 191 characters. Change the folder_name column
definition in the ALTER TABLE map_version statement from varchar(200) to
varchar(191) to prevent potential overflow. Also apply the same reduction to the
folder_name column definitions at lines 22-23 and line 30 in this migration
file.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 15f3d33e-5559-44b5-ab11-d004948e31d1
📒 Files selected for processing (1)
migrations/V144__map_version_folder_name.sql
| -- and existing API clients continue to see the identical `maps/<folder>.zip` value. | ||
|
|
||
| -- 1. add the new source-of-truth column | ||
| ALTER TABLE map_version ADD COLUMN folder_name varchar(200) NULL AFTER filename; |
There was a problem hiding this comment.
Align column lengths to prevent derived filename overflow.
folder_name is allowed up to 200 chars, but generated filename is varchar(200) and adds 9 fixed chars (maps/ + .zip). Any folder_name longer than 191 can fail writes (or truncate, depending on SQL mode).
Suggested fix
-ALTER TABLE map_version ADD COLUMN folder_name varchar(200) NULL AFTER filename;
+ALTER TABLE map_version ADD COLUMN folder_name varchar(191) NULL AFTER filename;
...
ALTER TABLE map_version
- MODIFY folder_name varchar(200) NOT NULL,
+ MODIFY folder_name varchar(191) NOT NULL,
ADD UNIQUE KEY folder_name (folder_name);Also applies to: 22-23, 30-30
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@migrations/V144__map_version_folder_name.sql` at line 13, The folder_name
column is set to varchar(200) but the generated filename concatenates it with
fixed characters (maps/ prefix and .zip suffix totaling 9 characters), which can
exceed the filename column's varchar(200) limit if folder_name exceeds 191
characters. Change the folder_name column definition in the ALTER TABLE
map_version statement from varchar(200) to varchar(191) to prevent potential
overflow. Also apply the same reduction to the folder_name column definitions at
lines 22-23 and line 30 in this migration file.
Why
Every
/data/maprequest that filters onversions.folderName(e.g. the client's "find latest version by folder name" lookup) takes a flat ~5s. Root cause: in the APIfolderNameis a transient/computed attribute derived in-memory fromfilenamein a@PostLoadenricher, so Elide cannot translateversions.folderName == Xinto SQL. It falls back to loading and enriching the entiremap_versiontable on every request and filtering in memory.filename(maps/<folder>.zip) just stores the folder name with a fixed prefix/suffix. This migration makes the folder name a real, indexable column.What
folder_name(NOT NULL,UNIQUE) as the new source of truth and backfill it fromfilename(prefix/suffix-agnostic).filenamecolumn and re-add it as a VIRTUAL generated columnCONCAT('maps/', folder_name, '.zip')— no storage, recomputed on read.filenameis kept (generated) purely for backwards compatibility: faf-server (game_service,ladder_service, coop lookups), faf-rust-replayserver (get_game_stat_row) and any API clients reading thefilenameattribute keep seeing the identical value with no changes. ItsUNIQUEindex is dropped as redundant — it is a 1:1 function of the already-UNIQUEfolder_name.Coordinated change
The API side (entity mapping, upload service, enricher) lives in
faf-java-apion branchfeature/map-version-folder-name, which pins the migrations image tov144. This PR must be released asv144for that branch's integration tests to pass.🤖 Generated with Claude Code
Summary by CodeRabbit