Skip to content

Commit 2d49e5b

Browse files
tadasantclaude
tadasant
and
claude
authored
fix: prevent NULL status values during migration 008 (#587)
## Summary - Fixes migration 008 failing with "column contains null values" error - Adds COALESCE protection to handle 126 servers without status fields - Includes test utilities to reproduce and verify fixes with production data ## Problem Migration 008 was failing in production because: - 126 out of 781 servers have no `status` field in their JSON - When extracting `rec.value->>'status'`, these became NULL values - The `ALTER TABLE servers ALTER COLUMN status SET NOT NULL` constraint then failed ## Solution 1. **Prevent NULLs during migration**: Use COALESCE to provide 'active' default when status is NULL/empty 2. **Enhanced debugging**: Added RAISE NOTICE statements to log problematic values 3. **Safety check**: Double-check that throws exception if any NULLs remain 4. **Test utilities**: Created mirror_data tools to test migrations with real production data ## Testing Created and used `scripts/mirror_data/` tools to: - Fetch all 781 servers from production API - Load them into a test database - Verify the migration fix works correctly ### Results: - **Before fix**: 126 NULL status values created during migration - **After fix**: 0 NULL status values, all servers have valid status - Migration completes successfully with NOT NULL constraint ## Test plan - [x] Tested locally with production data using mirror_data tools - [x] Verified 0 NULL values after migration (vs 126 before) - [x] Confirmed NOT NULL constraint adds successfully - [ ] Deploy to staging and verify migration succeeds - [ ] Monitor production deployment 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude <[email protected]>
1 parent e39b008 commit 2d49e5b

File tree

5 files changed

+506
-2
lines changed

5 files changed

+506
-2
lines changed

‎internal/database/migrations/008_separate_official_metadata.sql‎

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,10 @@ BEGIN
3838
-- Note: status is at top level in old format, not in official_meta
3939
UPDATE servers
4040
SET
41-
status = rec.value->>'status',
41+
status = COALESCE(
42+
NULLIF(NULLIF(rec.value->>'status', ''), 'null'),
43+
'active'
44+
),
4245
published_at = COALESCE((official_meta->>'publishedAt')::TIMESTAMP WITH TIME ZONE, NOW()),
4346
updated_at = COALESCE((official_meta->>'updatedAt')::TIMESTAMP WITH TIME ZONE, NOW()),
4447
is_latest = COALESCE((official_meta->>'isLatest')::BOOLEAN, true)
@@ -47,7 +50,10 @@ BEGIN
4750
-- Handle records without official metadata (set defaults)
4851
UPDATE servers
4952
SET
50-
status = rec.value->>'status',
53+
status = COALESCE(
54+
NULLIF(NULLIF(rec.value->>'status', ''), 'null'),
55+
'active'
56+
),
5157
published_at = NOW(),
5258
updated_at = NOW(),
5359
is_latest = true
@@ -84,13 +90,41 @@ DROP FUNCTION migrate_official_metadata();
8490

8591
-- Safety check: Normalize invalid status values before adding NOT NULL constraints
8692
-- This handles: NULL (missing field), empty string, string "null", and any invalid values
93+
-- First, log how many records have problematic status values for debugging
94+
DO $$
95+
DECLARE
96+
null_count INTEGER;
97+
empty_count INTEGER;
98+
invalid_count INTEGER;
99+
BEGIN
100+
SELECT COUNT(*) INTO null_count FROM servers WHERE status IS NULL;
101+
SELECT COUNT(*) INTO empty_count FROM servers WHERE status = '' OR status = 'null';
102+
SELECT COUNT(*) INTO invalid_count FROM servers WHERE status IS NOT NULL AND status NOT IN ('active', 'deprecated', 'deleted', '', 'null');
103+
104+
IF null_count > 0 OR empty_count > 0 OR invalid_count > 0 THEN
105+
RAISE NOTICE 'Status cleanup: NULL=%, empty/null string=%, invalid=%', null_count, empty_count, invalid_count;
106+
END IF;
107+
END $$;
108+
109+
-- Fix all problematic status values
87110
UPDATE servers
88111
SET status = 'active'
89112
WHERE status IS NULL
90113
OR status = ''
91114
OR status = 'null'
92115
OR status NOT IN ('active', 'deprecated', 'deleted');
93116

117+
-- Double-check that no NULLs remain (this should never fail after the above)
118+
DO $$
119+
DECLARE
120+
remaining_nulls INTEGER;
121+
BEGIN
122+
SELECT COUNT(*) INTO remaining_nulls FROM servers WHERE status IS NULL;
123+
IF remaining_nulls > 0 THEN
124+
RAISE EXCEPTION 'Still have % NULL status values after cleanup!', remaining_nulls;
125+
END IF;
126+
END $$;
127+
94128
UPDATE servers SET published_at = NOW() WHERE published_at IS NULL;
95129

96130
-- Safety check: Ensure updated_at is never NULL

‎scripts/mirror_data/.gitignore‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
# Ignore downloaded production data
2+
production_servers.json

‎scripts/mirror_data/README.md‎

Lines changed: 237 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,237 @@
1+
# Mirror Production Data
2+
3+
> **Note:** These tools were created by Claude Code as a simple way to kick the tires on data migrations.
4+
> They are not intended for production use.
5+
6+
Tools to fetch and load production registry data for local testing and migration debugging.
7+
8+
## Overview
9+
10+
These scripts help you:
11+
1. Fetch all server data from the production registry API
12+
2. Load it into a local PostgreSQL database
13+
3. Test migrations against real production data
14+
15+
## Prerequisites
16+
17+
- Go 1.24.x
18+
- PostgreSQL (via Docker or local installation)
19+
- Required Go packages:
20+
```bash
21+
go get github.com/google/uuid github.com/lib/pq
22+
```
23+
24+
## Usage
25+
26+
### 1. Fetch Production Data
27+
28+
```bash
29+
# From the project root directory
30+
go run scripts/mirror_data/fetch_production_data.go
31+
```
32+
33+
This will:
34+
- Fetch all servers from https://registry.modelcontextprotocol.io/v0/servers
35+
- Handle pagination automatically
36+
- Save data to `scripts/mirror_data/production_servers.json`
37+
- Be respectful to the API with rate limiting
38+
39+
Output: `production_servers.json` containing all server records
40+
41+
### 2. Set Up Test Database
42+
43+
Start a PostgreSQL container for testing:
44+
45+
```bash
46+
docker run -d --name test-postgres \
47+
-p 5433:5432 \
48+
-e POSTGRES_PASSWORD=testpass \
49+
-e POSTGRES_DB=registry_test \
50+
postgres:16
51+
```
52+
53+
Or reset an existing database:
54+
55+
```bash
56+
docker exec test-postgres psql -U postgres postgres -c "DROP DATABASE IF EXISTS registry_test;"
57+
docker exec test-postgres psql -U postgres postgres -c "CREATE DATABASE registry_test;"
58+
```
59+
60+
### 3. Load Production Data
61+
62+
```bash
63+
# From the project root directory
64+
go run scripts/mirror_data/load_production_data.go
65+
```
66+
67+
This will:
68+
1. Connect to PostgreSQL at `localhost:5433`
69+
2. Apply migrations up to a configurable point (default: migration 007)
70+
3. Load all production server data
71+
4. Analyze the data and report statistics
72+
5. Show sample servers with NULL status values
73+
74+
To test a different migration, edit `maxMigration` in the script (line 24)
75+
76+
### 4. Test Migrations
77+
78+
After loading the data, you can test migrations against real production data.
79+
80+
#### Testing a Single Migration
81+
82+
```bash
83+
# Test migration 008
84+
cat internal/database/migrations/008_separate_official_metadata.sql | \
85+
docker exec -i test-postgres psql -U postgres -d registry_test
86+
```
87+
88+
#### Debugging Failed Migrations
89+
90+
1. **Check the error output:**
91+
```bash
92+
# Run migration and capture all output
93+
cat internal/database/migrations/008_separate_official_metadata.sql | \
94+
docker exec -i test-postgres psql -U postgres -d registry_test 2>&1 | \
95+
grep -E "(ERROR|NOTICE|WARNING)"
96+
```
97+
98+
2. **Inspect data before migration:**
99+
```bash
100+
# Check for NULL values that might cause issues
101+
docker exec test-postgres psql -U postgres -d registry_test -c \
102+
"SELECT COUNT(*) FROM servers WHERE value->>'status' IS NULL;"
103+
104+
# Find sample problematic records
105+
docker exec test-postgres psql -U postgres -d registry_test -c \
106+
"SELECT value->>'name', value->>'version' FROM servers \
107+
WHERE value->>'status' IS NULL LIMIT 5;"
108+
```
109+
110+
3. **Test partial migrations:**
111+
```bash
112+
# Extract and run only part of a migration to isolate issues
113+
# For example, run only up to the NOT NULL constraint:
114+
docker exec test-postgres psql -U postgres -d registry_test <<EOF
115+
BEGIN;
116+
-- Paste migration SQL up to the problematic line
117+
-- Check intermediate state
118+
SELECT COUNT(*) FROM servers WHERE status IS NULL;
119+
ROLLBACK; -- or COMMIT if testing further
120+
EOF
121+
```
122+
123+
#### Testing Migration Rollback
124+
125+
```bash
126+
# Start a transaction to test and rollback
127+
docker exec test-postgres psql -U postgres -d registry_test <<EOF
128+
BEGIN;
129+
\i /dev/stdin < internal/database/migrations/008_separate_official_metadata.sql
130+
-- Inspect the results
131+
\d servers
132+
SELECT COUNT(*) FROM servers;
133+
ROLLBACK; -- Undo all changes
134+
EOF
135+
```
136+
137+
#### Iterative Testing Workflow
138+
139+
1. **Reset database to clean state:**
140+
```bash
141+
docker exec test-postgres psql -U postgres postgres -c "DROP DATABASE IF EXISTS registry_test;"
142+
docker exec test-postgres psql -U postgres postgres -c "CREATE DATABASE registry_test;"
143+
go run scripts/mirror_data/load_production_data.go
144+
```
145+
146+
2. **Test your migration changes:**
147+
```bash
148+
cat internal/database/migrations/008_separate_official_metadata.sql | \
149+
docker exec -i test-postgres psql -U postgres -d registry_test
150+
```
151+
152+
3. **Verify the results:**
153+
```bash
154+
# Check final table structure
155+
docker exec test-postgres psql -U postgres -d registry_test -c "\d servers"
156+
157+
# Verify data integrity
158+
docker exec test-postgres psql -U postgres -d registry_test -c \
159+
"SELECT COUNT(*), COUNT(DISTINCT server_name) FROM servers;"
160+
```
161+
162+
#### Common Migration Issues to Check
163+
164+
- **NULL values:** Check columns before adding NOT NULL constraints
165+
- **Unique constraints:** Verify no duplicates exist before adding
166+
- **Check constraints:** Validate all existing data matches the constraint
167+
- **Data type changes:** Ensure all values can be cast to new type
168+
- **Foreign keys:** Verify referential integrity before adding
169+
170+
### 5. Clean Up
171+
172+
```bash
173+
docker stop test-postgres
174+
docker rm test-postgres
175+
```
176+
177+
## Configuration
178+
179+
The `load_production_data.go` script connects to:
180+
- Host: `localhost`
181+
- Port: `5433`
182+
- Database: `registry_test`
183+
- User: `postgres`
184+
- Password: `testpass`
185+
186+
Modify line 17 in the script if you need different connection parameters.
187+
188+
## Example Output
189+
190+
When running `load_production_data.go`:
191+
192+
```
193+
Running migrations 001-007...
194+
Applying migration 1: 001_initial_schema.sql
195+
Applying migration 2: 002_add_server_extensions.sql
196+
...
197+
198+
Loading production data...
199+
Loading 779 servers...
200+
Data loaded successfully!
201+
202+
Total servers in database: 779
203+
204+
Analyzing status field in JSON data...
205+
Total servers: 779
206+
NULL status: 126
207+
Empty status: 0
208+
'null' string status: 0
209+
'active' status: 652
210+
'deprecated' status: 0
211+
'deleted' status: 0
212+
Other/Invalid: 1
213+
214+
Sample servers with NULL status:
215+
- com.biodnd/[email protected]
216+
- io.github.jkakar/[email protected]
217+
...
218+
219+
Database is ready for testing migration 008!
220+
```
221+
222+
## Troubleshooting
223+
224+
### Port already in use
225+
If port 5433 is already in use, either:
226+
- Stop the existing service using that port
227+
- Use a different port in both the Docker command and the Go script
228+
229+
### Connection refused
230+
Ensure the PostgreSQL container is running and healthy:
231+
```bash
232+
docker ps | grep test-postgres
233+
docker logs test-postgres
234+
```
235+
236+
### Migration failures
237+
The script stops at migration 007 intentionally. To test migration 008, run it manually after the data is loaded.

‎scripts/mirror_data/fetch_production_data.go‎

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
// This tool was created by Claude Code as a simple way to kick the tires on data migrations
2+
// by fetching production data from the public registry API.
3+
// It is not intended for production use.
4+
//
5+
//go:build ignore
6+
7+
package main
8+
9+
import (
10+
"encoding/json"
11+
"fmt"
12+
"io"
13+
"net/http"
14+
"os"
15+
"time"
16+
)
17+
18+
type ServerResponse struct {
19+
Servers []json.RawMessage `json:"servers"`
20+
Metadata struct {
21+
NextCursor string `json:"next_cursor,omitempty"`
22+
Count int `json:"count"`
23+
} `json:"metadata"`
24+
}
25+
26+
func main() {
27+
baseURL := "https://registry.modelcontextprotocol.io/v0/servers"
28+
var allServers []json.RawMessage
29+
cursor := ""
30+
pageCount := 0
31+
32+
for {
33+
pageCount++
34+
url := baseURL
35+
if cursor != "" {
36+
url = fmt.Sprintf("%s?cursor=%s", baseURL, cursor)
37+
}
38+
39+
fmt.Printf("Fetching page %d: %s\n", pageCount, url)
40+
41+
resp, err := http.Get(url)
42+
if err != nil {
43+
panic(fmt.Sprintf("Failed to fetch: %v", err))
44+
}
45+
defer resp.Body.Close()
46+
47+
body, err := io.ReadAll(resp.Body)
48+
if err != nil {
49+
panic(fmt.Sprintf("Failed to read body: %v", err))
50+
}
51+
52+
var serverResp ServerResponse
53+
if err := json.Unmarshal(body, &serverResp); err != nil {
54+
panic(fmt.Sprintf("Failed to parse JSON: %v", err))
55+
}
56+
57+
allServers = append(allServers, serverResp.Servers...)
58+
fmt.Printf(" Got %d servers (total: %d)\n", len(serverResp.Servers), len(allServers))
59+
60+
if serverResp.Metadata.NextCursor == "" {
61+
break
62+
}
63+
cursor = serverResp.Metadata.NextCursor
64+
65+
// Be nice to the API
66+
time.Sleep(100 * time.Millisecond)
67+
}
68+
69+
// Save all servers to a file
70+
output := map[string]interface{}{
71+
"servers": allServers,
72+
"count": len(allServers),
73+
"fetched": time.Now().Format(time.RFC3339),
74+
}
75+
76+
data, err := json.MarshalIndent(output, "", " ")
77+
if err != nil {
78+
panic(fmt.Sprintf("Failed to marshal output: %v", err))
79+
}
80+
81+
outputFile := "production_servers.json"
82+
if err := os.WriteFile(outputFile, data, 0644); err != nil {
83+
panic(fmt.Sprintf("Failed to write file: %v", err))
84+
}
85+
86+
fmt.Printf("\nDone! Saved %d servers to %s\n", len(allServers), outputFile)
87+
}

0 commit comments

Comments
 (0)