-- 07_corrected_functions.sql -- Corrected versions of all four utility functions from the change request. -- Each fix is annotated with the issue number it addresses (see REVIEW.md). -- ── Fix for cleanup_expired_artifacts ──────────────────────────────────────── -- Issue 4: Added partition_key to WHERE and IN-list for partition pruning. -- Issue 4: Replaced FOR UPDATE with FOR UPDATE SKIP LOCKED to prevent -- stacking behind a concurrent cleanup job. -- Bonus: RETURNING clause allows callers to log what was actually deleted. CREATE OR REPLACE FUNCTION demo.cleanup_expired_artifacts( p_batch_size INT DEFAULT 10000 ) RETURNS INT AS $$ DECLARE v_deleted INT; BEGIN DELETE FROM demo.ci_job_artifacts WHERE (id, partition_key) IN ( SELECT id, partition_key FROM demo.ci_job_artifacts WHERE expire_at < NOW() AND locked = FALSE AND partition_key < DATE_TRUNC('month', NOW())::DATE -- prune to old partitions only ORDER BY expire_at ASC LIMIT p_batch_size FOR UPDATE SKIP LOCKED ); GET DIAGNOSTICS v_deleted = ROW_COUNT; RAISE NOTICE 'Cleaned up % expired artifacts', v_deleted; RETURN v_deleted; END; $$ LANGUAGE plpgsql; -- ── Fix for terminate_idle_sessions ────────────────────────────────────────── -- Issue 5: Added filter to exclude PgBouncer pooler connections. -- In transaction-mode PgBouncer the backend appears idle between -- client transactions. Terminating it breaks the pool and causes -- "SSL connection has been closed unexpectedly" for active clients. -- Bonus: filter also excludes replication and background workers. CREATE OR REPLACE FUNCTION demo.terminate_idle_sessions( p_idle_threshold INTERVAL DEFAULT '10 minutes', p_max_terminated INT DEFAULT 50, p_pgbouncer_app TEXT DEFAULT 'pgbouncer' -- set to your app_name ) RETURNS TABLE(pid INT, username TEXT, idle_duration INTERVAL, query TEXT) AS $$ BEGIN RETURN QUERY WITH idle_sessions AS ( SELECT a.pid, a.usename::TEXT, NOW() - a.state_change AS duration, a.query FROM pg_stat_activity a WHERE a.state = 'idle' AND NOW() - a.state_change > p_idle_threshold AND a.pid <> pg_backend_pid() AND a.backend_type = 'client backend' AND a.application_name NOT ILIKE '%' || p_pgbouncer_app || '%' AND a.client_addr IS NOT NULL -- exclude unix socket (local tools) ORDER BY duration DESC LIMIT p_max_terminated ) SELECT s.pid, s.usename, s.duration, s.query FROM idle_sessions s WHERE pg_terminate_backend(s.pid); END; $$ LANGUAGE plpgsql; -- ── Fix for calculate_namespace_storage ────────────────────────────────────── -- Issue 6: Cache race condition fixed with FOR UPDATE SKIP LOCKED. -- Callers that can't acquire the cache row lock return the stale -- cached value rather than all recomputing simultaneously. -- Issue 7: Recursive CTE depth limit enforced (max 20 levels). -- Requires tables: namespaces(id, parent_id), project_storages(namespace_id, storage_bytes), -- namespace_storage_cache(namespace_id, total_bytes, calculated_at) -- (These don't exist in our demo schema; this is illustrative code only.) CREATE OR REPLACE FUNCTION demo.calculate_namespace_storage( p_namespace_id BIGINT ) RETURNS BIGINT AS $$ DECLARE v_total_bytes BIGINT := 0; v_cached_bytes BIGINT; v_cached_at TIMESTAMP; BEGIN -- Attempt to lock the cache row exclusively. -- SKIP LOCKED means concurrent callers return the stale value immediately -- instead of piling up and all recomputing the expensive CTE. SELECT total_bytes, calculated_at INTO v_cached_bytes, v_cached_at FROM namespace_storage_cache WHERE namespace_id = p_namespace_id FOR UPDATE SKIP LOCKED; -- Cache hit: return without recomputing IF v_cached_at IS NOT NULL AND v_cached_at > NOW() - INTERVAL '1 hour' THEN RETURN v_cached_bytes; END IF; -- Cache miss or expired: recompute with depth-limited recursive CTE. -- Issue 7 fix: depth column added to prevent infinite recursion on cycles. WITH RECURSIVE ns_tree AS ( SELECT id, 0 AS depth FROM namespaces WHERE id = p_namespace_id UNION ALL SELECT n.id, t.depth + 1 FROM namespaces n JOIN ns_tree t ON n.parent_id = t.id WHERE t.depth < 20 -- hard cap; cyclic namespaces stop here ) SELECT COALESCE(SUM(ps.storage_bytes), 0) INTO v_total_bytes FROM ns_tree nt JOIN project_storages ps ON ps.namespace_id = nt.id; INSERT INTO namespace_storage_cache (namespace_id, total_bytes, calculated_at) VALUES (p_namespace_id, v_total_bytes, NOW()) ON CONFLICT (namespace_id) DO UPDATE SET total_bytes = EXCLUDED.total_bytes, calculated_at = EXCLUDED.calculated_at; RETURN v_total_bytes; END; $$ LANGUAGE plpgsql; -- ── Fix for database_health view ───────────────────────────────────────────── -- Issue 8: NULLIF prevents division by zero if n_live_tup is ever 0. -- The original WHERE n_live_tup > 1000 makes the crash unlikely in practice, -- but this is fragile to future refactoring that removes the filter. CREATE OR REPLACE VIEW demo.database_health AS SELECT schemaname, relname AS table_name, n_live_tup AS live_rows, n_dead_tup AS dead_rows, ROUND(n_dead_tup::NUMERIC / NULLIF(n_live_tup, 0) * 100, 2) AS dead_row_pct, last_vacuum, last_autovacuum, last_analyze, pg_size_pretty(pg_total_relation_size(schemaname || '.' || relname)) AS total_size FROM pg_stat_user_tables WHERE n_live_tup > 1000 ORDER BY n_dead_tup DESC;