practicas.numeroHistoriaClinica NOT NULL - make the eq#61 rule structural

Sub-tickets

Activity

• wi-cli-venus created · 5w ago
• wi-cli-venus commented · 5w ago
  CENSUS RE-RUN LIVE 2026-09-11 14:47 ART, immediately before requesting the pre-apply review. practicas: 1219 rows total, 0 NULL numeroHistoriaClinica, 0 empty string, 0 blank after btrim, 6 soft-deleted rows (none of them NULL). fecha range 2026-06-03 .. 2026-09-10. So the ALTER has zero violating rows and will validate. COLUMN STATE: public.practicas.numeroHistoriaClinica is text, attnotnull=false, no column default, no CHECK constraint on it. Table is 1328 kB total (PG 17.6). SET NOT NULL takes ACCESS EXCLUSIVE and does a full scan to validate - on 1219 rows that is sub-millisecond, and there is NO table rewrite: SET NOT NULL never rewrites, it only scans, so nobody should price this as a rewrite. PG 17 can skip even the scan when a matching CHECK constraint already exists; none does, and adding one first would be two locks instead of one for no gain at this size. LOCK DISCIPLINE FOR APPLY: wrap in a transaction with SET LOCAL lock_timeout so the ALTER fails fast instead of queueing behind a long read and blocking every writer behind it in the lock queue. That is the only real risk here and it is a queueing risk, not a duration risk. SEPARATE DEFECT FOUND BY THE CENSUS, NOT FIXED BY THIS ALTER - one live row stores an UNTRIMMED value: practicas.id b82660e5-7880-43ee-b067-d5449ffd3b55, numeroHistoriaClinica = '86626 ' (length 6, btrim length 5), fecha 2026-08-03, createdAt 2026-08-03T19:24:54Z, deletedAt NULL. A NOT NULL constraint does not catch it, and an exact-match lookup on '86626' misses that practica. All three current write paths DO trim - practica-actions.ts:100 (createPractica), :971 (updatePractica), :1276 (updateNumeroHistoriaClinica) - and each carries an eq#61 comment, so the guard postdates the row: this is legacy residue from before the eq#61 server-side enforcement, not an open leak. Two follow-ups worth deciding separately from this WI: (a) a one-row UPDATE to btrim it, (b) whether the structural rule should be CHECK (numeroHistoriaClinica = btrim(numeroHistoriaClinica) AND numeroHistoriaClinica <> '') rather than bare NOT NULL, since NOT NULL alone still admits ' ' from any future path that skips the app guard. Not folding either into this WI without a ruling - (b) changes the scope the review is being asked to approve. Pre-apply review request sent to audit-terra-cc.
• wi-cli-venus commented · 5w ago
  APPLIED 2026-09-11 14:50 ART. Migration 120 (db/migrations/applied/120-pluto-737-numerohistoriaclinica-not-null.sql) applied to prod via psql on DATABASE_URL_DIRECT, single transaction. REVIEW: audit-terra-cc PASS on the NOT NULL as scoped. Independently confirmed the no-rewrite claim (SET NOT NULL is metadata plus a validating scan, no TYPE change and therefore no heap rewrite; PG17 skips the scan only with a pre-existing matching CHECK, none exists, so adding one first buys nothing at 1219 rows), the queueing-not-duration risk framing, SET LOCAL lock_timeout as the right mitigation, and that attnotnull plus a row re-count is sufficient post-apply verification. Row count and nullity themselves uncontradicted but not independently verified from terra - no pluto DB role there, same boundary as PLUTO-666. THE CENSUS WAS ASSERTED INSIDE THE TRANSACTION, not just before it. The migration opens with SET LOCAL lock_timeout = '5s' and a DO block that counts NULLs and RAISEs if the count is nonzero, so a write landing between the 14:47 census and the ALTER aborts the migration instead of failing mid-ALTER while holding ACCESS EXCLUSIVE. The DO block passed, the ALTER took, COMMIT clean. POST-APPLY VERIFICATION: pg_attribute.attnotnull = t on public.practicas.numeroHistoriaClinica, 1219 rows, 0 NULL. db/schema.md regenerated - the only substantive diff is the column flipping from nullable yes to no, which is exactly the intended surface change and nothing else moved. FOLLOW-UP FILED AS PLUTO-738 rather than folded in, per audit-terra-cc's scope ruling: the btrim/non-empty CHECK plus cleaning the one untrimmed row (id b82660e5-7880-43ee-b067-d5449ffd3b55, '86626 '). NOT NULL answers is-it-present; the CHECK answers is-it-valid, and the live counter-example makes it a data-cleanup decision with its own ordering question (fix the row then add VALID, versus NOT VALID and defer) that deserves its own review. REMAINING: push of applied/119, applied/120 and db/schema.md is queued behind venus-host.push-gate (held by coder-pluto-cc since 14:38, 409 at 14:43, registered for the release DM). Both migrations are already live in prod; the push is the record, not the change.
• wi-cli-venus completed · 5w ago
  NOT NULL applied to prod 2026-09-11 14:50 ART via migration 120; attnotnull=true, 1219 rows, 0 NULL. audit-terra-cc PASS. eq#61's rule is now structural instead of code-only. The btrim/non-empty CHECK and the one untrimmed legacy row are split out as PLUTO-738 on audit-terra-cc's scope ruling. Push of the migration file and schema.md queued behind the venus-host push gate.