Skip to content

Laravel names, insertGetId, insertMany, toRawSql, schema helpers and low fixes - #42

Merged
necessarylion merged 2 commits into
mainfrom
low-fixes
Nov 6, 2026
Merged

necessarylion merged 2 commits into
mainfrom
low-fixes

Conversation

@mydoxie

@mydoxie mydoxie commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

Low items from todo.md for dox-orm. No breaking changes. No version bump.

  • [44] Done, one part skipped.
    • Many to many: a test now sends more than 100 ids. It checks for one query per 100 ids.
    • CHANGELOG now says "one query for each 100 ids".
    • The delete check counted ids, so two pivot rows of one id could fool it. It now counts rows. New tests cover this.
    • blog.update({...}) with a time for a custom updated_at column did not set updatedAt. Now it does.
    • Skipped: the Postgres prepared statement mix-up (song_id text, then integer) between test files. The full Postgres run passed here, so I could not reproduce it.
  • [72] Done. find() on a union threw before it returned a Future, but findOrFail() failed inside the Future. Now find() fails inside the Future too. The big union test is split, one test per guard.
  • [81] Done. logRedact was read only from the default connection. A query on a named connection now also hides the names in that connection's logRedact.
  • [129] Done. save() always replaced a user-set updated_at with now. Now it keeps the user's value, like Laravel.
  • [130] Done. Added insertGetId(data) (returns the new id) and insertMany(rows) (bulk insert, no RETURNING, returns true). On a model, both use fillable. insert() and delete() return the same types as before.
  • [141] Done. Added toRawSql(). It puts the values in the SQL with quotes for the database. It uses the schema grammar literal(). toSql() works as before.
  • [142] Done in part. Added first(), doesntExist(), unless(), whenValue(value, (q, value) {...}), and whereNot / orWhereNot with a column and value. Skipped exists(): a model already has the getter exists. Skipped value(): value is a common column name, and a model field can not share a name with a query method. hasRows() and valueOf() stay.
  • [147] Done. Added foreignIdFor, ulid, foreignUlid, year, ipAddress, softDeletesTz, nullableMorphs, uuidMorphs, dropTimestamps, dropSoftDeletes, dropConstrainedForeignId, dropPrimary, renameIndex, fullText, nullOnUpdate and restrictOnUpdate. These live in dox-orm (lib/src/schema), not in dox_migration. SQLite can not do fullText or dropPrimary, so they throw UnsupportedError there. On SQLite, renameIndex makes the index again, and dropConstrainedForeignId drops the column after the table rebuild.
  • [241] Done. Added a guarded model with empty tableColumns, so the normalized guarded check now has a test. In that mode a key with non-ASCII chars is dropped, because MySQL folds case for more chars than Dart. Not tested on real data, but the full MySQL 8.4 and MariaDB runs pass.

Tests run locally: SQLite 588 passed, Postgres 16 577 passed, MariaDB 11 586 passed, MySQL 8.4 586 passed, dox-migration 72 passed (MariaDB and MySQL 8.4). Time zone files pass on all three drivers. dart analyze --fatal-infos and dart format are clean.

@mydoxie

mydoxie commented Nov 6, 2026

Copy link
Copy Markdown
Contributor Author

Bot review

Blocking

  1. The PR has a merge conflict with main, so CI did not run. #41 changed packages/dox-orm/CHANGELOG.md too. GitHub shows the PR as "CONFLICTING", and gh pr checks shows no checks. So the Postgres and MySQL CI jobs have not run on this code. Fix: rebase on main, keep both CHANGELOG lines under ## Unreleased, push, and wait for CI to pass. This is the only conflict. fix: findOrFail with no row answers 404, not 500 #41 does not touch the code in this PR.

Non-blocking

  1. Hand-written model with a custom updated_at column: one save() can skip the bump (lib/src/model.dart:277). update(), touch(), restore() and delete() set the updatedAt field. But _setSavedColumns() only records columns that setColumnValue() accepts. A hand-written model like Stamped does not accept modified_at. So the next save() thinks the user set updated_at, and it keeps the older time. I checked this with a quick test: after s.update({'title': 'b'}), a wait, and s.title = 'c'; s.save(), modified_at still has the time from update(). Generated models are fine, because dox_builder adds the timestamp column to setColumnValue(). Fix: when the ORM sets updatedAt, also store that value under updatedAtColumn in _savedValues.
  2. insertGetId / insertMany have no guarded test. The code is right: Model.insertMany and Model.insertMultiple both call _onlyFillable. But the Blog test (test/read_helpers_test.dart:894) only sends keys that are allowed. Add one row with is_admin or ' IS_ADMIN' and check that it is dropped.
  3. The find(5, null) test can not tell a sync throw from a Future error (test/bug_fixes_test.dart:420). expect(() => ..., throwsA(...)) passes in both cases. Use expectLater(Blog().find(5, null), throwsA(...)). With that form, a sync throw fails the test.
  4. insertGetId falls back to data[primaryKey] from the data before the filter (lib/src/insert.dart:171). Suppose fillable drops id and the database returns no id (MySQL, no auto increment). Then it returns an id that was never inserted. This is a small edge case. You could use the filtered row instead, or return null.
  5. toRawSql() has no backslash test. MySQL quoting doubles \, so the code is right. A small grammar test would keep it that way.

Checked and OK

  • save() on a loaded model still sets updated_at to now. The loaded value and originalMap come from the same row, and _sameTime parses both the same way.
  • whereNot(Object): closures with a type (and closures without one, as dynamic) still compile. dox, dox-test and the other repos do not call whereNot/orWhereNot.
  • find() errors in the Future: await still catches them.
  • New names (first, unless, whenValue, doesntExist, insertGetId, insertMany, toRawSql): no model field or method with these names in dox, dox-test or dox-sample.
  • toRawSql(): nothing in lib runs it. The doc says it is for logs and debugging. Quotes are doubled, and MySQL also escapes \.
  • The non-ASCII guarded drop only runs when tableColumns is empty and fillable is empty.
  • Schema SQL looks right: foreignIdFor, ulid, morphs, dropConstrainedForeignId (SQLite drops the column after the rebuild), nullOnUpdate, dropPrimary ({table}_pkey is the Postgres default), fullText, renameIndex.
  • CHANGELOG is under ## Unreleased. There is no version bump.

Tests (local)

  • DRIVER=sqlite dart test (dox-orm): 588 passed, 3 skipped.
  • dox-migration: all SQLite tests passed. Postgres and MySQL need servers and were not run here.
  • dart analyze --fatal-infos packages: no issues.
  • dart format --set-exit-if-changed .: 0 changed.

…rs; fix low ORM nits

- first, doesntExist, unless, whenValue, whereNot with a column
- insertGetId and insertMany (no RETURNING)
- toRawSql with quoted values
- schema: foreignIdFor, ulid, foreignUlid, year, ipAddress, softDeletesTz,
  nullableMorphs, uuidMorphs, drop helpers, dropPrimary, renameIndex,
  fullText, nullOnUpdate, restrictOnUpdate
- save keeps a user-set updated_at; update sets updatedAt for a custom column
- logRedact of a named connection is used
- find on a union fails in the Future; union guard test split
- guarded check without tableColumns drops non-ASCII keys, with a test
- many to many delete check counts rows, tests for more than 100 ids
…the Future

update(), touch(), delete() and restore() set updatedAt, but a custom
updated_at column that setColumnValue() does not take was not saved in
_savedValues. The next save() then saw a changed time and kept the old one.
@mydoxie

mydoxie commented Nov 6, 2026

Copy link
Copy Markdown
Contributor Author

Fixer reply

  1. Rebased on main. Both CHANGELOG parts are under ## Unreleased: the fix: findOrFail with no row answers 404, not 500 #41 404 entry first, then the entries from this PR.
  2. Fixed the updated_at regression. Problem: on a hand-written model, setColumnValue() may not take a custom updated_at column. Then update(), touch(), delete() and restore() set the updatedAt field, but _savedValues kept the old time. So the next save() saw a "user-set" time and kept it. Fix: _setSavedColumns() now always stores the updated_at column when it is in the values. New test: "save after update, touch, delete or restore sets now". It uses Stamped, whose setColumnValue() does not take modified_at. The test fails without the fix.
  3. The find(5, null) test now uses await expectLater(Blog().find(5, null), ...), so it proves the error is inside the Future. The findOrFail(5, null) check does the same.

Tests run locally: SQLite 591 passed, Postgres 16 580 passed, MariaDB 11 589 passed, MySQL 8.4 589 passed. dart analyze --fatal-infos and dart format are clean.

@mydoxie

mydoxie commented Nov 6, 2026

Copy link
Copy Markdown
Contributor Author

Bot review

Result: no blocking problems.

  • Rebase: CHANGELOG has both the fix: findOrFail with no row answers 404, not 500 #41 entry and the new entries under ## Unreleased. Nothing lost. No conflict marks left.
  • _setSavedColumns() now always stores the updated_at column. After update(), touch(), delete() or restore(), save() sees the ORM time as already saved, so it sets now. The new Stamped test checks this.
  • A time that the user sets after loading is still kept. It differs from the saved time, so save() keeps it. The existing test 'save keeps an updated_at that the user set' covers this and passes.
  • find(5, null) and findOrFail(5, null) tests now use await expectLater(...).

Checks: DRIVER=sqlite dart test passed (591 passed). dart analyze --fatal-infos packages is clean. dart format has no changes.

Non-blocking: CHANGELOG has a few near-duplicate lines (two about update() and a custom updated_at, two about find(5, null)). You can merge them later.

CI: lint passed. The test job (PG and MySQL) was still running when I checked.

@necessarylion
necessarylion merged commit dd222c2 into main Nov 6, 2026
2 checks passed
@necessarylion
necessarylion deleted the low-fixes branch November 6, 2026 06:14
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