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

Add set_update_hook/2 #260

Merged
merged 8 commits into from
Sep 25, 2023

Conversation

greg-rychlewski
Copy link
Contributor

Closes #221

I took a stab at this based on how the erlang sqlite library is handling it.

The implementation is pretty straight forward. The user supplies a connection and a pid and we create a callback that sends a message to the pid of the form {:insert | :update | :delete, db_name, table, row_id} (that's all the information SQLite exposes about the change. For example, you could create a GenServer to listen to the notifications.

Right now, there can only be one pid per database connection. But I think it can probably be extended later to work with a list of pids.

@greg-rychlewski
Copy link
Contributor Author

This is exposed on the driver level but I think if someone really wanted to use it with Ecto they could use the after_connect configuration option and supply {Exqlite.Sqlite3, :set_update_hook, [genserver_name]}.

I believe this will work and also be called on reconnects but haven't tested. That's why I didn't advertise it in the docs yet.

Comment on lines 22 to 27
typedef struct connection
{
sqlite3* db;
ErlNifMutex* mutex;
ErlNifPid update_hook_pid;
} connection_t;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we could capture the App.Repo atom and store it with the connection and pass that in the callback instead of the raw db name. This would probably change how the sqlite database is opened.

Co-authored-by: Matthew Johnston <warmwaffles@gmail.com>
Copy link
Member

@warmwaffles warmwaffles left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is good. I think this is probably the simplest way to implement this without having to do some changes on connection construction.

@warmwaffles warmwaffles merged commit e00b272 into elixir-sqlite:main Sep 25, 2023
22 checks passed
@greg-rychlewski
Copy link
Contributor Author

Oh I was looking into whether Ecto passes the repo into the connect function. Would you be interested in a separate PR for that if it works?

@warmwaffles
Copy link
Member

Yea I think that would be fine. I have a feeling it may be a breaking api change. Not entirely sure.

@greg-rychlewski greg-rychlewski deleted the set_update_hook branch September 25, 2023 23:31
@ruslandoga ruslandoga mentioned this pull request Oct 19, 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

Successfully merging this pull request may close these issues.

Feature request: Add support for Data Change Notification Callbacks
2 participants