Add modules to direct-sqlite for lower-level access

[joeyadams]

Sorry, this is a pretty big pull request. The commits trace the development history, which is basically:

• Add a module with FFI bindings.
• Change Database.SQLite3 to use those bindings.
• Add Database.SQLite3.Direct, a lower-level variant of Database.SQLite3 implemented with the FFI bindings.
• Change Database.SQLite3 again, to use Database.SQLite3.Direct.

  Merging information

This pull request renames SQLite3.hsc to SQLite3.hs, which makes the diff a bit less useful. To see what changed in Database.SQLite3, you can do the following:

git remote add joeyadams git://github.com/joeyadams/direct-sqlite.git
git fetch joeyadams
git diff 84efb91:Database/SQLite3.hsc b312202:Database/SQLite3.hs

This pull request includes the commit by @nurpax from pull request #9, with the merge conflict resolved.

What this pull request does

This splits direct-sqlite into three levels of abstraction:

• Database.SQLite3: Existing API.

• Database.SQLite3.Direct: Like Database.SQLite3, but return errors instead of throwing them, and only use cheap conversions (meaning no String and Text).

  Note: the SQLData type defined here is different from the one in Database.SQLite3. The new SQLData type has strictness annotations, so it is incompatible with the old type. This should be fixed in a future version, once we decide to break the existing API.

• Database.SQLite3.Bindings: Raw FFI bindings, using newtypes to clarify parameters and make the bindings easier to use correctly.

My goal was to avoid breaking the existing API, so people can start using the new internals without compatibility issues. However, some minor changes have been made to the existing API:

• In bindInt, use sqlite3_bind_int64 instead of sqlite3_bind_int, to avoid truncation on platforms where CInt is smaller than Int.
• bindText no longer truncates to the first NUL character. The database might, though.
• Add Show instances for Database and Statement.
• Tweak documentation of bindParameterCount and bindParameterName.

  Future directions

I took plenty of liberties with Database.SQLite3.Direct. It represents the direction I would like to see Database.SQLite3 go, namely:

• Stronger typing
• More precise error handling
• Hyperlinks to upstream documentation

I'd also want to factor the Error type:

data Result
    = Okay
    | StepResult    !StepResult
    | Error         !Error

    -- Extended error types; see http://www.sqlite.org/c3ref/c_abort_rollback.html
    | ErrorIO       !ErrorIO
    | ErrorLocked   !ErrorLocked
    | ErrorBusy     !ErrorBusy
    | ErrorCantOpen !ErrorCantOpen
    | ErrorCorrupt  !ErrorCorrupt
    | ErrorReadOnly !ErrorReadOnly
    | ErrorAbort    !ErrorAbort

But we'd have to remove these data constructors from Error:

• ErrorOK
• ErrorRow
• ErrorDone

[IreneKnapp]

Just for the record, if you use git mv in future, it preserves history on the file. I'm now reading through this all!

[joeyadams]

Just for the record, if you use git mv in future, it preserves history on the file.

Unfortunately, that's not true: http://stackoverflow.com/questions/1094269/whats-the-purpose-of-git-mv

That is, unless they changed it in a recent version of git. In any case, thanks for putting up with my shenanigans.

[IreneKnapp]

Just as a comment, we made a breaking API change so recently (the switch to Text, with 2.0) that I'm not worried about a second one. So you should probably go ahead and add any changes that you'd like to make "when we're ready to break things".

[joeyadams]

Just as a comment, we made a breaking API change so recently (the switch to Text, with 2.0)

Wow, only five days ago. I didn't realize it was that recent.

So you should probably go ahead and add any changes that you'd like to make "when we're ready to break things".

I'd be happy to. In any case, since no functions in Database.SQLite3 currently take or return an 'Error', I doubt changing it will cause compatibility issues.

[nurpax]

I won't mind any API breaking changes in direct-sqlite, but if you do break the existing API, please bump up the version number to 2.1 so that I can protect sqlite-simple and snaplet-sqlite-simple with a version upper bound.

[IreneKnapp]

Yes, naturally. Don't worry!

On Mon, Aug 20, 2012 at 3:13 AM, Janne Hellsten notifications@github.comwrote:

I won't mind any API breaking changes in direct-sqlite, but if you do break the existing API, please bump up the version number to 2.1 so that I can protect sqlite-simple and snaplet-sqlite-simple with a version upper bound.

— Reply to this email directly or view it on GitHubhttps://github.com/IreneKnapp/direct-sqlite/pull/12#issuecomment-7862237.

-- Irene Knapp

[joeyadams]

Actually, I decided not to break down the error code enumeration (data Result = ...). I see it as a lot of work, with very little benefit:

• We'd have to rename CError to CResult all over the place.
• Users would have redundant-looking patterns like Left (Error ErrorError).

One thing I would like to break, though, is the name of bind:

bind :: Statement -> [SQLData] -> IO ()

Unlike column, which only reads one column, bind sets all the parameters. Here's what I'd like to do instead:

bind :: Statement -> ParamIndex -> SQLData -> IO ()
binds :: Statement -> [SQLData] -> IO ()

This should also make it easier for people to implement named parameters in their higher-level libraries.

[joeyadams]

@IreneKnapp: Have you finished reviewing this pull request yet? I plan to improve the API of Database.SQLite3 itself, but the commits will be based on the ones in this pull request.

[IreneKnapp]

Sorry it's taking so long! I want to understand it, not just rubber-stamp it. It's fairly invasive and if I don't keep myself current with what's going on in the codebase, I'll cease to understand it, you know? Hopefully tonight.

[IreneKnapp]

(If I conclude that I'm never going to get the time, then I will just rubber-stamp it, just to be clear. That's the only thing that would be fair. I'm aware that this phase when you've done the work and are waiting to hear back is pretty frustrating. But right now I expect to have time.)

[IreneKnapp]

I guess I'll just trust that your notes are accurate rather than reading and understanding the code. I feel like I've kept you waiting too long already.