gh-128505: Expose an interface to sqlite3_file_control#128507
gh-128505: Expose an interface to sqlite3_file_control#128507hashbrowncipher wants to merge 15 commits intopython:mainfrom
Conversation
f36b975 to
e32c372
Compare
erlend-aasland
left a comment
There was a problem hiding this comment.
I briefly started looking at this; will do a full review later today (CET).
| @@ -0,0 +1 @@ | |||
| sqlite Connection objects now expose a method set_file_control, which is a thin wrapper for `sqlite3_file_control https://www.sqlite.org/c3ref/file_control.html`_. | |||
There was a problem hiding this comment.
See https://devguide.python.org/documentation/markup/ for help with docs markup.
Modules/_sqlite/connection.c
Outdated
| /*[clinic end generated code: output=d9d2d311892893b6 input=0253798d9514fea2]*/ | ||
| { | ||
| int rc; | ||
| long val = arg; |
There was a problem hiding this comment.
Not all sqlite3_file_control opcodes use the int1 datatype. For example SQLITE_FCNTL_VFSNAME and SQLITE_FCNTL_TEMPFILENAME are char *. Suggesting to limit this (like we do for sqlite3_db_config) to only the supported opcodes, and raise an exception for unsupported opcodes.
In any case, we should use int here.
| long val = arg; | |
| int val = arg; |
Footnotes
-
int, notlong↩
There was a problem hiding this comment.
Please also address the first part of this remark.
There was a problem hiding this comment.
oops! sorry for making you ask twice
There was a problem hiding this comment.
Done. I have also removed the SQLITE_FCNTL_TEMPFILENAME and SQLITE_FCNTL_VFSNAME constants. I hope that's alright.
223328f to
4867672
Compare
|
Just a heads-up: we normally do not force-push. PRs are squashed into a single commit when we merge to |
Add a private API for raising DB-API compatible exceptions based on the result code of SQLite C APIs. Some APIs do not store the error indicator on the database pointer, so we need to be able to deduce the DB-API compatible exception directly from the error code. - rename _pysqlite_seterror() as set_error_from_db() - introduce set_error_from_code()
…qlite3_file_control
|
|
||
| def test_file_control_raises(self): | ||
| with memory_database() as cx: | ||
| with self.assertRaises(sqlite.InternalError): |
There was a problem hiding this comment.
InternalError sounds suspicious. What's the underlying SQLite error code? SQLITE_MISUSE?
There was a problem hiding this comment.
Based on the failed test run, the full exception is sqlite3.InternalError: unknown operation, which appears to correspond to SQLITE_NOTFOUND.
|
@hashbrowncipher, can you sync with |
sqlite3_file_control can affect the semantics of a sqlite3 database, but is not exposed by the sqlite.Connection class. In particular, this addition allows callers to use the SQLITE_FCNTL_PERSIST_WAL opcode. SQLITE_FCNTL_PERSIST_WAL ensures that callers can open a WAL-mode database on a read-only mount even when the writers have closed the DB.