GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | |||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | |||||||||||||||||||
GeezerVison is showing github.com — fix(data): stream the hash pass, so memory stays flat however large the table by jasdeepkhalsa · Pull Request #265 · DBDiff/DBDiff Conversation
Member
The data diff's streaming merge (every PostgreSQL data diff, and cross-server MySQL) only looked like streaming. The bug
The fix: new
Why PostgreSQL is adaptive: cursors cost speed. PDO can't fetch the next batch while PHP compares the current one, so server and client take turns. At 2M rows that made a diff about 50% slower. So a table PostgreSQL expects to have up to about 1M rows keeps the buffered read, which needs at most about 180 MB.
Measured (PHP 8.3, PostgreSQL 17, MySQL 8.0; output byte-identical to before in every case):
What memory still grows with is the number of differences found, which it has to hold to write the migration. Tests: new
Locally:
🤖 Generated with Claude Code
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions…he table PDO's pgsql and mysql drivers buffer a whole result before the first fetch, so the streaming merge held every row's key and hash at once: about 90 bytes a row a side, past the default 1G limit at a few million rows. PostgreSQL now reads a large table through a cursor (a small one keeps the faster buffered read) and MySQL unbuffered. Same output; at 2M rows peak memory falls from 353 to 35 MB on PostgreSQL and 372 to 35 MB on MySQL. Co-Authored-By: Claude Opus 5.5 <[email protected]> …hort Dolt ignores cte_max_recursion_depth, so the fixture failed before any diffing ran. A cross join of digits works on both; the 400k-row memory test is skipped on Dolt, which shares MySQL's code path. Co-Authored-By: Claude Opus 5.5 <[email protected]> …etch split up No change in behaviour. Addresses SonarCloud's complexity and return-count findings on the streaming merge. Co-Authored-By: Claude Opus 5.5 <[email protected]>
All reactions
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. Uh oh!
There was an error while loading. Please reload this page.
jasdeepkhalsa
added a commit
that referenced
this pull request
**Builds on #265.** Merge that first; this PR's diff includes #265's commit until then. **The bug** (present in rc.25 and earlier) - The data diff's streaming merge walks both tables in primary-key order and pairs rows up. - The database sorted text keys one way, and PHP compared them another: - **Numeric-looking text** (`'10'`, `'9'`): the server sorts as text (`'10' < '9'`), but the merge compared them as numbers. - **Linguistic collations** (PostgreSQL `en_US` / ICU, MySQL's default `utf8mb4_0900_ai_ci`): the server sorts `'a' < 'B'`, but the merge compared bytes (`'B' < 'a'`). - When the two orders disagree at a difference, the merge falls out of step and reports rows **both sides have** as a `DELETE` and an `INSERT`. - Replaying that leaves the row itself intact. But an `ON DELETE CASCADE` child goes with it, delete triggers fire, and the migration is wrong either way. Reproduced on PostgreSQL 18 (glibc `en_US.utf8`) and MySQL 8.0. For example, a target missing only `'a'`, `'apple'` and `'10'` produced: ```sql DELETE FROM `t` WHERE `code` = '9'; DELETE FROM `t` WHERE `code` = 'B'; INSERT INTO `t` (`code`,`v`) VALUES('10','5'); INSERT INTO `t` (`code`,`v`) VALUES('9','4'); INSERT INTO `t` (`code`,`v`) VALUES('B','2'); INSERT INTO `t` (`code`,`v`) VALUES('a','1'); INSERT INTO `t` (`code`,`v`) VALUES('apple','6'); ``` With this fix, the same diff is just the three `INSERT`s. PostgreSQL images on Alpine sort `en_US` as bytes, so they hide the problem; Debian images and Supabase don't. **The fix:** each key column is ordered by the server and compared by the merge the same way, chosen from its type in the catalog. | Key type | Server `ORDER BY` | Merge compares | |---|---|---| | Number | native (index used) | as numbers | | uuid (PostgreSQL) | native (index used), which is already hex order | bytes | | Anything else | its text as bytes (`::text COLLATE "C"` / `CAST(… AS BINARY)` / SQLite `COLLATE BINARY`) | bytes | Integer and uuid keys, the common case, sort exactly as before. Text keys now sort by bytes, which the server can't serve from the index, so it sorts instead. **Tests** (all fail on the old code, all pass with the fix) - `StreamingMergeDiffTest` (SQLite, runs everywhere): - a text key holding `'9'` and `'10'` gives one `INSERT`, not a delete and re-insert; - `comparePK` follows the key type. - `StreamingMergeServerTest`, PostgreSQL (ICU collation where available) and MySQL: text keys mixing case and numbers give exactly the expected inserts, deletes and updates. Locally: - Postgres suite: 225 passed. - MySQL `DBDiff,Unit`: 1,083 passed. - The 2M-row integer-key benchmark gives byte-identical output in the same time. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
jasdeepkhalsa
added a commit
that referenced
this pull request
…dles json (#267) **Builds on #265 and #266.** Merge those first; until then this PR's diff includes their commits. Tables without a primary key were already diffed **correctly** at small sizes, on PostgreSQL, MySQL and SQLite: whole rows are matched, duplicates are counted, each `DELETE` removes exactly one copy (`ctid` / `LIMIT 1` / `rowid`), and NULLs match with `IS NULL`. I checked each by applying the migration and comparing. Three problems remained. **1. Memory and time** - Both tables were paged with `ORDER BY` every column, using `OFFSET` (quadratic), and every row was held in PHP. - A 501k-row keyless table took **77 s and 984 MB**; at 500k rows with changed timestamps it ran out of the default 1G limit. **2. json columns could not be diffed at all** - PostgreSQL can't order `json`, so the paging query itself failed: > could not identify an ordering operator for type json **3. The `DELETE` could not match a json value** - PostgreSQL `json` (and `xml`, `point`, …) has no `=` operator. - MySQL `JSON` compares unequal to a string. - Dolt even reads JSON as `{"a":1}` but casts it to `{"a": 1}`. **The fix** **Phase 1:** each server groups its rows by a hash of the whole row and counts them, in hash order: ```sql SELECT h, n FROM (SELECT <hash of every column> AS h, count(*) AS n FROM t GROUP BY h) g ORDER BY h ``` - The two sorted streams are merged with the same `RowStream` as #265. Different counts become that many inserts or deletes. - Memory follows the number of distinct differences, not the table. **Phase 2** runs only when something differs. It's one pass over each side, picking up one copy of each row to insert or delete. **json matching:** the catalog marks the columns with no plain equality, and the `DELETE` matches them by text (PostgreSQL `::text`) or as JSON (MySQL/Dolt `= CAST(… AS JSON)`). Other columns render exactly as before. **Measured** (PostgreSQL 17, PHP 8.3) | Keyless table, 501k rows, 500 differ | Before | After | |---|---|---| | Time | 77.4 s | **4.4 s** | | Peak memory | 984 MB | **107 MB** | - The same statements as before, ordered by hash instead. - Applying the UP makes the target identical to the source (row-set md5 compared). **Tests** - **`KeylessDataRoundTripPostgresTest`** (CLI, end to end): a keyless table with duplicates, NULLs, `bytea`, `json` and `timestamptz`. The generated UP must turn the target into the source and the DOWN must restore it. On the old code it fails with the json ordering error above. - **`StreamingMergeServerTest`**, PostgreSQL and MySQL: - the same round trip in-process; - a 400k-row keyless table must grow peak memory by under 25 MB. Locally, all passing: | Suite | Result | |---|---| | Postgres + Corpus | 383 passed | | MySQL 8.0 `DBDiff,Unit` | passed | | Dolt 2.2.3 `DBDiff,Unit` (as the Dolt job runs it) | passed | | SQLite | 16 passed | | Unit on PHP 8.4 / 8.5 | passed | 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
ReviewersNo reviewsUh oh!
There was an error while loading. Please reload this page. AssigneesNo one assignedLabelsProjectsMilestoneNo milestoneDevelopmentSuccessfully merging this pull request may close these issues. Uh oh!
There was an error while loading. Please reload this page. Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later. | |||||||||||||||||||
| You are visitor 01402503 — GeezerVison does not store the pages you look up. Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required. |