fix(db): V39_5 test fixture — unblock V40 in CI (audit_logs_pkey collision)

[ahmetabdullahgultekin]

Summary (UPDATED 2026-05-12 after first CI run)

V40 (partition_audit_logs.sql) has two latent bugs that surface when run end-to-end on a fresh Testcontainers DB. This PR resolves the first one structurally; the second is documented for operator decision because the brief's constraints do not permit a code-side fix.

Bug #1: audit_logs_pkey name collision (FIXED in this PR)

V5 declares audit_logs.id UUID PRIMARY KEY inline → PG auto-names the PK constraint and index audit_logs_pkey. V40 then:

1. ALTER TABLE audit_logs RENAME TO audit_logs_legacy (constraint name is NOT renamed, stays attached to legacy)
2. CREATE TABLE audit_logs ... PARTITION BY RANGE (created_at) (new heap)
3. ADD CONSTRAINT audit_logs_pkey PRIMARY KEY (id, created_at) (collision)

ERROR: relation "audit_logs_pkey" already exists

Fix: test-only fixture V39_5__rename_audit_logs_pkey_for_v40.sql (loaded only by integration profile via spring.flyway.locations=classpath:db/migration,classpath:db/test-fixtures) renames audit_logs_pkey → audit_logs_v5_pkey before V40 runs. Idempotent. Matches the V28_5 precedent exactly.

CI log from this PR's first run (commit 20d61ea) confirms the fixture executes correctly:

2026-05-12T21:05:03.115Z INFO ... DB: V39_5: renamed audit_logs_pkey -> audit_logs_v5_pkey to free the name for V40.

Bug #2: || operator inside COMMENT ON TABLE (NOT fixed; operator decision required)

After my fix freed the pkey name, V40 progressed further and surfaced a SECOND bug at line 270:

COMMENT ON TABLE audit_logs IS
    'Range-partitioned by created_at (monthly). Pre-created partitions: ... '
    || 'Schedule ensure_audit_logs_partition() ... '
    || 'PK is (id, created_at) — semantic change from V5 ...';

PG grammar for COMMENT ON ... IS requires a single string literal, not an expression. The || concatenation operator is invalid in DDL context:

ERROR: syntax error at or near "||"
Position: 172
Line: 270

I reproduced this locally against pgvector/pgvector:pg17:

COMMENT ON TABLE ttest IS 'a' || 'b' || 'c';
-- ERROR: syntax error at or near "||"

This means V40 cannot succeed on any clean DB anywhere. Prod escaped this because V40 was BASELINE-SKIPPED in 2026-04 (per V57 header comment + BACKEND_REVIEW_2026-04-30 §4); the migration is recorded as success=t in prod's flyway_schema_history but its body never ran.

Why this PR cannot also fix Bug #2

The task brief explicitly forbids:

1. "Don't rewrite V40 in place (locked checksum in prod)" — would mismatch prod's recorded checksum + require manual flyway repair.
2. "Don't change the flyway_schema_history directly in prod or in test fixtures" — the only way to mark V40 as success without running it.

Bug #2 has no purely additive fix:

• Test fixture pre-creating partitioned state: V40's guards IF NOT EXISTS (audit_logs heap) THEN RAISE EXCEPTION would fire → V40 marked failure → subsequent migrations don't run. Same blocker.
• Flyway baselineVersion=40: would also baseline V1-V39 (single point), leaving the test DB schemaless for V41+.
• Flyway target=39: skips V40 AND V41+; loses the entire subsequent migration chain.
• Flyway callbacks (beforeMigrate.sql): would need to manipulate flyway_schema_history, forbidden by brief.

Recommended next step (operator decision)

The narrowly-scoped fix is to edit V40 line 270-273 in place to use a single string literal (no ||), commit it, then run mvn flyway:repair against prod to refresh V40's checksum in flyway_schema_history. This is a 2-line code change + 1 operator command. The brief's "don't edit V40 in place" rule was written before Bug #2 was discovered and likely needs explicit operator approval to relax.

Alternative: accept current state — Integration tests stays continue-on-error indefinitely, but Bug #1 is now off the critical path. When the operator deploys Option A (custom pgvector+partman image, OPERATOR_ACTIONS_2026-05-12.md item 1) and rebuilds api, V57 will idempotently convert prod's heap → partitioned regardless of V40's state.

Why not edit V40 in place in THIS PR

Three previous in-place V40 patches were each reverted:

• 2b00a7b — reverted by 43d0050
• a30287b — reverted by 614a94f
• 81101f0 — reverted by f404d12

The reverts predate today's brief and may not have known about Bug #2. The brief's explicit "Don't rewrite V40 in place" is what I am honoring here. If the operator chooses to relax that on review, a 2-line follow-up patch + flyway repair clears CI fully.

Why not a new V62 production migration

V57 (audit_logs_pg_partman.sql, merged in PR #68) already provides the prod-side conversion path: idempotent detection of relkind='r', full conversion when pg_partman is available, fail-soft when it isn't. A V62 duplicating that logic would be dead weight. And V62 would never RUN in CI anyway, because V40 fails first.

Verification

Local SQL simulation (pre-CI):

NOTICE: V39_5: renamed audit_logs_pkey -> audit_logs_v5_pkey to free the name for V40.
ALTER TABLE  -- V40 RENAME TO audit_logs_legacy
CREATE TABLE -- V40 partitioned root
ALTER TABLE  -- V40 ADD CONSTRAINT audit_logs_pkey PRIMARY KEY (id, created_at) — succeeds
 result
--------
 PASS

CI run on this PR (commit 20d61ea):

• V39_5 fixture executes successfully (visible in flyway logs).
• V40 progresses past the pkey collision.
• V40 fails at line 270 on the COMMENT || syntax error (Bug Add comprehensive README documentation for identity-core-api #2).
• Outcome: same final state as before (Integration tests red), but the failure root cause has shifted from a fixable-in-fixture bug (Initialize Repo #1) to a fundamental V40-body bug (Add comprehensive README documentation for identity-core-api #2) that needs operator policy approval.

Idempotency:

• V39_5's IF guards check relkind='r' + constraint name match. Re-running against a post-V40 DB falls through and no-ops. Verified locally.

Coordination

• This PR adds no prod V-number, so no collision with Team Infra/DB's V61 PR feat(db): V61 audit_logs.tenant_id NOT NULL (lock down P1 from senior reviews 2026-05-12) #99.
• Team Infra/DB's PR feat(db): V61 audit_logs.tenant_id NOT NULL (lock down P1 from senior reviews 2026-05-12) #99 (V61 audit_logs.tenant_id NOT NULL) applies cleanly downstream of my fix once Bug Add comprehensive README documentation for identity-core-api #2 is addressed.
• This PR does NOT modify V40 or directly alter flyway_schema_history.

Deployment notes (operator)

• No prod migration changes. Test-resource-only change.
• No image rebuild required for this PR specifically. Memory rule feedback_audit_delta_before_rebuild still applies whenever the next prod rebuild is scheduled.
• The pre-existing prod-side work item — Option A custom pgvector+partman image (OPERATOR_ACTIONS_2026-05-12.md item 1) — is independent of this fix. V57's heap→partitioned conversion remains the prod-side path.

Test plan

• Reproduce Bug Initialize Repo #1 against pgvector/pgvector:pg17 locally — confirmed.
• Reproduce Bug Add comprehensive README documentation for identity-core-api #2 (|| in COMMENT ON ... IS) locally — confirmed unfixable in-grammar.
• Verify V39_5 fix unblocks Bug Initialize Repo #1 end-to-end in psql — confirmed.
• Verify V39_5 is idempotent — confirmed.
• mvn test-compile succeeds — confirmed.
• Run on CI to surface remaining failure mode — confirmed (Bug Add comprehensive README documentation for identity-core-api #2 cleanly surfaces).
• Operator decision needed: approve narrow V40 edit + flyway repair to resolve Bug Add comprehensive README documentation for identity-core-api #2 fully, OR accept partial fix with Integration tests staying continue-on-error.

🤖 Generated with Claude Code

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.