LGTM so far!

(Just needs a change log update for merge)

@MaxGabriel I'll add a changelog soon, I'm also experimenting with variations of withSqliteConn & co that make it easier to actually access the Connection. I'm still playing what works nicest in my code, probably something along the lines of withSqliteConn :: (MonadBaseControl IO m, MonadIO m, MonadLogger m, IsSqlBackend backend) => Text -> (Connection -> backend -> m a) -> m a (not sure whether that should just go into Database.Persist.Sqlite or into a separate Database.Persist.Sqlite.Internal). Anyway, I'm also dealing with a paper deadline, so I will probably finalise this PR and add a change log somewhere next week after the paper demon has been slain.

Ok, so I finally had time to get back to this. I added a RawSqlite type and corresponding withRawSqliteConnInfo that packs up the Sqlite.Connection with the persistent SqlBackend. So that it's easier to run a persistent monad and still have access to the pointer via Database.Sqlite.Internal.

I implemented all the relevant PersistCore, PersistStoreRead, etc. classes too (via liftPersist), so it should be possible to just directly use ReaderT (RawSqlite SqlBackend) as drop-in for ReaderT SqlBackend.

I'm not very opinionated about the names of types/fields, because I suck at coming up with them, so feel free to suggest better ones.

Also, I could migrate the new functionality to a new Database.Persist.Sqlite.Internal module, but that would've required moving a bunch of the existing code and then importing it back, which seemed like a hassle, but if that's preferred I can change it.

If people support the change suggested in #783 this PR might have to wait those changes get in so I can add a withRawSqlitePoolInfo variant too.

@merijn I think you mean #783

@MaxGabriel whoops, yeah.

@MaxGabriel besides waiting for what to do with #783 is there anything you'd like to see changed/named differently/organised about this? (I'd like to at least settle on the names/modules so I can commit to them in my own code, regardless of when this gets merged/released)

@merijn Yeah this looks good to me, the only thing I would ask to change is can you export separate functions for the RawSqlite constructor? The approach here https://www.snoyman.com/blog/2016/11/designing-apis-for-extensibility or the lens approach used in the main SQLite module are OK. Sorry this isn't documented anywhere—Persistent recently is trying to do new constructors this way

(If anyone else wants to take a look at this now would be a good time)

#886 has been merged via #893, so work on this can resume with a bit more productivity I hope :)

@merijn This PR looks great! once you've resolved merge conflicts (should be fairly straightforward) then we can get this released 😄

@parsonsmatt I'll update this "Real Soon (TM)" (it'll have to wait until I'm done moving)

Finally getting around to reviving this. Still have to figure out how to fit the code with the revamped typeclasses, so consider this a work-in-progress.

Ok, I think that's everything? Seems like it needed less work than I thought.

@parsonsmatt I think this should be good to merge (assuming the tests don't unexpectedly break).

Incidentally, who is responsible for maintaining/merging PRs for persistent-sqlite (and persistent-template)?

I've got a few other (super simple) patches that fix minor warts/problems lying around to PR, but I'm unsure who to ping to get them looked at, since some of the PR seem to take quite a while to get noticed and I'd prefer to finish upstreaming them in less than 1.5 years ;)

@merijn I'm helping out with it where I can, though I ping @snoyberg for anything more involved as I don't use SQLite much. I'll take a look at this today, thanks!

I am, however, pre-caffeinated, so I am going to take another peek in an hour or two before merging.

OK, so on a first pass, this looks great. Can you add a test or test-suite that will ensure that this new Connection type works with all the existing functionality?

OK, so on a first pass, this looks great. Can you add a test or test-suite that will ensure that this new Connection type works with all the existing functionality?

Do you mean the RawSqlite type? The Connection type is completely unchanged, it's just been moved from Database.Sqlite to Database.Sqlite.Internal so that the constructor can be exported.

Yes, that's right - I was typing from memory, my bad.

In retrospect I think having it satisfy all the class instances from persistent should do it. I'll review the code again and see if there's aynthing missing.