touchlab/SQLiter

Use sqlite amalgamation for compatibility

chippmann opened this issue · 3 comments

I propose to use the sqlite amalgamation (https://www.sqlite.org/amalgamation.html) rather than linking the libsqlite3 library which is installed on the host.

This has several advantages:

  • It is more portable:
    • as it does not rely on the sqlite version installed on the building host
    • as it does not depend on installed library versions on the building host (like glibc, which was an issue in this PR). This also means that developers on linux can run the tests locally as well even with very recent linux installs
  • Consumer of this library don't have to link sqlite themselves
  • We don't have to link sqlite ourselves
  • It should (at least according to the sqlite docs) give a slight performance boost

While i already have a working prototype branch using sqliter in conjunction with sqldelight one test fails:


Can you @kpgalligan tell me why and what this hardcoded timeout is and why it's set to 1300?
Because with my prototyping branch the actual value is slightly lower (around 1000) and hence I'm unsure if this is a bug in my prototyping branch or not.

The timeout in the config is 1500, so I'm thinking I picked 1300 as a reasonable value that would mean the writer waited for an amount of time. It won't always be "1500" or greater, because we're creating a statement and starting a query. You could rewire that to start the timer before starting the read I guess?

It's a relatively obscure config, as most people would pick WAL. Android, by default, does not use WAL, even though Room does. Were it not for Android support, I would likely only support WAL in the driver.

Anyway, on "amalgamation", I'm not quite sure I understand. If it's just shipping the sqlite binary rather than linking to the one on from the host, I would say that depends on the situation. If we're talking just linux, I don't have much of an opinion, to be honest. If we're talking about also doing that for iOS, while I understand there are some benefits, I would implement it as some kind of extension, or an alternative option. Not as default, for sure. My opinion on this is it's up to the implementer to decide which sqlite to link to. It can be the system one, or one you provide. That is true on iOS now. We only link to the system one for testing purposes.

We do alternate linking for sqlcipher, and I know of at least one implementation that ships with their own sqlite. If "amalgamation" is something else, we can discuss (I'm doing a whole bunch of Monday meetings, so haven't really digested the link).

Having said that, sqldelight adds that linker flag in the plugin, and you need to explicitly tell it not to if you don't want that. Use linkSqlite = false in the db config in gradle. If you do static frameworks, it won't propagate, but dynamic will (IIRC, that is maybe a cocoapods thing instead, but it's been a while since I've had to sort that out in a new config).

I see. Yeah will try that.
Hmm yeah i see your point. And i didn't think of the case where a user of this lib would link to something else.
It's not really "shipping with your binary", sqlite becomes "part of your binary". But still the same regarding our discussion here I guess.

Given your explanation, I definitely agree that this should not be the default then.
If you're open for the idea in general, I'll look into if I find a form of extension or config setup which allows for the needed flexibility and ease of use.