the code looks good ! Isn't there any way to unregister the hook without registering a new one ? I suppose calling a js function on each row update has a significant performance cost, even if the function is empty.

the code looks good !

Thx ☺️

Isn't there any way to unregister the hook without registering a new one ?

Not according to the sqlite documentation. For the quite similar sqlite3_preupdate_hook the docs (first paragraph) say that you can disable that by passing NULL as a callback.

I suppose calling a js function on each row update has a significant performance cost, even if the function is empty.

Your're probably right.

But a typical use-case for the update hook is to enable it at the start of your application, maybe after loading your base data.
Then you keep that setup to track all changes to the db until your app is closed.

You'll very likely not ever enable the update hook if you have a workload that is affected by the performance cost of empty function calls. An alternative is also to close and open the database again to get rid of the update hook.

So, I'd like to stick to the official docs and not invent my own - but it's your call, I can also document that passing null will (most likely) disable it.

I also noticed that I need to somehow clean up the function pointer when the database connection is closed to not add a mem leak.
Also I should probably keep the original signature of callback(update, database_name, table_name, rowid) instead of omitting the database_name.

Will update the PR in the evening.

Also the CI job is failing weirdly (Puppeteer fails to start because of missing entryies in /sys), need to check why.

I opened a topic on the official sqlite forums: https://sqlite.org/forum/forumpost/652aef4747

Oh cool, I am curious as well. I also took a look at the c code for update_hook and preupdate_hook and with my limited knowledge of C did they look pretty similar. I also assume the pointer for the update_hook is null initially.

I opened a topic on the official sqlite forums: https://sqlite.org/forum/forumpost/652aef4747

That's now addressed https://sqlite.org/forum/forumpost/5ccd29dc29: NULL is the correct way to unset those, it just wasn't documented for update hooks. The website's docs won't reflect this change until 3.50 approaches, but the behavior has been unchanged since that API was introduced, so can be relied upon in pre-3.50 versions.

Woah that was fast! Thanks for clarifying this @sgbeal.

@lovasoa will update the PR to allow passing null to disable the hook.

Soo i investigated the CI action failure:

Looks like for some reason starting chrome in sandbox mode for the worker test fails.

Running the tests with RUN_WORKER_TEST_WITHOUT_PUPPETEER_SANDBOX=1 works. A sandbox is not required anyways as only code from this repository is tested and not arbitrary (well except all the npm deps 😄) code from the interwebs.

No idea why it fails with this PR but the last run of that action was 6months ago so maybe due to OS updates in Github runners.

@lovasoa any opinions, comments?

Can you rebase ? I removed the dependency to puppeteer altogether, and updated other dependencies

removed the now obsolete env var from CI.yml and rebased to the latest master