Add sqlite3_sql & debug trace support

[nurpax]

As discussed in https://github.com/IreneKnapp/direct-sqlite/pull/22, here's my PR for adding tracing support. I think this is a very useful feature for debugging. If you read the comments, you will see that there's some amount of unsafety involved.

There's also a minor memory leak in there. When setting a log function with setTrace, a single Haskell FunPtr is leaked.. I thought it's not really a big deal, since this function is probably only called once per application invocation. If you want to get rid of it, you'd need to add a ref to the Connection object, which would need a lot more changes for close to zero !/$.

This functionality is exposed through only Database.SQLite3.Bindings and *.Direct, but not via Database.SQLite3. Database.SQLite3 will be easier to add later if necessary.

BTW - I'm open to renaming suggestions if you have a better name for statementText and/or setTrace.

@joeyadams You wanted to be pinged for code review.. Here you go!

[joeyadams]

Thanks. I made some tweaks and squashed them into your two commits. You can get them from my tracing branch and force-push to yours.

Tweaks I made:

• Fixed the signature of c_sqlite3_trace to include the returned pointer. We would need it to implement a version of c_sqlite3_trace that doesn't leak FunPtrs.
• Moved the c_sqlite3_sql binding up with the other statement management bindings.
• Added a comment about the FunPtr leak.
• Changed Utf8 . T.encodeUtf8 . T.pack $ "" to just Utf8 BS.empty.

For statementText, perhaps a better name would be statementSql. Up to you.

In the commit message, you say statementText

... can be used for debugging purposes -- say formatting error messages with SQL in them ...

I should point out that it's kind of a security risk to include SQL queries in error messages. If the person using your library isn't careful, end users might see them and know things about the business logic they shouldn't.

In module exports, you can put a comma at the end of the last export, like you can with enumerations and arrays in C:

module Foo (
    a,
    b,
    c,
) where

This makes it more diff-friendly, so if someone adds d later, they won't have to change the line for c.

Finally, regarding the testsuite and use of assertEqual versus partial patterns: do what is easiest. IMHO, the most important job of the testsuite is to fail if something doesn't do what we expect. If a test fails but we don't know what failed, we can still narrow down the problem by hand. Thus, it's more far important that we get around to writing tests at all than it is to have tests that give us useful location information on failure.

[nurpax]

Thanks, good clean up! I missed the sqlite3_trace return value - good catch.

I pushed your updated versions into this PR (github doesn't show it here in this thread though).

@IreneKnapp As far as I am concerned, this PR should be good to go.

...

... can be used for debugging purposes -- say formatting error messages with SQL in them ...

I should point out that it's kind of a security risk to include SQL queries in error messages. If the person using your library isn't careful, end users might see them and know things about the business logic they shouldn't.

Good point. I should've used more precise wording. I actually needed this to construct exception objects that contain the SQL query in them, and are handy for debugging query problems. These are not mean to be show'n to the user.

Diff-friendly lists - I tend to use the other convention where the comma starts the line. Only seen it used in Haskell but kind of like it.

Also agreed on the testsuite part.

PS. Sorry for the messy history, I wanted to squash all the 3 last commits into a single commit but pushed too early and fumbled a bit with rebasing.

[IreneKnapp]

Excellent work, both of you. My pleasure to merge this. Will publish to hackage in the morning when I'm back at a proper keyboard.