Add sqlite3_file_control() support #1000
Conversation
sqlite3.go
Outdated
| // return code are both opcode-specific. Please see the SQLite documentation. | ||
| // | ||
| // See: sqlite3_file_control, https://www.sqlite.org/c3ref/file_control.html | ||
| func (c *SQLiteConn) FileControl(dbName string, op int, arg unsafe.Pointer) int { |
There was a problem hiding this comment.
The last argument seems somewhat troubling to me. For example, for some of the ops, it needs to be a pointer to some C struct, but the caller will be hard-pressed to do that within the confines of Go. Consequently, I'm not sure we really want to expose sqlite3_file_control directly like this.
Taking a page from the syscall package, we could do something like SetFileControlInt/GetFileControlInt, and use a more user-friendly type for the final parameter, performing the conversion to pointer internally.
Alternatively, we can make a method specifically for getting/setting the persistent WAL mode.
There was a problem hiding this comment.
Good call. I switched it to FileControlInt() instead. 👍
sqlite3_test.go
Outdated
| ConnectHook: func(conn *SQLiteConn) error { | ||
| arg := int(1) | ||
| if rc := conn.FileControl("", SQLITE_FCNTL_PERSIST_WAL, unsafe.Pointer(&arg)); rc != 0 { | ||
| t.Fatalf("Unexpected return code from FileControl(): %d", rc) |
There was a problem hiding this comment.
You probably should not call t.Fatalf here, since this may be happening on some other goroutine, depending how database/sql works. Instead, just return an error. Below, call db.Ping after sql.Open and move the assertion there.
There was a problem hiding this comment.
Fixed. It now returns an error instead.
benbjohnson
force-pushed
the
sqlite3-file-control
branch
from
ceb814b to
27cc653
Compare
|
@rittneje I fixed up the PR. Can you take another look please? |
sqlite3.go
Outdated
| // Standard File Control Opcodes | ||
| // See: https://www.sqlite.org/c3ref/c_fcntl_begin_atomic_write.html | ||
| const ( | ||
| SQLITE_FCNTL_LOCKSTATE = 1 |
There was a problem hiding this comment.
SQLITE_FCNTL_LOCKSTATE = int(C.SQLITE_FCNTL_LOCKSTATE)
There was a problem hiding this comment.
Can you use C.SQLITE_FCNTL_XXX instead of number literals?
There was a problem hiding this comment.
@mattn Good call. I updated it and push up a new commit. 👍
benbjohnson
force-pushed
the
sqlite3-file-control
branch
from
27cc653 to
fb398cc
Compare
benbjohnson
force-pushed
the
sqlite3-file-control
branch
from
fb398cc to
101a6aa
Compare
benbjohnson
force-pushed
the
sqlite3-file-control
branch
from
101a6aa to
67844d2
Compare
Codecov Report
@@ Coverage Diff @@
## master #1000 +/- ##
==========================================
+ Coverage 45.98% 46.04% +0.05%
==========================================
Files 11 11
Lines 1481 1490 +9
==========================================
+ Hits 681 686 +5
- Misses 665 667 +2
- Partials 135 137 +2
Continue to review full report at Codecov.
|
benbjohnson
force-pushed
the
sqlite3-file-control
branch
from
67844d2 to
c3a7284
Compare
|
@rittneje good to merge? |
| @@ -297,6 +297,47 @@ const ( | |||
| /*SQLITE_RECURSIVE = C.SQLITE_RECURSIVE*/ | |||
| ) | |||
|
|
|||
| // Standard File Control Opcodes | |||
| // See: https://www.sqlite.org/c3ref/c_fcntl_begin_atomic_write.html | |||
| const ( | |||
There was a problem hiding this comment.
@mattn By referencing the actual C constants here, this won't compile for anyone linking with an older version of SQLite (via the libsqlite3 tag). For example, SQLITE_FCNTL_SIZE_LIMIT wasn't added until 3.27.0.
How do you want to handle this?
There was a problem hiding this comment.
Ah, I didn't consider it. @benbjohnson Sorry much. Could you please rollback changes for the constants?
This commit adds the SQLiteConn.FileControlInt() method which calls the underlying sqlite3_file_control() function with an int argument. This can be used for low-level operations on SQLite databases such as persisting the WAL file after database close.
benbjohnson
force-pushed
the
sqlite3-file-control
branch
from
c3a7284 to
c640ad0
Compare
|
Thank you |
This pull request adds the
SQLiteConn.FileControl()method which calls the underlyingsqlite3_file_control()function. This can be used for low-level operations on SQLite databases such as persisting the WAL file after database close.Fixes #999