Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PRAGMA foreign_keys assumed to be ON in Electric #242

Closed
davidmartos96 opened this issue Jul 12, 2023 · 7 comments
Closed

PRAGMA foreign_keys assumed to be ON in Electric #242

davidmartos96 opened this issue Jul 12, 2023 · 7 comments

Comments

@davidmartos96
Copy link
Contributor

The Electric Satellite logic assumes that foreign_keys are ON but it never actually sets the PRAGMA. For example, when defer_foreign_keys is set to ON and there is error handling during transactions.

The tests work correctly by default when using the better-sqlite3 adapter because that dependency changes the SQLite foreign keys default to ON, but an "untouched" SQLite driver has foreign keys off by default. For instance, when querying PRAGMA foreign_keys in the example app from the monorepo, which uses wa-sqlite, it returns OFF.

Does it make sense to enable the PRAGMA internally in Electric, or is it something that the user has to set?

@icehaunter
Copy link
Contributor

@thruflo, any opinions? One one hand, we indeed assume that it's ON, on the other it maybe good that it's off for the upcoming shape stuff. But it's weird to work around FKs without them being enforced

@thruflo
Copy link
Contributor

thruflo commented Jul 13, 2023

Hmm. I think this is more a question for @balegas or @paulharter.

@balegas
Copy link
Contributor

balegas commented Jul 13, 2023

Will have a look at what would be a safe approach with the limitations we have right now.

@thruflo
Copy link
Contributor

thruflo commented Jul 20, 2023

Fwiw, I've just run up against the same issue, working on a data model with a foreign key with ON DELETE SET NULL and wondering why the child doesn't update when the parent is deleted.

This obviously doesn't mean that it's an Electric responsibility to set it but strikes me that it's a much saner default, coming from Postgres land. We obviously do quite a lot of setup in the generated migrations. Adding PRAGMA foreign_keys = 1 to the other setup would make sense to me.

@thruflo
Copy link
Contributor

thruflo commented Aug 31, 2023

@balegas just bumping thread — we do def want to move this into the client. Currently apps are having to set the right Pragma in their init code.

@balegas
Copy link
Contributor

balegas commented Sep 6, 2023

@kevin-dp can we close this?

@balegas
Copy link
Contributor

balegas commented Sep 7, 2023

Closed in #397

@balegas balegas closed this as completed Sep 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants