Skip to content

test(schema): let the oracle declare a second migration lineage, for identifiers that cannot be renumbered - #905

Open
vishk23 wants to merge 1 commit into
ryanbr:mainfrom
vishk23:upstream-pr/schema-oracle-lineages
Open

test(schema): let the oracle declare a second migration lineage, for identifiers that cannot be renumbered#905
vishk23 wants to merge 1 commit into
ryanbr:mainfrom
vishk23:upstream-pr/schema-oracle-lineages

Conversation

@vishk23

@vishk23 vishk23 commented Jul 27, 2026

Copy link
Copy Markdown

testGrdbMigrationIdentifiersAreUniqueAndSequential (and its Kotlin twin) asserts that every GRDB
migration identifier's vN equals its registration index. That is the right default, and this PR
keeps it as the default. What it adds is a way to say "these identifiers are numbered separately,
here is why" — for the one case where the rule asks for something that cannot safely be done.

Renumbering is not always available

The test's remedy is "Renumber before merging." For two unmerged PRs that both claim a vN — the
#369 / #475 case the doc comment cites — that is exactly right, and this PR does not change it.

It stops being available once an identifier has reached a database. GRDB keys migrations by
identifier string: rename v26-foo to v32-foo and a device that already ran it no longer finds
that name in grdb_migrations, so the migrator reads it as un-applied and re-runs its body against a
schema that already has the table. The migration then throws, and the store fails to open. So for a
fork, a long-lived branch, or any identifier already shipped in a release, the identifier is frozen —
and the suite has no way to express that short of deleting the check.

Concretely: a fork carrying four iOS-only migrations numbered from v26 before upstream's own
v26..v29 existed ends up with both lineages claiming the same four numbers, no way to renumber, and
four red tests on every merge from upstream.

The mechanism

An optional grdbMigrationLineages in the oracle declares a set of identifiers numbered
independently of the baseline:

"grdbMigrationLineages": {
  "fork-ios-only": {
    "reason": "Why these identifiers are frozen and cannot be renumbered.",
    "migrations": ["v26-cloud-tombstone", "v27-apple-step-hour", "v28-phone-timezone"]
  }
}

Whatever is not declared is the baseline lineage, and the baseline is still held to exactly
v1..vN with no gaps and no repeats.

Nothing is weakened by default

  • The key is absent from the oracle in this PR. With no lineage declared the behaviour is
    bit-for-bit what it is on main today — this is a no-op for upstream until someone declares one.
  • Full-identifier uniqueness stays absolute, checked across everything before any subtraction. A
    genuinely duplicated identifier still fails, because GRDB would silently skip the second body.
  • An undeclared collision still fails. #369 vs #475 is caught exactly as before.
  • A declared lineage must be true: every member must actually be registered, so an entry that
    stops being true fails rather than rotting — the same rule divergenceReasons already lives by.
  • A lineage's own vN must strictly increase.
  • A gap cannot be laundered through a lineage. Declaring an upstream migration as part of one to
    dodge the sequence check fails anyway, because removing it from the baseline shifts everything
    after it.

The escape hatch therefore costs a written-down reason and cannot be taken by accident, which is the
same bargain divergenceReasons strikes for schema drift.

Verification

On this branch:

Then, to confirm the new key cannot be used to wave a real problem through, each of these was
injected on this branch and fails on both halves:

Mutation Swift Kotlin
lineage declares an unregistered migration (stale entry) fails fails
a real baseline migration hidden in a lineage (gap laundering) fails fails
lineage declared but empty fails fails
lineage's own vN does not strictly increase fails fails

The undeclared-collision case cannot be staged here, because main's registered migrations are a
clean v1..vN; it was exercised on a branch that really does register two lineages claiming the same
four numbers, where removing the declaration puts the test straight back to failing.

The _readme in both oracle copies documents the new key; the copies stay byte-identical.

…identifiers that cannot be renumbered

testGrdbMigrationIdentifiersAreUniqueAndSequential asserts every migration
identifier's vN equals its registration index, and tells the author to
"Renumber before merging" otherwise. For two unmerged PRs both claiming a vN —
the #369 / #475 case the doc comment cites — that is the right remedy, and this
does not change it.

It stops being available once an identifier has reached a database. GRDB keys
migrations by identifier STRING: rename v26-foo to v32-foo and a device that
already ran it no longer finds that name in grdb_migrations, so the migrator
reads it as un-applied and re-runs its body against a schema that already has
the table. For a fork, a long-lived branch, or any identifier already shipped in
a release, the identifier is frozen, and the suite has no way to say so short of
deleting the check.

So the oracle gains an optional grdbMigrationLineages: a named set of
identifiers numbered independently of the baseline, carrying the reason they
cannot be renumbered. Whatever is NOT declared is the baseline lineage and is
still held to exactly v1..vN with no gaps and no repeats.

Nothing is weakened by default. No lineage is declared here, so upstream
behaviour is bit-for-bit unchanged. Full-identifier uniqueness stays absolute,
checked before any subtraction, because a genuine duplicate makes GRDB silently
skip the second body. An undeclared collision still fails. A declared lineage
must be true — every member registered — so an entry that stops being true fails
rather than rotting, the rule divergenceReasons already lives by. A lineage's
own vN must strictly increase. And a gap cannot be laundered through a lineage:
declaring a baseline migration as part of one to dodge the check fails anyway,
because removing it from the baseline shifts everything after it.

Verified on this branch: Swift 330/330 (SchemaOracleTests 7/7); Android
SchemaOracleTest 4/4. The suite's one failure,
DeepCaptureMigrationTest.repositoryInsertV18Aux_insertsThenPrunes, is red on
main independently of this change and is what #897 fixes. Each of the four abuse
mutations above was injected and fails on both halves.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant