RFC: Swap out SQLite dependency

[designatednerd]

SQLite.swift does not appear to still be actively developed, as it hasn't received a new commit for the last year. We'd like to make sure we're using something that gets updates reasonably often, so I propose moving us to a different underlying SQLite dependency.

There are two big ones I can think of: FMDB and GRDB. Of those two, I'm leaning towards GRDB since:

• It is in Swift, which lines up with our commitment to keep things as Swifty as possible.
• It also supports SQLCipher, which I know some folks are using for db encryption.
• It is very actively maintained - as of my most recent look there were zero open issues or PRs and the last commit was two hours ago.

I'm planning to do some tests to make sure databases created by SQLite.swift can be opened by GRDB, but I think this shouldn't be a big deal.

Would love to hear from folks using ApolloSQLite about how swapping out the underlying library would affect you:

• Are you 👍 or 👎 on this change, or something more mixed?
• Are you using SQLite.swift outside of Apollo as well, or only with Apollo?
• Is it a serious problem if I can't make the migration from an old database work, and you have to create a new database?

[groue]

Hello, I'm the maintainer of GRDB. Thanks for considering using it!

• Are you using SQLite.swift outside of Apollo as well, or only with Apollo?

Applications can depend on both SQLite.swift and a future Apollo+GRDB without any conflict.

Now, mixing both the system SQLite and SQLCipher in a single binary is a recurrent pain point. The problem is that both libraries expose the same C symbols, and this trips up the linker. In an ideal world, Apollo would accept both, so that applications can choose which SQLite flavor they prefer (when they maintain their own database for whatever purpose). This is easy with CocoaPods (with an Apollo/SQLCipher subspec), but I have no clue for other package management systems.

Also, SQLite throws errors when two database connections attempt at concurrently write in a given database file (an SQLite database has a single write lock). It also throws errors when reads happen concurrently with writes, unless the database is in the WAL mode. If you intend to expose the Apollo database file to both GRDB and SQLite.swift, then you'll have to prevent or deal with those errors. If the database file is not shared, then you don't have to worry.

• Is it a serious problem if I can't make the migration from an old database work, and you have to create a new database?

You shouldn't need to: GRDB accepts all SQLite databases.

[eliperkins]

Are you 👍 or 👎 on this change, or something more mixed?

Sweet! 👍 Given how modular using these stores are, migrating the Apollo-specific solution to another SQLite library seems completely fair to me!

Are you using SQLite.swift outside of Apollo as well, or only with Apollo?

We're using SQLite.swift in other parts of GitHub for iOS, where we need simple tables of data but don't need all the glitz and glamour of Core Data.

Reiterating the earlier point, all of these bits are pretty modular anyway, so swapping out the database that Apollo uses won't affect our other usages of SQLite.swift for another library.

Is it a serious problem if I can't make the migration from an old database work, and you have to create a new database?

Not at all! It's a cache, so it seems like evicting/emptying the old cache should let things work just fine anyway, where data can be fetched from the server in most/all cases!

---

Related thought exercise as I wrote up this comment: is there a reason this needs to be done via SQLite with another dependency, versus using Core Data?

As much as I dislike using Core Data (given that "you're holding it wrong" is really easy to do, evidenced by the 387923487 blog posts about using Core Data "correctly"), using it as a simple persistent key-value store doesn't seem like the worst thing to set up (no relationships to maintain, a single table in the model, etc.)

The upside of using Core Data would mean that consumers wouldn't need to integrate another third-party dependency beyond Apollo.

Not super advocating for Core Data here, but mainly trying to understand the thought process towards using SQLite here!

[AnthonyMDev]

Related thought exercise as I wrote up this comment: is there a reason this needs to be done via SQLite with another dependency, versus using Core Data?

As much as I dislike using Core Data (given that "you're holding it wrong" is really easy to do, evidenced by the 387923487 blog posts about using Core Data "correctly"), using it as a simple persistent key-value store doesn't seem like the worst thing to set up (no relationships to maintain, a single table in the model, etc.)

@eliperkins I've thought pretty extensively about integrating CoreData since I joined the Apollo team. It's doable, but it kind of works against the whole idea of GraphQL, to the point where I don't think it's valuable.

With GraphQL queries, you are usually fetching only the needed fields from an entity at any given time. (One of the big benefits of GraphQL is that it helps prevent overfetching.) If you fetch some fields on an object in one query and other fields at another time, then merging them onto you NSManagedObjects means you need to have all the fields on your objects be optional (because you never know what fields have been fetched).

There are a lot of other complications with subtypes and fragments that CoreData wouldn't represent very well also.

I'm not opposed to a CoreData wrapper being created at some point, but I don't think it will be as useful or easy to use as people hope.

[eliperkins]

With GraphQL queries, you are usually fetching only the needed fields from an entity at any given time. (One of the big benefits of GraphQL is that it helps prevent overfetching.) If you fetch some fields on an object in one query and other fields at another time, then merging them onto you NSManagedObjects means you need to have all the fields on your objects be optional (because you never know what fields have been fetched).

Oh, I really only meant a Core Data implementation of NormalizedCache with the same SQL-ish structure of a single Record entity, just as the current SQLite implementation has.

I 💯 agree that exposing full models from a GraphQL schema via NSManagedObjects would not be a good idea!

I'm mainly just trying to understand SQLite vs other database-providers when it comes to fulfilling NormalizedCache's requirements here.

[designatednerd]

@eliperkins Completely reasonable question! I think it comes down to the fact that Core Data is a relational abstraction that maps onto a database rather than a relational database itself. Even though in practice it's mostly used with an underlying SQLite database, that isn't absolutely necessary. It's concentrated on figuring out how things are related, and abstracts all the actual work to fetch things from the database away from you. @AnthonyMDev outlines pretty clearly above why that approach doesn't work very well with the way GraphQL works.

@groue Man I already was about to tell you to take a break because you're so on top of GRDB, then I didn't even tag you on this and you're the first comment 😆 . Right we tell folks using SQLCipher to pass that in to the database connection for SQLite.swift so that we don't have to have that dependency on our end, only the consuming app does. I haven't dug far enough into GRDB to see if that works the same but if it does, then that should solve that issue.

[designatednerd]

I'm mainly just trying to understand SQLite vs other database-providers when it comes to fulfilling NormalizedCache's requirements here.

I think there's a lot of reasons, not least that SQLite is heavily supported elsewhere, so you get things like "Someone wrote an encryption layer for this!" without us having to do anything. I think there's certainly room for other implementations if you find them to be more efficient, but from a standpoint of what we can realistically maintain for everyone, SQLite seems like a good place to start.

[miradesne]

I'm working on an app and using SQLite.swift because I saw Apollo using it. Now reading stephencelis/SQLite.swift#1031 (comment) I'm a bit concerned if I should keep using SQLite.swift. For people that use ApolloSQLite -- have you had any issues with App Store submission?

[designatednerd]

I haven't heard of any trouble with App Store submission. And people are not shy about telling me their troubles.😜

[designatednerd]

OK, last call for comments - keeping this open until wednesday and then I'm gonna start with the sledgehammer. 😇

[groue]

@designatednerd, I don't know if you plan to switch to GRDB. I have two concerns against choosing it today:

• SQLite is only used in Apollo's SQLiteNormalizedCache.swift, for simple CRUD operations. Doesn't GRDB come with much too many features for this simple use case? That would be a lot of useless code to compile and embed in client apps. Maybe GRDB could be modularized, but this is not the case today: it's an all-or-nothing library in its current form.
• The GRDB git repository contains bad big commits (years ago, I committed big files by mistake) that slow down its checkout. I don't quite know how to fix the repository, unfortunately. But mind this could slow down the Apollo users experience.

I think it's worth performing checks about download time, compile time, and code size.

Given SQLiteNormalizedCache.swift is really simple, did you consider... dropping any dependency and use the raw C SQLite API?

[designatednerd]

The GRDB git repository contains bad big commits (years ago, I committed big files by mistake)

We've got a couple of those in here as well, so I don't think that's going to be as much of a concern.

did you consider... dropping any dependency and use the raw C SQLite API?

Definitely understand the thought, but there's a bunch of stuff that we'd need to figure out in terms of making that work across all package managers that I strongly suspect is going to be extremely time-consuming given problems we've had in the past with SQLite.swift throwing absolute fits with Carthage in particular.

In terms of what we're trying to accomplish with Apollo, I'm not sure that's the best use of our time right now when solutions that have already handled this and have thoroughly tested it (such as yours) already exist.

[designatednerd]

@groue I'm going to see what I can do given what #1722 uncovered, but no guarantees you're off the hook yet.

[nathanfallet]

Hello, I joined the SQLite.swift maintainers team, and we're going to merge pull requests and fix issues in the coming days. (Already a few were merged at time of writing, and we are replacing the outdated travis CI with GitHub Actions)

[AnthonyMDev]

That is so exciting to hear @nathanfallet ! Thanks for letting us know! We will definitely revisit this when we get back to this part of the project on our roadmap and see if sticking with SQLite.swift makes sense now that we know it's being maintained again.

[groue]

The future SQLite.swift version, which is intended to ship with this change, will stop notifying about database errors during iteration of database results. SQLite.swift will just pretend the iteration has ended normally.

I'm not here to have an opinion on this change on SQLite.swift, but I'm here to help Apollo, which I use, not suffer from unintended consequences, if I can.

This Apollo line is impacted. Currently it crashes in case of database error. In the future, it will ignore database errors, and just return incomplete arrays (with consequences I'm not able to evaluate).

The safer SQLite.swift API is FailableIterator.failableNext(). It does not hide errors, it is already available today, and it is likely to remain so in the future SQLite.swift release.