-- +goose Up
-- Two corrections to schema that is already live: three indexes that duplicate indexes 0048 had already
-- created, and the one user reference in the whole schema that was never pinned to a user.

-- ---------------------------------------------------------------------------
-- 1. Three duplicate indexes on the pipeline's hottest write path.
--
-- Checked column by column against 0048 before dropping anything. Same table, same columns, same order,
-- same direction, and no WHERE clause on either side of any pair, so each pair is byte for byte the same
-- index under two names:
--
--   idx_applications_req_stage (0054:261) equals idx_applications_req (0048:75)
--       applications (organization_id, requisition_id, stage)
--   idx_applications_cand (0054:262) equals idx_applications_candidate (0048:76)
--       applications (organization_id, candidate_id)
--   idx_ase_app_time (0054:264) equals idx_ase_application (0048:112)
--       application_stage_events (organization_id, application_id, occurred_at)
--
-- Nothing errored when the second set was created because Postgres identifies an index by name and never
-- compares definitions, so a duplicate under a new name is created silently and then maintained forever.
--
-- The fourth index 0054 created, idx_applications_furthest on (organization_id, furthest_stage_ord), is
-- NOT a duplicate. furthest_stage_ord is a column 0054 itself added and no 0048 index covers it, so it
-- stays.
--
-- The cost is paid on writes, and it lands on the busiest transaction in the product. A single stage move
-- inserts one application_stage_events row and updates one applications row, and each of those maintained
-- two identical btrees instead of one: double the index insertions, double the WAL those insertions
-- generate, double the pages dirtied, double the eventual bloat and the vacuum work to reclaim it, and
-- twice the shared buffer space held by index pages that answer the same question. Reads gained nothing in
-- return, because a scan uses one index and the planner was simply choosing between two copies. The board
-- refresh, the funnel and every phase 17 duration metric all sit on these three reads, so this is the path
-- least able to afford the waste.
--
-- The 0048 names survive and the 0054 names are dropped, so the remaining index is the one whose creating
-- migration also owns its rollback.
DROP INDEX IF EXISTS idx_applications_req_stage;
DROP INDEX IF EXISTS idx_applications_cand;
DROP INDEX IF EXISTS idx_ase_app_time;

-- ---------------------------------------------------------------------------
-- 2. term_expansions.created_by, the only user reference in the schema with no foreign key.
--
-- 0044 declared it as a bare uuid. Every other table that records who did something pins that reference
-- with a composite key leading with organization_id, so it cannot resolve to a user in another tenant, and
-- 0053 sets out why such a key must name its column in the SET NULL clause. Unpinned, created_by can hold
-- any uuid at all, including one belonging to a user in a different organization, and it goes on pointing
-- at a user id long after that user is deleted, which is a dangling reference that any future join treats
-- as authorship.
--
-- The nullable organization_id is the wrinkle and it is worth being exact about it. The default matching
-- rule is MATCH SIMPLE, so the constraint is tested only when BOTH key columns are non null:
--
--   organization_id present and created_by present: enforced, and that is every tenant authored row
--   created_by NULL: not tested, correctly, because authorship is simply unknown
--   organization_id NULL and created_by present: not tested, the residual gap
--
-- That third case is accepted rather than papered over. organization_id IS NULL means a global seed row,
-- and the 0044 RLS insert policy requires organization_id to equal the calling tenant, so the application
-- role cannot create a global row at all. Only the migrator can, and the seed rows in 0044 and 0045 are
-- inserted with alias and expansion only, so they carry no created_by to dangle. MATCH FULL would close
-- the gap, but it demands that either all key columns are NULL or none are, which would forbid the tenant
-- row that legitimately has no recorded author. That is the common case, so MATCH FULL would trade a real
-- capability for a hole no reachable code path can open.
--
-- Any created_by that does not resolve to a real user in the same organization is nulled first, or the
-- ALTER aborts on the first such row. The predicate matches exactly what the constraint will test, so a
-- global seed row is never touched: a row MATCH SIMPLE will never check is left exactly as it is.
UPDATE term_expansions t
   SET created_by = NULL
 WHERE t.organization_id IS NOT NULL
   AND t.created_by IS NOT NULL
   AND NOT EXISTS (SELECT 1 FROM users u
                    WHERE u.organization_id = t.organization_id
                      AND u.id              = t.created_by);

-- SET NULL rather than CASCADE, for the same reason user_tool_access.granted_by is SET NULL in 0062: the
-- reference is an audit note, not the reason the row exists. An expansion outlives the person who added
-- it, and deleting the recruiter who typed one acronym must never quietly remove a term the whole tenant's
-- matching depends on.
--
-- The column list form is required. A bare SET NULL would null organization_id as well, and because
-- organization_id is NULLABLE on this one table it would not even fail loudly the way 0053 describes: the
-- row would become a global seed row, visible to every tenant in the deployment. Silent cross tenant
-- disclosure is a worse outcome than the aborted delete 0053 repaired, so this key never gets the bare
-- form.
ALTER TABLE term_expansions ADD CONSTRAINT term_expansions_created_by_fkey
    FOREIGN KEY (organization_id, created_by) REFERENCES users (organization_id, id)
    ON DELETE SET NULL (created_by);

-- +goose Down
ALTER TABLE term_expansions DROP CONSTRAINT IF EXISTS term_expansions_created_by_fkey;
-- The created_by values nulled above are not restored. They pointed at users that do not exist, so there
-- is nothing to restore them to, and inventing an author is worse than an absent one. Nothing reads the
-- column today, so nothing regresses.

-- The duplicates come back, because a down restores the shape the previous migration left. IF NOT EXISTS
-- on all three: 0054's own down drops these same three names, so an operator rolling 0067 back and then
-- 0054 back must not be stopped by either half having already happened.
CREATE INDEX IF NOT EXISTS idx_ase_app_time
    ON application_stage_events (organization_id, application_id, occurred_at);
CREATE INDEX IF NOT EXISTS idx_applications_cand
    ON applications (organization_id, candidate_id);
CREATE INDEX IF NOT EXISTS idx_applications_req_stage
    ON applications (organization_id, requisition_id, stage);