Feature: Support for multiple "SQLiteClientTypes" (Storage Options)

[thorsten-wolf-neptune]

Hi @etiennenoel,

first of all thanks a lot for this easy to use and awesome client library!
Since the deprecation of websql this is the backbone of our solution.
Unfortunately we currently are tackling some performance issues that occur during big data operations (see sqlite/sqlite-wasm#61). So we want to try using the OPFS SAH Approach (like mentioned in the issue) as well as for other scenarios maybe even the memory db approach.

As we really love your client library (very nice implementation, easy to use and understand) i would like to contribute to this project by allowing different Client Types:

• OPFS: current implementation
• OPFS_SAH: SyncAccessHandle Pool approach for max performance but with some tradeoffs (see issue of sqlite-wasm repository)
• MEMORY: non persisted data but running in memory (either on the main thread or even in the worker to distribute memory consumption of the threads)

I forked this sqlite-client repository and implemented these new client types. I tried to stick as much as possible to your coding conventions. But please feel free to change it or give critical feedback. For the creation of the various client types i introduced a SqliteClientFactory that would return the corresponding SQLiteClient instance. Also i wanted to be backwards compatible so you can continue using new SqliteClient(filename, "c", webSqliteWorkerPath) (would create the OPFS client type) so existing code should still work.
In addition to this repository i also forked your https://github.com/neptune-software/sqlite-client-demo repository to play with the endresult of the new options.
Here some screenshots to give you an idea how it looks like:
Plain Memory DB running inside the main thread:

Plain Memory DB running inside the worker thread:

"Legacy Client" (instantiate the client with new SqliteClient(filename, "c", webSqliteWorkerPath) to be backwards compatible will create an OPFS client :

"OPFS_SAH" client (instantiate the client with the new factory

SqliteClientFactory.getInstance({
        clientType: clientType,
        options: {
          filename: filename,
          flags: "c",
          sqliteWorkerPath: webSqliteWorkerPath,
          useWorker: useWorker,
        }
      });

Testing the implementation

Step1:

clone my fork and switch to the branch feature/differentClientTypes
https://github.com/neptune-software/sqlite-client/tree/feature/differentClientTypes
In the new cloned repo run npm i and npm run package and npm link (to get a local link to this repository on the machine)

Step2:

clone the other forked repo and also switch to the branch feature/differentClientTypes
https://github.com/neptune-software/sqlite-client-demo/tree/feature/differentClientTypes
run cd examples/vanilla-js
run npm i
run npm link @magieno/sqlite-client (that will use the local version of the sqlite-client repository)
run npm run build
run npm run start:server
open the browser with http://localhost:3000/

[etiennenoel]

Thank you so much for doing this! I'm currently travelling but will take a look as soon as I can.

I totally support the idea behind the changes, will look at the code and will provide feedback. Thanks again!

[thorsten-wolf-neptune]

cool. No worries and no stress :-)
Now that i look at the code again it might be better to split the "Memory main thread" and "Memory Service Worker" approach just by two clientTypes "MEMORY_MAIN" and "MEMORY_WORKER" maybe instead of adding a useWorker boolean flag.
But anyways looking forward to your feedback and safe travels!

[etiennenoel]

Usually, I'm a big fan of having multiple client types so things are well decoupled so I would agree with you.

[thorsten-wolf-neptune]

cool i separated them with the last commit also adding your code guidelines

[etiennenoel]

Ok thank you for the update. Looking at this more carefully, here are a few high level elements:

• I like passing an options to the constructor. We could have some mandatory and optional parameters so I like this idea.
• I wonder if we should have multiple clients or just one where the type of storage is configurable via an enum maybe?
  • The reason is that I want this library to be very easy to use sqlite without understanding much of what is happening and I feel multiple clients might be confusing. That being said, I like the general logic of decoupling the clients into their specific files. Maybe, the sqlite.client.ts could be the default entrypoint still but underneath, it instantiates the right proxy? I think the proxy mechanism is also a good idea.

Let me know what you think.

[thorsten-wolf-neptune]

I like passing an options to the constructor. We could have some mandatory and optional parameters so I like this idea.

I agree. With an object passed we are a lot more flexible to enhance that in the future and not rely on multiple separate parameters.

I wonder if we should have multiple clients or just one where the type of storage is configurable via an enum maybe?

Agree. We should try to make that library as easy to use as possible. I can give it a try if you want with your suggestions or do you want to change it?

[etiennenoel]

I'll take a jab at it.

[etiennenoel]

What do you think of such a behaviour? #4

I made it very barebone just to understand if it would make sense.

Will add all the other Client Types and will try to make it backwards compatible by keeping the parameters in SqliteClient and by instantiating by default the OpfsWorker Client.

Let me know if as a developer, this API would suit you.

Second, as a contributor, let me know if you think the structure would make sense to you. Thanks!!

[thorsten-wolf-neptune]

I checked out your branch and I love it! It makes a lot more sense for external useage that you just work with the sqliteClient and internally we have then an adapter that handles the communication depending on the type.
This would totally work for us and probably everyone else too 😊

Let me know if you want me to also do something or you want to finalize the missing clientTypes on your end.

[etiennenoel]

Great! I will update it for the remaining clients and will let you know! ETA, probably should have something done by early next week, will try sooner.

[etiennenoel]

I've made the modifications and published a test version at 3.45.1-test1. I've updated my PR with the latest code so if you can test it out and let me know how it works. We can continue to make some changes and fix some issues.

Meanwhile, my next steps are to add an automated test infrastructure and I also need to update the readme.

[thorsten-wolf-neptune]

Very cool and nice implementation. That makes so much more sense now and is easy to understand.

I tested it and stumbled across an issue for the worker scenarios. The promise seems to be created too early as it uses the message instances that are not yet created. That will lead to an error both for the createDatabaseMessage and the executeSqlMessage:


When i rearrange the code so the messages are instantiated at the beginning it works 😊

One suggestion: should we maybe have a "defaults" object that one can add in the creation of the client that would contain a default returnValue and rowMode so you can set that up once and don't have to pass it on each executeSql call? Also to be backwards compatible you can think of having the values that were there statically in the previous version that was Array and ResultRows.

[etiennenoel]

Oops, yes absolutely, an oversight on my part. I moved the the promise above to ensure that when there's an answer from the worker, that we always have the promises saved but I moved the code above the creation of the message. I've published a new test version: 3.45.1-test2

I'm setting the test infrastructure in this project, I should be able to publish an official version next week.

[etiennenoel]

I've updated the code, setup the test infrastructure, I've updated the README.md and published a new version: 3.45.1-build1

Let me know how it works and if you have any other ideas! Thanks a lot for your time.

[thorsten-wolf-neptune]

Sorry for my late reply. It looks very good and works perfectly! i would suggest also updating the client-demo playground.
I updated my PR to use the new logic and also removed the legacy client.
To test the OPFS_SAH version i also added a script that won't return the coop header fields.
magieno/sqlite-client-demo#1