Skip to content

fix: many to many case/dup ids, cursorPaginate key and null, restore, find(5, null), soft delete timestamps, forceFill errors, foreignId chain, rollback missing file - #37

Merged
necessarylion merged 9 commits into
mainfrom
fix/orm-followups
Nov 2, 2026

Conversation

@mydoxie

@mydoxie mydoxie commented Nov 2, 2026

Copy link
Copy Markdown
Contributor

Bug fixes from the dox-orm#30 and dox-orm#31 reviews and other follow-ups. One commit per fix. No version bump. Each fix has a changelog line under Unreleased.

1. Many to many on a case-insensitive text key

  • Problem: MySQL and MariaDB compare text without case by default (SQLite does with COLLATE NOCASE). With a stored 'ABC', sync(['abc']) added 'abc', then deleted 'ABC'.
  • Effect: the database deleted both rows, so the link was lost. The same thing happened for a UUID in the other case, and for 'abc ' on MariaDB.
  • Fix: sync() and toggle() ask the database if an id that did not match in Dart is the same as a stored row. If it is, sync keeps that row and toggle removes it. This check only runs for text ids, when there are stored rows, and it costs one extra query. The delete also leaves out every id that must stay (NOT IN), so the database never deletes a kept row. attach() and detach() already behaved like the database, so they did not change.

2. Many to many: same id twice

  • Problem: on a text key, toggle(['1', 1]) removed both '1' and '01'. On a number key with two stored rows of one id, the result had detached: [1, 1].
  • Fix: ids with the same text ('1' and 1) are now one id. The result lists show each id once.

3. cursorPaginate

  • (a) Problem: the primary key check only compared the last part of the name, so orderBy('author.id') on a join did not add the key as a tie breaker. Rows with the same value could be skipped. Fix: only id, <table>.id and <alias>.id count as the key of the model.
  • (b) Problem: a null in the first row of a page made a good cursor look invalid, so page 1 came back and a client looped on it. Fix: the cursor check skips nulls. The order columns must still have no nulls, so this page now throws the clear "never null" StateError instead.
  • (c) Not changed: the savepoint inside a Postgres transaction. It is still needed. A cursor comes from the user, so a bad value (like 'abc' for an int column) gives a Postgres "data exception". Without the savepoint, this breaks the whole transaction of the app. The test "a bad cursor in a transaction keeps the transaction" checks this.

4. Query restore()

  • Problem: Blog().where(...).restore() returned true when any row matched, even a row that was not deleted. MySQL counts only changed rows, so it gave a different answer than Postgres and SQLite.
  • Fix: it now adds deleted_at IS NOT NULL.

5. find(5, null)

  • Problem: find(5, null) threw a type error with no clear message.
  • Fix: it now throws an ArgumentError that says the first arg must be a column name. Other find calls work as before.

7. Soft delete

  • Problem: a soft delete did not set updated_at. blog.delete() also did not set the deletedAt field on the model in memory.
  • Fix: a soft delete sets updated_at when the model has timestamps, for both a model delete and a query delete. blog.delete() sets deletedAt and updatedAt. blog.restore() sets deletedAt back to null, so the two stay in step. delete.dart is only used by plain queries, which do not know about timestamps, so the fix is in Model.delete().

8. forceFill

  • Problem: on a hand-written model with no setColumnValue(), forceFill did nothing and gave no error. A value of the wrong type was also skipped with no error.
  • Fix: both now throw an ArgumentError with a clear message. An ISO date text now sets a DateTime field. A key that is not a column is still skipped, as before. forceFill is not released yet, so no released behaviour changes.

9. foreignId(...).unique().constrained()

  • Problem: after .unique(), .defaultTo(), .change() and other modifiers, constrained() did not compile.
  • Fix: every modifier of ForeignIdColumnDefinition now returns that type, like Laravel.

10. Rollback with a missing migration file

  • Problem: rollback threw when the file of a migration that already ran was gone.
  • Fix: like Laravel, it prints "Migration not found: ", keeps that row, and rolls back the others.

Not in this PR

    1. reload() keeps an old hasOne/belongsTo value: the fix must go in the code that dox_builder writes (the if (result[...] != null) guard in the relation getter). That is in the dox repo, so it is not changed here.

Tests

  • dox_orm: SQLite, MariaDB 11 and MySQL 8.4 pass. On Postgres everything passes except many_to_many_text_key_test and many_to_many_big_key_test, which only fail when they run after other files. This is the same on main. The cause is the stale prepared type bug in the pub.dev native driver used locally. Both files pass when run alone.
  • dox_migration: postgres, MariaDB, MySQL 8.4 and SQLite pass.
  • dart format and dart analyze --fatal-infos are clean.

@mydoxie

mydoxie commented Nov 2, 2026

Copy link
Copy Markdown
Contributor Author

Bot review

I ran dart analyze --fatal-infos (clean, with sibling overrides) and dart format (clean). I ran the dox_orm tests on SQLite, Postgres 16 and MariaDB 11. All pass, and on Postgres that includes the text key files. The dox_migration tests pass on Postgres, MariaDB and SQLite. The new tests fail on the old many_to_many.dart, get.dart and model.dart, so they really test the fixes.

I also ran extra checks for sync()/toggle():

  • Number key, new text id (sync(['5', 1]), toggle(['6'])): correct on Postgres, MariaDB and SQLite. The new whereIn on an integer column does not fail on Postgres.
  • Text key, mixed case (sync(['A','d','b']) with stored a,b,c): Postgres and SQLite treat A as a new id. MariaDB keeps a. I saw no lost links and no extra rows.

The soft delete change works with timestamps: false (null column, so ?updatedAtColumn leaves it out) and with a custom updated_at name. No version: bump. The changelog lines are under ## Unreleased.

Blocking

None.

Non-blocking

  • many_to_many.dart:318 (delete with keep): with stored 'ABC' and 'abc' on a column that ignores case, sync(['ABC']) returns detached: ['abc'], but the NOT IN keeps that row. I checked this on MariaDB: both rows stay. No link is lost, but the result lists a row as removed when it is still there. You could leave such ids out of detached, or say so in the doc comment.
  • many_to_many_text_key_test.dart:206: the test only checks isNotEmpty. It could check the exact keys and r.detached, so the point above is pinned down.
  • many_to_many.dart:355: when the count finds a match, it runs one SELECT for each text id. This is fine for short lists. A long list with one case match costs N queries.
  • model.dart:600: a model restore() now sets deletedAt, but it does not set updated_at. A soft delete now does. Laravel's restore updates updated_at too. This was not promised in this PR, so it can be a later item.

@necessarylion
necessarylion merged commit a3e1627 into main Nov 2, 2026
2 checks passed
@necessarylion
necessarylion deleted the fix/orm-followups branch November 2, 2026 11:48
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.

2 participants