Skip to content

fix(db): fix support for sqlite3 connector#2009

Merged
jethrokuan merged 1 commit intomasterfrom
fix/sqlite3
Dec 21, 2021
Merged

fix(db): fix support for sqlite3 connector#2009
jethrokuan merged 1 commit intomasterfrom
fix/sqlite3

Conversation

@jethrokuan
Copy link
Copy Markdown
Member

Moving the PRAGMA foreign_keys invocation out of the transaction seems
to work now.

Moving the PRAGMA foreign_keys invocation out of the transaction seems
to work now.
@jethrokuan
Copy link
Copy Markdown
Member Author

cc @nobiot if you want to confirm :)

@nobiot
Copy link
Copy Markdown
Contributor

nobiot commented Dec 21, 2021

@jethrokuan
Thank you. Will check on my end :)

@nobiot
Copy link
Copy Markdown
Contributor

nobiot commented Dec 21, 2021

See detail of my tests in #1927.
I have confirmed that the fix works on Windows 10 :)

@nobiot
Copy link
Copy Markdown
Contributor

nobiot commented Dec 21, 2021

@jethrokuan
Ubuntu struggles with sqlite3.

Ubuntu's Sqlite3 package gives me an older version (and has the same issue with the newest) so I compiled from source.
Creating new files seems to be OK (ish) but updating the same file gives me an error.

Screenshot from 2021-12-21 12-47-13

@nobiot
Copy link
Copy Markdown
Contributor

nobiot commented Dec 21, 2021

Maybe the Widows binary does something special?

nobiot added a commit to nobiot/org-roam that referenced this pull request Dec 27, 2021
This commit is follow-up for PR org-roam#2009 should fix issues org-roam#1927 & org-roam#1910.

Currently, `[:pragma (= foreign_keys ON)]` is present only in function
`org-roam-db--init`. This means the `foreign_keys` is turned on only when the db
file is created for the first time. Subsequent Emacs sessions won't turn it on
as the db file is already present and does not evaluate `org-roam-db--init`.

This PRAGMA needs to be turned on each database connection at runtime according
to sqlite's documentation (https://sqlite.org/foreignkeys.html#fk_enable)

```
Foreign key constraints are disabled by default (for backwards compatibility),
so must be enabled separately for each database connection
```

I have observed that on Windows only the Emacs session that initially creates
the Org-roam db file works correctly with the `sqlite3` option. Subsequent Emacs
sessions have the same "Unique constraint failed" error messages.

I have tested this patch on Windows and Ubuntu; both seem to work in the first
and subsequent Emacs sessions.

I am not 100% sure if the location of the PRAGMA is the best; use it as a
proof-of-cocenpt for the fix. Thank you.
jethrokuan pushed a commit that referenced this pull request Jan 2, 2022
* (fix):db: pragma foreign keys to work with sqlite3

This commit is follow-up for PR #2009 should fix issues #1927 & #1910.

Currently, `[:pragma (= foreign_keys ON)]` is present only in function
`org-roam-db--init`. This means the `foreign_keys` is turned on only when the db
file is created for the first time. Subsequent Emacs sessions won't turn it on
as the db file is already present and does not evaluate `org-roam-db--init`.

This PRAGMA needs to be turned on each database connection at runtime according
to sqlite's documentation (https://sqlite.org/foreignkeys.html#fk_enable)

```
Foreign key constraints are disabled by default (for backwards compatibility),
so must be enabled separately for each database connection
```

I have observed that on Windows only the Emacs session that initially creates
the Org-roam db file works correctly with the `sqlite3` option. Subsequent Emacs
sessions have the same "Unique constraint failed" error messages.

I have tested this patch on Windows and Ubuntu; both seem to work in the first
and subsequent Emacs sessions.

I am not 100% sure if the location of the PRAGMA is the best; use it as a
proof-of-cocenpt for the fix. Thank you.

* remove: [:pragma (= foreign_keys ON)] from org-roam-db--init

* fix: defconst org-roam--sqlite-available-p outputs error

When `org-roam-database-connector` is not `sqlite`, it outputs an unnecessary
error when Org-roam starts up as `defconst` evaluates
`(emacsql-sqlite-ensure-binary)`.

```
Org-roam initialization: (error "No EmacSQL SQLite binary available, aborting")
```

For other database connectors, this is not relevant.

* chore: rm org-roam--sqlite-available-p

As per our conversation, org-roam--sqlite-available-p not needed.
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 7, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants