Add support for pre-update hooks #597
Conversation
| @@ -19,6 +19,7 @@ package sqlite3 | |||
| #cgo CFLAGS: -DSQLITE_TRACE_SIZE_LIMIT=15 | |||
| #cgo CFLAGS: -DSQLITE_OMIT_DEPRECATED | |||
| #cgo CFLAGS: -DSQLITE_DISABLE_INTRINSIC | |||
| #cgo CFLAGS: -DSQLITE_ENABLE_PREUPDATE_HOOK | |||
There was a problem hiding this comment.
Please move this to a file named sqlite3_opt_preupdate_hook.go and set a build tag sqlite_preupdate_hook
Because of possible performance penalities, this should be an option which a user should activate with a build tag.
| @@ -118,6 +119,7 @@ int compareTrampoline(void*, int, char*, int, char*); | |||
| int commitHookTrampoline(void*); | |||
| void rollbackHookTrampoline(void*); | |||
| void updateHookTrampoline(void*, int, char*, char*, sqlite3_int64); | |||
| void preUpdateHookTrampoline(void*, sqlite3 *, int, char *, char *, sqlite3_int64, sqlite3_int64); | |||
There was a problem hiding this comment.
Please move this to a file named sqlite3_opt_preupdate_hook.go and set a build tag sqlite_preupdate_hook
There was a problem hiding this comment.
See sqlite3_opt_userauth.go as an example
| @@ -253,6 +255,17 @@ type SQLiteRows struct { | |||
| done chan struct{} | |||
| } | |||
|
|
|||
| // SQLitePreUpdateData represents all of the data available during a | |||
| // pre-update hook call. | |||
| type SQLitePreUpdateData struct { | |||
There was a problem hiding this comment.
Please move this to a file named sqlite3_opt_preupdate_hook.go and set a build tag sqlite_preupdate_hook
| OldRowID int64 | ||
| NewRowID int64 | ||
| } | ||
|
|
||
| type functionInfo struct { |
There was a problem hiding this comment.
Please move this to a file named sqlite3_opt_preupdate_hook.go and set a build tag sqlite_preupdate_hook
| // If there is an existing update hook for this connection, it will be | ||
| // removed. If callback is nil the existing hook (if any) will be removed | ||
| // without creating a new one. | ||
| func (c *SQLiteConn) RegisterPreUpdateHook(callback func(SQLitePreUpdateData)) { |
There was a problem hiding this comment.
Please move this to a file named sqlite3_opt_preupdate_hook.go and set a build tag sqlite_preupdate_hook
| @@ -1977,3 +2006,70 @@ func (rc *SQLiteRows) Next(dest []driver.Value) error { | |||
| } | |||
| return nil | |||
| } | |||
|
|
|||
| // Depth returns the source path of the write, see sqlite3_preupdate_depth() | |||
| func (d *SQLitePreUpdateData) Depth() int { | |||
There was a problem hiding this comment.
Please move this to a file named sqlite3_opt_preupdate_hook.go and set a build tag sqlite_preupdate_hook
| } | ||
|
|
||
| // Count returns the number of columns in the row | ||
| func (d *SQLitePreUpdateData) Count() int { |
There was a problem hiding this comment.
Please move this to a file named sqlite3_opt_preupdate_hook.go and set a build tag sqlite_preupdate_hook
| return int(C.sqlite3_preupdate_count(d.Conn.db)) | ||
| } | ||
|
|
||
| func (d *SQLitePreUpdateData) row(dest []interface{}, new bool) error { |
There was a problem hiding this comment.
Please move this to a file named sqlite3_opt_preupdate_hook.go and set a build tag sqlite_preupdate_hook
| @@ -1513,6 +1513,120 @@ func TestPinger(t *testing.T) { | |||
| } | |||
| } | |||
|
|
|||
| type preUpdateHookDataForTest struct { | |||
There was a problem hiding this comment.
Please move this to a file named sqlite3_opt_preupdate_hook_test.go and set a build tag sqlite_preupdate_hook
| newRow []interface{} | ||
| } | ||
|
|
||
| func TestPreUpdateHook(t *testing.T) { |
There was a problem hiding this comment.
Please move this to a file named sqlite3_opt_preupdate_hook_test.go and set a build tag sqlite_preupdate_hook
|
See review and please rebase |
|
@rbranson Moving this to its own option like userauth and others will also fix the build errors. |
Because the closed property of the SQLiteRows's *SqliteStmt was not guarded, it was causing an issue during context cancellation. https://github.com/segmentio/go-sqlite3/blob/be424d27acde822f080bdcd8a7ae6abd4d7d801e/sqlite3.go#L1785-L1796 When a statement is performing a query(), if it determines that the context has been canceled, it will launch a goroutine that closes the resulting driver.Rows if it's not already completed. If the driver.Rows is not done (and the context has been canceled), it will interrupt the connection and more importantly, perform a rows.Close(). The method rows.Close() guards the closed bool with a sync.Mutex to set it to true. If a reader is reading from the SqliteRow, it will call Next() and that performs this check: https://github.com/segmentio/go-sqlite3/blob/be424d27acde822f080bdcd8a7ae6abd4d7d801e/sqlite3.go#L1915-L1917 Because this is not guarded, a data race ensues, and this was actually caught by the Go race detector recently. I didn't include a test case here because the fix seemed straightforward enough and because race conditions are hard to test for. It's been verified in another program that this fixes the issue. If tests should be provided I'm more than happy to do so.
Ensure that SqliteStmt.closed property is guarded.
|
Implemented in v2.0.0 branch |
|
Closed by #736 |
This PR adds support for pre-update hooks. Pre-update hooks allow getting the before & after row images for a given mutation. The standard update hook is insufficient for many use cases. Some careful orchestration would allow getting the new row image by ID in the insert & update cases, in the delete case, the data is already gone.
The only caveat here is that it does require compiling SQLite with the SQLITE_ENABLE_PREUPDATE_HOOK directive. Given that it is not the default, one would think there is additional risk or a performance penalty involved.