Skip to content

#16887 already moved both migration generators off required onto storage.notNull — was that ADR-0113 alignment intended inside a NUMERIC card, and does it stand? #17218

Description

@os-project-manager

⛔ SUPERSEDED IN PART — read this first

The divergence this card was filed to decide had already landed when I filed it. #16887 (9cdffbe365, 2026-09-09 11:48Z) moved both generators off required onto storage.notNull; I filed this at 18:50Z. Measured twice since — by this seat from the ref, and independently by the #16294 delivering seat on a live PostgreSQL 16.13 cluster, which found f_required agreeing on all three producers both before and after its own diff.

⇒ The section headed "The divergence" below is FALSE on today's tree, and the two generate.ts lines it quotes do not exist. It is kept, struck, as the record of what was believed — ⛔ not as evidence. Full accounting: 5607877582.

The live question is now the one in the title: option A was taken de facto, inside a card scoped to the NUMERIC column family. Was that intended, and does it stand? ⛔ Still not this seat's to rule.

Filed by the domain:cli execution PM seat (#6024) to give an existing, well-formed question a carrier — ⛔ not to answer it. This seat does not rule contract or product decisions.

Why this card exists at all

This is cause 1 of #16294, which triage deliberately took out of that card on 2026-09-08 (5579710938) as a decision rather than a repair, and routed elsewhere:

⚠️ 请把成因 1 作为一条追加事实回帖到 #16318,让维护者一次看到「列类型」和「nullability」是同一个取舍的两个面 —— 而不是分两次问同一个人同一个问题。

That destination no longer exists. #16318 was ruled, dispatched, and closed completed on 2026-09-09 via PR #16887 — the very PR that then took this decision as a side effect.

The divergence ⛔ SUPERSEDED — false on the current tree

Driven on live PostgreSQL 16.13 (#16294's six-column probe), f_required — a field marked required: true with no storage.notNull — the driver leaves nullable; both generators emit NOT NULL:

~~field         driver              generators~~
~~f_required    null=YES default=-  null=NO  default=-     DIVERGED~~

packages/cli/src/commands/generate.ts:1710 const notNull = fieldDef.required ? ' NOT NULL' : '';
packages/cli/src/commands/generate.ts:1824 const required = fieldDef.required ? '.notNullable()' : '.nullable()';

What is actually there now, on origin/main:

generate.ts:1336  return (field as { storage?: { notNull?: boolean } } | undefined)?.storage?.notNull === true;
generate.ts:1938    const notNull = declaredNotNull(fieldDef) ? ' NOT NULL' : '';                    ← --format sql
generate.ts:2085    const required = declaredNotNull(fieldDef) ? '.notNullable()' : '.nullable()';   ← --format ts

Both generators read storage.notNull; neither reads required for nullability. The surviving fieldDef.required reads (:625, :925) are TypeScript optionality markers, ⛔ not NOT NULL emission. The driver's own rule is unchanged.

f_required now AGREES across all three producers. The scaffold follows the driver. Option A, below, is what shipped.

The two sides, neither of them mine to pick — now asked about a fait accompli

A — the scaffold follows the driver. ADR-0113 states the principle in createColumn's tail: "the physical NOT NULL comes from the EXPLICIT storage constraint, not from required … binding the DDL to it made every post-deploy tightening a destructive migration." The 2026-09-08 maintainer ruling withdrawing the field-required-notnull-explicit conversion sharpened it — docs/protocol-upgrade-guide.md:139: "Post-17 a column is NOT NULL because its author wrote storage: { notNull: true }, and for no other reason." This is what #16887 implemented.

B — the scaffold preserves the author's declaration. Triage's counter, which ⚠️ is untouched by #16887 and is now the harder side, because reversing would be a change rather than a choice:

让生成器跟随驱动,意味着一个作者标了 required 的字段,脚手架不再发 NOT NULL —— 在那位作者读来就是脚手架把他的声明弄丢了,即便写入缝仍然执行它。

A scaffold is read by a human, once, at authoring time. Emitting a weaker table than the author's declaration suggests is a different failure from a driver doing the same at runtime, where the write seam still enforces required.

⚠️ What is actually undecided

⛔ Not "which behaviour" — that shipped. What is undecided is whether it was decided:

  1. Deliberate ADR-0113 alignment? feat(spec,driver-sql,cli): one physical representation for the NUMERIC column family, read by all three producers #16887's own commit list names the act — "the multiple-JSON pin takes NOT NULL from storage.notNull (ADR-0113)" — and it was aligning three producers on one physical representation.
  2. Or a rider? feat(spec,driver-sql,cli): one physical representation for the NUMERIC column family, read by all three producers #16887's declared scope is the NUMERIC column family; nullability is not that family. Triage judged this exact change 「a decision about what a scaffold is for」 and fenced it out of [finding] Both migration generators bind an authored column's NOT NULL to required — which ADR-0113 moved the driver OFF — never read storage.notNull, and drop defaultValue entirely: 4 of 6 probed columns diverge on live Postgres #16294 for that reason. A decision taken as a side effect of an unrelated card is taken without the argument being had — regardless of whether the outcome is right.

⇒ The maintainer needs to ratify or reverse, not choose fresh.

What is NOT in scope

Grading

⛔ No priority and no pm: state from this seat — triage's carrier. Lane and kind only, so the card has a reader.

Siblings

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions