diff --git a/REFINED_SCHEMA_PLAN.md b/REFINED_SCHEMA_PLAN.md index 205da4d..2c08f04 100644 --- a/REFINED_SCHEMA_PLAN.md +++ b/REFINED_SCHEMA_PLAN.md @@ -63,12 +63,15 @@ **Decision:** Line-by-line stream scanning with regex pattern matching. **Rationale:** -- **Performance:** 2x faster for 959-line file (50ms vs 100ms) +- **Performance:** 2x faster for 960-line file (50ms vs 100ms) - **Memory:** Only current line in memory at once - **Maintainability:** Simple `bufio.Scanner` pattern - **Debugging:** Can log which line contains which table +- **Defensive:** Handles both `IF NOT EXISTS` and legacy table statements -**Pattern:** `regexp.MustCompile(`CREATE TABLE IF NOT EXISTS (?:\w+\.)?(\w+)`)` +**Pattern:** `regexp.MustCompile(`CREATE TABLE (?:IF NOT EXISTS )?(?:\w+\.)?(\w+)`)` + +**Note:** Pattern matches both CREATE TABLE formats to support Phase 1 transition and prevent silent skips if tables lack IF NOT EXISTS clause. --- @@ -111,6 +114,11 @@ database/ # Inserts needing ON CONFLICT (2): # - Line 14: library_types (name) # - Line 44: system_settings (setting_key) + +# Already idempotent (no changes needed): +# - 8 ALTER TABLE statements (all have IF NOT EXISTS) +# - 6 CREATE FUNCTION statements (all use OR REPLACE) +# - 1 INSERT statement (system_config has ON CONFLICT) ``` #### Step 1.2: Verify Schema Syntax @@ -144,6 +152,7 @@ import ( "log" "regexp" "strings" + "time" _ "embed" // Used for go:embed "github.com/jackc/pgx/v5" @@ -171,9 +180,10 @@ func generateLockID(key string) int64 { // parseTableNames extracts all table names from CREATE TABLE statements // Uses stream scanning for performance and memory efficiency +// Handles both "CREATE TABLE" and "CREATE TABLE IF NOT EXISTS" formats func parseTableNames() ([]string, error) { scanner := bufio.NewScanner(strings.NewReader(SchemaFile)) - pattern := regexp.MustCompile(`CREATE TABLE IF NOT EXISTS (?:\w+\.)?(\w+)`) + pattern := regexp.MustCompile(`CREATE TABLE (?:IF NOT EXISTS )?(?:\w+\.)?(\w+)`) tables := make(map[string]bool) for scanner.Scan() { @@ -239,6 +249,47 @@ func verifyTables(ctx context.Context, db *pgxpool.Pool, expectedTables []string return nil } +// verifyFunctions checks all expected functions exist in database +func verifyFunctions(ctx context.Context, db *pgxpool.Pool, expectedFunctions []string) error { + rows, err := db.Query(ctx, ` + SELECT routine_name + FROM information_schema.routines + WHERE routine_schema = 'public' + AND routine_type = 'FUNCTION' + `) + if err != nil { + return fmt.Errorf("failed to query existing functions: %w", err) + } + defer rows.Close() + + existingFunctions := make(map[string]bool) + for rows.Next() { + var functionName string + if err := rows.Scan(&functionName); err != nil { + return fmt.Errorf("failed to scan function name: %w", err) + } + existingFunctions[functionName] = true + } + + if err := rows.Err(); err != nil { + return fmt.Errorf("error iterating functions: %w", err) + } + + // Check all expected functions exist + var missing []string + for _, expected := range expectedFunctions { + if !existingFunctions[expected] { + missing = append(missing, expected) + } + } + + if len(missing) > 0 { + return fmt.Errorf("missing functions: %s", strings.Join(missing, ", ")) + } + + return nil +} + // executeSchema runs the entire schema.sql in a single transaction func executeSchema(ctx context.Context, db *pgxpool.Pool) error { tx, err := db.Begin(ctx) @@ -271,20 +322,23 @@ func Initialize(ctx context.Context, db *pgxpool.Pool) error { } defer conn.Release() - // Get advisory lock (blocks other instances) + // Get advisory lock with timeout (prevents indefinite hangs) log.Printf("DEBUG: Attempting to acquire advisory lock %d...", schemaInitLockID) + lockCtx, cancel := context.WithTimeout(ctx, 30*time.Second) + defer cancel() + var lockAcquired bool - err = conn.QueryRow(ctx, "SELECT pg_try_advisory_lock($1)", schemaInitLockID).Scan(&lockAcquired) + err = conn.QueryRow(lockCtx, "SELECT pg_try_advisory_lock($1)", schemaInitLockID).Scan(&lockAcquired) if err != nil { return fmt.Errorf("failed to acquire advisory lock: %w", err) } if !lockAcquired { log.Println("⏳ Another instance is initializing schema, waiting...") - // Use pg_advisory_lock instead (blocks until available) - _, err = conn.Exec(ctx, "SELECT pg_advisory_lock($1)", schemaInitLockID) + // Use pg_advisory_lock instead (blocks until available or timeout) + _, err = conn.Exec(lockCtx, "SELECT pg_advisory_lock($1)", schemaInitLockID) if err != nil { - return fmt.Errorf("failed to wait for advisory lock: %w", err) + return fmt.Errorf("timeout waiting for advisory lock: %w", err) } } log.Println("✅ Advisory lock acquired") @@ -307,6 +361,16 @@ func Initialize(ctx context.Context, db *pgxpool.Pool) error { } log.Printf("DEBUG: Found %d expected tables in schema.sql", len(expectedTables)) + // Critical functions that must exist for app to function + criticalFunctions := []string{ + "update_updated_at_column", + "detect_format_group", + "convert_progress", + "detect_conflict", + "merge_progress", + "bulk_update_progress_from_koreader", + } + // Execute schema in a single transaction log.Println("🔧 Executing schema.sql in transaction...") err = executeSchema(ctx, db) @@ -325,7 +389,17 @@ func Initialize(ctx context.Context, db *pgxpool.Pool) error { } log.Println("✅ All expected tables verified") - log.Printf("✅ Database schema initialization complete (%d tables verified)", len(expectedTables)) + // Verify critical functions exist + log.Println("🔍 Verifying critical functions exist...") + err = verifyFunctions(ctx, db, criticalFunctions) + if err != nil { + log.Printf("ERROR: Function verification failed: %v", err) + return fmt.Errorf("function verification failed: %w", err) + } + log.Println("✅ All critical functions verified") + + log.Printf("✅ Database schema initialization complete (%d tables, %d functions verified)", + len(expectedTables), len(criticalFunctions)) return nil } ``` @@ -389,18 +463,24 @@ func main() { - [ ] Converted 18 CREATE TABLE → CREATE TABLE IF NOT EXISTS (27 total) - [ ] Converted 62 CREATE INDEX → CREATE INDEX IF NOT EXISTS (82 total) - [ ] Added ON CONFLICT to 2 INSERT statements (3 total) +- [ ] Verified 8 ALTER TABLE statements have IF NOT EXISTS (already done) +- [ ] Verified 6 CREATE FUNCTION statements use OR REPLACE (already done) - [ ] Verified schema.sql syntax is valid - [ ] Confirmed single schema.sql exists as source of truth **Phase 2 Verification:** - [ ] `internal/database/schema.go` compiles without errors - [ ] Single file with all schema initialization functions -- [ ] Imports only what's used (bufio, embed, pgx v5) +- [ ] Imports only what's used (bufio, context, embed, fmt, log, regexp, strings, time, pgx v5) - [ ] Uses *pgxpool.Pool concrete type - [ ] Pre-computed constant for lock ID (7804706162000639061) - [ ] Stream scanning implementation for performance +- [ ] Regex pattern handles both IF NOT EXISTS and legacy table formats +- [ ] Lock acquisition has 30-second timeout to prevent indefinite hangs - [ ] Successfully embeds existing schema.sql file - [ ] No schema file duplication (single source of truth) +- [ ] Verifies critical functions exist, not just tables +- [ ] Provides clear error messages for missing schema components **Phase 3 Verification:** - [ ] `cmd/server/main.go` compiles without errors @@ -429,11 +509,12 @@ func main() { ### What Will Work: ✅ **App starts successfully** on fresh database ✅ **App starts successfully** on existing database -✅ **Multiple instances can start** simultaneously with advisory locks +✅ **Multiple instances can start** simultaneously with advisory locks (30s timeout) ✅ **TestKoboInitialization and all tests pass** -✅ **Clear log messages** show schema initialization +✅ **Clear log messages** show schema initialization with table/function counts ✅ **Production-safe** with no race conditions -✅ **Self-healing** from partial/corrupted state +✅ **Self-healing** from partial/corrupted state (missing tables or functions) +✅ **Better error messages** indicate which schema components are missing ### What Will NOT Change: ✅ **No changes to API endpoints** @@ -448,6 +529,7 @@ func main() { - **HTTP request handling:** No change - **Database queries:** No change - **Memory usage:** Minimal increase (schema parsing optimization) +- **Lock timeout:** 30-second safety prevents indefinite hangs --- @@ -482,7 +564,7 @@ The app will initialize the database automatically on first startup. **Monitor logs for:** ``` -✅ Database schema initialized and verified (27 tables verified) +✅ Database schema initialization complete (27 tables, 6 functions verified) ``` ### Subsequent Deployments @@ -515,6 +597,14 @@ podman logs bookhoard_app | grep -i schema podman exec bookhoard_db psql -U postgres -d bookhoard -c "\dt" ``` +### Verify functions exist: +```bash +podman exec bookhoard_db psql -U postgres -d bookhoard -c " + SELECT routine_name FROM information_schema.routines + WHERE routine_schema = 'public' AND routine_type = 'FUNCTION'; +" +``` + ### Check advisory locks: ```bash podman exec bookhoard_db psql -U postgres -d bookhoard -c "