Skip to content

fix(migrator): read the pre-3.0 null column option as an alias of allowNull - #4395

Merged
bpamiri merged 2 commits into
developfrom
peter/migration-null-alias
Oct 5, 2026
Merged

bpamiri merged 2 commits into
developfrom
peter/migration-null-alias

Conversation

@bpamiri

@bpamiri bpamiri commented Oct 5, 2026

Copy link
Copy Markdown
Collaborator

Migration column option null is read again

Before Wheels 3.0, a migration column's nullability option was called null. 3.0 renamed it allowNull with no alias, so from 3.0 through 4.1 null was silently ignored. With no allowNull, no NULL / NOT NULL clause is emitted, and the column gets the database's default, nullable on most databases. A 2.x migration's t.string(columnNames = "email", null = false) has created a nullable column ever since.

null is now a deprecated alias of allowNull again, in the typed column functions (t.string() and the others), t.column() and t.references():

  • allowNull wins when both are passed.
  • The value is read as arguments["null"], never arguments.null, since null is a keyword on some engines.
  • The deprecation is logged to wheels.log once per request (one migration run), not once per column.
  • references() stays NOT NULL by default. Its allowNull = false default moved from the signature into the body, so it can tell a passed allowNull from the default.

wheels upgrade check lists null= migrations

Two advisories for an upgrade to 4.2, scanning app/migrator/migrations with comments stripped:

  • null=false. Wheels 3.0 through 4.1 created these columns nullable, so the running database may hold NULL rows in them. A database rebuilt on 4.2 from the same migrations gets NOT NULL columns, stricter than the one in production. The advice is to check each column for NULL rows before tightening it, and to rename null to allowNull.
  • null=true. Rename it to allowNull.

Docs

  • The 4.1 → 4.2 upgrade guide gets a "Migration column option null" section, with the three steps for null=false columns.
  • A fixed fragment.

Specs

vendor/wheels/tests/specs/migrator/nullAliasSpec.cfc passes null through argumentCollection (RustCFML can't parse null = x as a named argument). No table is created.

  • null sets allowNull on typed functions (false and true), on column() and on references().
  • allowNull wins over null, on a typed function and on references().
  • references() defaults to NOT NULL.
  • The deprecation is logged once per request.

cli/lucli/tests/specs/commands/UpgradeCheck4x2Spec.cfc:

  • null=false and null=true lines are listed separately, with file and line.
  • allowNull=false and a commented-out null=false aren't listed.

RED on develop: the typed-function and column() / references() cases fail (no allowNull is set), and so does the deprecation case.

GREEN:

  • Lucee 7 + SQLite after merging develop: full core suite 6942/6942.
  • CLI suite: 2354/0, including the two new specs.
  • RustCFML: 6836/0.
  • Complexity gate passes.

🤖 Generated with Claude Code

bpamiri and others added 2 commits October 5, 2026 12:10
…owNull

Before 3.0 a migration column's nullability option was `null`; 3.0 renamed it
`allowNull` with no alias, so `null` was ignored and a `null = false` column was
created nullable. Column functions, column() and references() read it again
(arguments["null"]): `allowNull` wins when both are passed, and the deprecation
is logged once per migration run.

`wheels upgrade check` lists migrations that use null=false (with what to
check before tightening) and null=true.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Peter Amiri <peter@alurium.com>
@bpamiri

bpamiri commented Oct 5, 2026

Copy link
Copy Markdown
Collaborator Author

rev1-r3 review @ c7017ae: code APPROVE (final sign-off after matrix run 37365306608 and PR CI); one merge-order note for whichever of this and #4393 lands second

  • The alias is applied at every entry point.
    • $applyNullAlias() runs in column(), in $addTypedColumns() (the shared path for t.string() and the other typed helpers), and in references().
    • It copies args["null"] to allowNull only when allowNull wasn't passed, so allowNull wins when both are given (pinned by "loses to allowNull when both are passed"). Then it deletes null.
    • Reading via args["null"] / StructKeyExists(args, "null") avoids the bare keyword. The matrix's Adobe and BoxLang legs confirm it compiles and behaves there.
  • references() keeps its default. allowNull loses its = "false" default so the alias can be detected, and is set to false right after the alias step when neither name was passed. "keeps references() NOT NULL by default" pins it.
  • null = true is harmless: it maps to allowNull = true, which is today's default for the typed columns.
  • One deprecation line per request, keyed on request.$wheelsMigratorNullAliasWarned. A migration run (web or CLI bridge) is one request, so it's one line per run, not one per column.
  • The advisories. The null = false advisory explains that 3.0–4.1 created those columns NULLABLE (so they may hold NULL rows), that a database rebuilt on 4.2 is now stricter, and to check for NULL rows before tightening with changeColumn(allowNull = false). The null = true advisory just asks for the rename. Both regexes require null to follow ( or ,, so allowNull = … doesn't match. They scan only app/migrator/migrations/**/*.cfc, and the grep path strips comments first. The guide anchor "Migration column option null" exists in 4x-1-to-4x-2.mdx.

Merge order with #4393 (both touch TableDefinition.$addTypedColumns()): #4393 adds a $checkArguments(...) there with a declared list that includes allowNull but not null. Whichever PR merges second must keep $applyNullAlias(arguments.args) before that check. Otherwise development logs a strict-arguments warning ("null … Did you mean allowNull?") for a call the alias already handles. #4393's Migration.addColumn() still forwards null (it only removes table, addColumns and referenceName), so the alias keeps working through addColumn / changeColumn too. I'll check the order on the second PR's merge head.

@github-actions

github-actions Bot commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

Wheels Test Results

227 092 tests   221 971 ✅  1h 23m 27s ⏱️
 17 824 suites    5 121 💤
     32 files          0 ❌

Results for commit c7017ae.

♻️ This comment has been updated with latest results.

@bpamiri

bpamiri commented Oct 5, 2026

Copy link
Copy Markdown
Collaborator Author

rev1-r3 review @ c7017ae: APPROVE (final)

Matrix 37379851062 (fresh full run on this head, dispatched after the outage) completed with success: 32 of 32 legs have tests > 0 (7091–7108 each) and 0 failures and 0 errors on every leg. nullAliasSpec passed 5 of 5 on every leg (Adobe 2023/2025, BoxLang, Lucee 6/7, across all databases), with 0 skipped.

PR CI is 28 pass / 0 fail / 0 pending, with 2 skipping. That includes the LuCLI job (UpgradeCheck4x2Spec), the CLI suite and the guides build.

Ready to merge. Merge-order note with #4393, which is still open and also edits $addTypedColumns: whichever of the two merges second needs a conflict resolution that keeps $applyNullAlias before $checkArguments, so that null= is turned into allowNull= before strict argument checking rejects it.

@bpamiri
bpamiri merged commit 3988455 into develop Oct 5, 2026
84 of 97 checks passed
@bpamiri
bpamiri deleted the peter/migration-null-alias branch October 5, 2026 23:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant