Add updateHook by hoeck · Pull Request #604 · sql-js/sql.js (original) (raw)

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.