[wrangler/d1] Support migrations in a directory (multi-file migrations)#12703
[wrangler/d1] Support migrations in a directory (multi-file migrations)#12703Ehbraheem wants to merge 4 commits intocloudflare:mainfrom
Conversation
🦋 Changeset detectedLatest commit: b3405a9 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
| const files = fs.readdirSync(fullPath); | ||
| return files | ||
| .filter((f) => f.endsWith(".sql")) | ||
| .map((f) => path.join(fullPath, f)); |
There was a problem hiding this comment.
🔴 Missing sort on SQL files within a migration directory causes non-deterministic execution order
The resolveMigrationFiles function reads .sql files from a migration directory using fs.readdirSync() but does not sort the results before returning them. Node.js's readdirSync does not guarantee any particular ordering — it depends on the underlying filesystem. The PR description explicitly promises files are "executed in deterministic order", and the test uses files named 01_schema.sql and 02_seed.sql implying order matters (e.g., schema must be created before data is seeded).
Root Cause and Impact
At packages/wrangler/src/d1/migrations/apply.ts:247-250, the files are read from the directory and filtered but never sorted:
const files = fs.readdirSync(fullPath);
return files
.filter((f) => f.endsWith(".sql"))
.map((f) => path.join(fullPath, f));On many Linux filesystems (e.g., ext4 with dir_index), small directories may appear sorted, which is why tests pass. However, on other filesystems (XFS, Btrfs, or ext4 with hash-tree directories containing many entries), the order can differ. This means a migration directory containing 01_schema.sql (CREATE TABLE) and 02_seed.sql (INSERT INTO that table) could execute the INSERT before the CREATE, causing the migration to fail.
Impact: Migration directories with order-dependent SQL files may fail unpredictably depending on the filesystem, making this a latent production bug that's hard to reproduce in testing.
| const files = fs.readdirSync(fullPath); | |
| return files | |
| .filter((f) => f.endsWith(".sql")) | |
| .map((f) => path.join(fullPath, f)); | |
| const files = fs.readdirSync(fullPath); | |
| return files | |
| .filter((f) => f.endsWith(".sql")) | |
| .sort() | |
| .map((f) => path.join(fullPath, f)); |
Was this helpful? React with 👍 or 👎 to provide feedback.
|
Please could you rebase and resolve the conflicts. |
Fixes #11947.
Summary
Allow a migration to be either: