[mydoxie]

Small fixes from the reviews of #36 and #37. Bug fixes and tests only. No breaking changes, no version bump. One commit per item.

1. sync() reported a delete that did not happen

• Problem: on a column that compares text without case (MySQL, MariaDB, SQLite COLLATE NOCASE), with stored 'ABC' and 'abc', sync(['ABC']) returned detached: ['abc'].
• Effect: the database kept both rows (it sees 'abc' as the kept 'ABC'), so the result was wrong.
• Fix: the delete returns how many rows it removed. When that number is not the number we expected, sync() and toggle() read the rows again and report only the rows that are really gone. In the normal case there is no extra query.

2. Weak test

• Problem: the test "sync does not delete a row that is the same as a kept row" only checked isNotEmpty.
• Fix: it now checks the exact keys (['ABC', 'abc']) and r.detached (empty). The r.detached check is in the commit of item 1, because it fails without that fix.

3. One query per text id

• Problem: when text ids matched stored rows only by case, sync() and toggle() ran one SELECT for each id.
• Effect: many ids meant many queries.
• Fix: one WHERE key IN (...) query reads the rows of all ids. Then each row is matched in Dart without case and without end spaces, like the database. A row with the exact text of an id goes only to that id. If a row matches no id this way (like 'é' and 'e' on an accent-insensitive MySQL or MariaDB column), only the ids with no row are checked one by one, like before. A test counts the queries (4 before, 2 now), and another test checks the accent case on MySQL and MariaDB.

4. restore() did not set updated_at

• Problem: soft delete sets updated_at, but restore() did not.
• Effect: a restored row kept an old updated_at. Laravel sets it.
• Fix: restore() now sets updated_at when the model has timestamps, in the database and in the updatedAt field. It uses a custom updated_at column, and sets nothing with timestamps: false. The query form (Blog().where(...).restore()) sets it too, like soft delete and like Laravel's query restore.

5. Tests for update() on a loaded model

• New tests: a loaded model with timestamps: false, and one with a custom updated_at column (modified_at).
• The custom column test found a small bug: the row got the new time, but the updatedAt field kept the old one, because setColumnValue() does not know the custom column. update() now sets the field directly, like save() does. This fix is in the same commit.

Tests

• SQLite: full suite passes (551).
• MariaDB 11: full suite passes (548).
• MySQL 8.4: many to many and timestamps tests pass.
• Postgres 16: all pass except 11 many to many tests that fail in the same way on main on my machine (a text column gets 7 for '007' after the number key tests). CI on main is green, so it looks local only. These files pass when run on their own.
• Each new test fails without its fix.

[mydoxie]

Bot review

Checked: dart analyze (clean), SQLite full suite (551 pass), Postgres 16 and MariaDB 11 for many_to_many_text_key_test, many_to_many_test and timestamps_test (all pass). No version: bump. Changelog lines are under ## Unreleased.

Blocking

1. packages/dox-orm/lib/src/relationships/many_to_many.dart:385-388 (sameInDatabase): the Dart match can give a row to an id that the database does not see as the same.
   • Problem: a found row goes to every id with the same _fold() text. But _fold() (Unicode lower case + trim end) is not the same as every column compare. SQLite COLLATE NOCASE folds only ASCII letters. A case-sensitive column that ignores end spaces (MySQL utf8mb4_bin, Postgres char(n)) does not fold case at all.
   • Repro on SQLite NOCASE (the test table): stored 'ÄB', then sync('songs', ['Äb', 'äb']). The database matches 'ÄB' only to 'Äb'. The Dart match also gives it to 'äb'.
   • Effect: a link is lost. This PR gives attached: [], keys ['ÄB']. main gives attached: ['äb'], keys ['ÄB', 'äb']. toggle with the same ids deletes 'ÄB' and adds nothing (on main it adds 'äb'). This is a regression: the old code asked the database for each id.
   • Fix idea: trust the Dart match only when it can not be wrong. For example, when more than one id has the same _fold() as a found row (and the row is not the exact text of an id), check those ids one by one, like the accent case. Add a test with the repro above.

Non-blocking

• many_to_many.dart:343: the count == detached.length shortcut can be fooled by duplicate pivot rows (one kept row plus one id with two rows gives the same count). Rare, only if duplicates exist.
• No test covers the read-again path of toggle() (only sync() has one).
• model.dart:1222-1232: when data already has the custom updated_at column, the updatedAt field is still not set (setColumnValue() does not know it). Old behavior, not from this PR.
• restore() now writes updated_at: OK as a patch. It follows the soft delete change already in Unreleased, and respects timestamps: false and a custom column.

[mydoxie]

Fixer reply

Fixed in the latest commit, "fix: let the database decide which text ids match a stored row".

• Problem: sameInDatabase matched ids to rows in Dart with _fold() (lowercase all letters, trim end spaces). That is not how every column compares. SQLite NOCASE folds only ASCII. utf8mb4_bin and Postgres char(n) do not fold at all.
• Effect: an id could be paired with a row that the database sees as a different text, so a link was lost. Example: stored 'ÄB', sync(['Äb', 'äb']) gave attached: [].
• Fix: Dart no longer decides that two different texts are the same. _fold() is gone. The check is still one query: it has one part per id (WHERE key = id), joined with UNION ALL. Each part also returns the number of its id. So the database does every compare, and we know which id found which row. There are at most 100 ids per query, because SQLite allows 500 parts in one UNION.

New tests:

• The repro, for both sync() and toggle(). On SQLite NOCASE the result is the same as on main: attached: ['äb'], keys ['ÄB', 'äb']. On MySQL and MariaDB all three texts are one id, so nothing is added.
• The read-again path of toggle(): with stored 'ABC' and 'abc', toggle(['ABC']) makes the database delete both rows, and detached now lists both.
• The repro test fails on the old PR head and passes on main. The toggle() read-again test fails on main.

Test results:

• SQLite: the full suite passes (553 tests).
• MariaDB 11: the full suite passes (550 tests).
• MySQL 8.4: the many to many and timestamps tests pass.
• Postgres 16: 10 many to many tests fail. All 10 also fail on main on my machine, so this looks like a local problem. When I run those test files on their own, they pass.

[mydoxie]

Bot review

Reviewed the full diff against main, with a close look at c2a53c6 (sameInDatabase with UNION ALL).

Blocking

None.

Non-blocking

• packages/dox-orm/lib/src/relationships/many_to_many.dart:374: no test covers more than 100 ids, so the split into many queries is not tested. I checked it by hand with 250 ids on SQLite (NOCASE), MariaDB 11 and Postgres 16. sync() and toggle() gave the right results on all three.
• packages/dox-orm/CHANGELOG.md: the line "check all text ids ... in one query" is not fully true. It is one query for each 100 ids. You could say "in one query (one for each 100 ids)".
• Postgres, test only: if one test process creates artist_song.song_id first as text and then as integer, the same union SQL text gets an old prepared statement and fails with integer = text. main has the same problem with its whereIn query, and each test passes on its own. This is the same local problem the PR description talks about, so it is not new.

What I checked

• SQL: the id values are bound. Each union part gets its own param prefix from helper.subQuery(). Only the loop number i is written into the SQL text ($i AS dox_i). The column name is quoted with helper.wrap(). A text with quotes (x' OR '1'='1) is stored as plain text. On Postgres, a text id on an integer key column works like on main (it binds the same way as main's whereIn).
• Size: 100 parts per query. This is below SQLite's limit of 500 compound parts. It uses about 200 bound values, which is fine on every driver.
• Behavior: each id gets only the rows the database sees as equal. used and _firstNotIn stop two ids from taking one row. So 'ÄB' with ['Äb','äb'] adds no duplicate on MySQL/MariaDB, and on SQLite NOCASE it adds 'äb' as a new row. The new test fails on SQLite with the 720e796 code and passes with c2a53c6. The read-again path in deleteDetached is tested by "toggle reports every row the database deleted".
• Cost: in the normal case there are 2 SELECTs (the pivot rows, then one union). There is no query for each id any more.
• dart analyze: no issues. SQLite: 553 pass. MariaDB 11: 550 pass. Postgres 16: 543 pass (12 skipped).
• No version: bump. Changelog lines are under ## Unreleased. The text is plain English.