cloudflare / workers-sdk

⛅️ Home to Wrangler, the CLI for Cloudflare Workers®
https://developers.cloudflare.com/workers/
Apache License 2.0
2.42k stars 593 forks source link

🚀 Feature Request: Allow turning off foreign key check while migrating database. #5438

Open aperture147 opened 3 months ago

aperture147 commented 3 months ago

Some table altering tasks in SQLite cannot be achieved without replacing it with a new table (by dropping - recreating the table) (like adding foreign keys, changing primary keys, updating column type). Dropping a table which has column referenced by other tables with ON DELETE CASCADE will delete all records of the child tables.

Example: Assume that we have Table A and B. Table B has column a_id referencing (ON DELETE CASCADE) table A primary key id. For some reason I have to modify Table A by dropping and recreating table A (with original value), ALL records from table B are deleted by cascading. I'm trying to find a workaround for this situation as mentioned in this sqlx issue and some suggestion from Matt in Discord, but none of them work:

  1. Try bracketing the migration with PRAGMA foreign_keys = OFF; and PRAGMA foreign_keys = ON;:
PRAGMA foreign_keys = OFF;
create table a_temp ...;
insert into table a_temp select from a ...;
drop table a;
alter table a_temp rename to a;
PRAGMA foreign_keys = ON;
  1. Try bracketing the migration with PRAGMA defer_foreign_keys = OFF; and PRAGMA defer_foreign_keys = ON;:

    PRAGMA defer_foreign_keys = ON;
    create table a_temp ...;
    insert into table a_temp select from a ...;
    drop table a;
    alter table a_temp rename to a;
    PRAGMA defer_foreign_keys = OFF;
  2. Try committing the current transaction and create a new one later:

    
    COMMIT TRANSACTION;
    PRAGMA foreign_keys=OFF;
    BEGIN TRANSACTION;

create table a_temp ...; insert into table a_temp select from a ...; drop table a; alter table a_temp rename to a;

COMMIT TRANSACTION; PRAGMA foreign_keys=ON; BEGIN TRANSACTION;


The workaround `1` and `2` completed but all records in table B is still deleted. The workaround `3` threw and error like this:

✘ [ERROR] Wrangler could not process the provided SQL file, as it contains several transactions.

D1 runs your SQL in a transaction for you. Please export an SQL file from your SQLite database and try again.


Currently I have to copy data from table B to a temporally table then reinsert it to table B later.

```sql
create table b_temp;
insert into table b_temp select from b ...;

create table a_temp ...;
insert into table a_temp select from a ...;
drop table a;
alter table a_temp rename to a;

insert Into table b select from b_temp ...;
drop table b_temp;

This is a huge problem when I could have table B1, B2, B3 and more, all of them reference to table A, each table contains a few hundred thousands of records and more important, table B1, B2, B3 could be referenced from other tables too. This is really inefficient since it would definitely make a hit to billing metrics and might impact the traffic and make a migration takes longer to finish (which might affect the traffic to my D1).

Will there be a feature address this situation or is there any existing solution?

aperture147 commented 3 months ago

Hi, is there anyone reading this issue?

geelen commented 2 months ago

Hi @aperture147, my apologies. I'm looking into this exact issue as part of https://github.com/cloudflare/workers-sdk/issues/5683. Will make a record to update this issue once I have a resolution.

aperture147 commented 2 months ago

Hi @aperture147, my apologies. I'm looking into this exact issue as part of #5683. Will make a record to update this issue once I have a resolution.

Thanks. It would be nice to have disable foreign key check option on remote migration. For now I still have to drop and recreate every single tables and recreate it again to add foreign keys.

Gerbuuun commented 1 month ago

Just want to add to this that this is indeed very annoying. Any changes to a table that requires recreating it will cascade to other referenced tables. I cannot make the required changes to my database atm.. (at least not trivially)

colecrouter commented 4 weeks ago

I kept running into various errors trying something similar (foreign key constraint failed, LLVM syntax errors). I noticed @aperture147's second example is backwards.

PRAGMA defer_foreign_keys = ON;

CREATE TABLE ...

PRAGMA defer_foreign_keys = OFF;

Doing it exactly like this worked for me. Hope this helps.

Not sure exactly where my issue was. Possible causes:

Docs are a bit messy as far as this goes; one page uses true/false, another uses on/off. Neither use semicolons.

Gerbuuun commented 4 weeks ago

Does not work for me. Rows are still deleted or set to null.

edit: this is my migration file btw

PRAGMA defer_foreign_keys = ON;
--> statement-breakpoint

DROP INDEX `user_email_idx`;
--> statement-breakpoint
DROP INDEX `user_public_idx`;
--> statement-breakpoint
CREATE TABLE `new_user` (
    `id` integer PRIMARY KEY AUTOINCREMENT NOT NULL,
    `public_id` text NOT NULL,
    `first_name` text NOT NULL,
    `last_name` text NOT NULL,
    `email` text NOT NULL,
    `verified` integer NOT NULL,
    `created_at` text NOT NULL,
    `updated_at` text NOT NULL,
    `deleted_at` text
);
--> statement-breakpoint
INSERT INTO `new_user` (`id`, `public_id`, `first_name`, `last_name`, `email`, `verified`, `created_at`, `updated_at`, `deleted_at`)
SELECT `id`, `public_id`, `first_name`, `last_name`, `email`, `verified`, `created_at`, `updated_at`, `deleted_at`
FROM `user`;
--> statement-breakpoint
DROP TABLE `user`;
--> statement-breakpoint
ALTER TABLE `new_user` RENAME TO `user`;
--> statement-breakpoint
UPDATE sqlite_sequence SET seq = (SELECT MAX(ID) FROM `user`) WHERE name = 'user';
--> statement-breakpoint
CREATE UNIQUE INDEX `user_public_idx` ON `user` (`public_id`);
--> statement-breakpoint
CREATE UNIQUE INDEX `user_email_idx` ON `user` (`email`);
--> statement-breakpoint

PRAGMA defer_foreign_keys = OFF;
--> statement-breakpoint

edit 2: first renaming the old table and then creating the new table (which is highly discouraged by the sqlite docs) also does not work

aperture147 commented 3 weeks ago

I kept running into various errors trying something similar (foreign key constraint failed, LLVM syntax errors). I noticed @aperture147's second example is backwards.

PRAGMA defer_foreign_keys = ON;

CREATE TABLE ...

PRAGMA defer_foreign_keys = OFF;

Doing it exactly like this worked for me. Hope this helps.

Not sure exactly where my issue was. Possible causes:

  • Not capitalizing ON/OFF
  • Not turning off after
  • Not putting it at the very start/end of file
  • Missing semicolon

Docs are a bit messy as far as this goes; one page uses true/false, another uses on/off. Neither use semicolons.

Hi, sorry for the wrong example. I've corrected the original example. Actually I've tried defer_foreign_keys = ON then defer_foreign_keys = OFF first but it does not work, so I later tried OFF then ON (although it does not make any sense) and it expectedly does not work either. That's why the example has the defer_foreign_key backward.

It seems like defer_foreign_keys only works for inserting new records, I've tried defer_foreign_keys for inserting new records and it works. But it does not work on deleting old records or dropping the table as shown.

hrueger commented 3 weeks ago

To be honest, I'd probably rather mark that as a bug - I almost lost data today due to a migration silently setting those values to NULL 😬 If you need an easily runnable reproduction, checkout https://github.com/hrueger/prisma-24540 (not prisma specific). If there's anything I can test or help, just let me know 👍