IreneKnapp / direct-sqlite

MIT License
35 stars 54 forks source link

Add execPrint, lastInsertRowId, and other improvements #22

Closed joeyadams closed 11 years ago

joeyadams commented 11 years ago

This pull request does several little things:

nurpax commented 11 years ago

Nice, I've been meaning to add last_insert_rowid but looks like I don't need to. :)

@IreneKnapp I'm also planning to add a logging feature to direct-sqlite (https://github.com/nurpax/sqlite-simple/issues/21) - I can try to get it done over the weekend. Perhaps batch that changes with these in your next release?

IreneKnapp commented 11 years ago

Excellent work, thank you! I'm merging now and will do the release Monday; Janne wants a chance to do some logging infrastructure.

nurpax commented 11 years ago

@IreneKnapp Yes, my plan was to add support for enabling tracing with sqlite3_trace. This would be a useful debugging feature. However, looking at joey's code, hooking up callbacks seem to be somewhat complicated and I think I want to spend a bit more time on this. So please don't let my pull request hold back the release, it may be that I'll work on tracing a little bit later.

joeyadams commented 11 years ago

@nurpax: The main gotcha is that when a FunPtr created by foreign import ccall "wrapper" throws an exception, it crashes the whole program (I think). Thus, you'll usually want to catch the exception within the callback and handle it somehow.

In the case of execWithCallback, I stuff the exception in an IORef, return nonzero to abort the query, then throw the exception when sqlite3_exec fails with SQLITE_ABORT. Likewise, if we were to add support for [sqlite3_create_function](to add custom functions to SQL), we could use sqlite3_result_error to set an error message and tell SQLite3 that our function failed.

In the case of sqlite3_trace, assuming you use it solely for debugging and profiling, you might want to just ignore the exception and let it crash the program. Error on output would be quite exceptional indeed.

I would be happy to review a pull request for sqlite3_trace. Just ping me so I'll see it.

IreneKnapp commented 11 years ago

I'm going to go ahead and release now, as 2.3.1 because I believe we had decided to go back to that numbering scheme after last time.

(If you got this message twice, sorry! Accidentally submitted it through my work account - they don't yet know I'm trans, so I kinda had to delete it to avoid leaving a breadcrumb. Oh well.)